2015-07-23 06:31:52

by Kalle Valo

[permalink] [raw]
Subject: [PATCH] bcma: fix linking problem with of_default_bus_match_table

Stephen reported a build problem caused by commit cae761b5a6bd ("bcma: populate
bus DT subnodes as platform_device-s"):

ERROR: "of_default_bus_match_table" [drivers/bcma/bcma.ko] undefined!

Rafał Miłecki suggested as a quick fix to use IS_BUILTIN() to workaround the
issue. The downside is that this won't work when BCMA is compiled as a module,
but we can live with that for now just to unblock the breakage.

Reported-by: Stephen Rothwell <[email protected]>
Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/bcma/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 59128478a90f..6b7d54622058 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -410,7 +410,7 @@ int bcma_bus_register(struct bcma_bus *bus)
bcma_core_pci_early_init(&bus->drv_pci[0]);
}

- if (bus->host_pdev) {
+ if (IS_BUILTIN(CONFIG_BCMA) && bus->host_pdev) {
struct device *dev = &bus->host_pdev->dev;

of_platform_populate(dev->of_node, of_default_bus_match_table,
--
1.7.9.5



2015-07-23 16:04:21

by Jonas Gorski

[permalink] [raw]
Subject: Re: [PATCH] bcma: fix linking problem with of_default_bus_match_table

On 23.07.2015 17:51, Kalle Valo wrote:
> Jonas Gorski <[email protected]> writes:
>
>> Hi,
>>
>> On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <[email protected]> wrote:
>>
>>> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
>>> index 59128478a90f..6b7d54622058 100644
>>> --- a/drivers/bcma/main.c
>>> +++ b/drivers/bcma/main.c
>>> @@ -410,7 +410,7 @@ int bcma_bus_register(struct bcma_bus *bus)
>>> bcma_core_pci_early_init(&bus->drv_pci[0]);
>>> }
>>>
>>> - if (bus->host_pdev) {
>>> + if (IS_BUILTIN(CONFIG_BCMA) && bus->host_pdev) {
>>> struct device *dev = &bus->host_pdev->dev;
>>>
>>> of_platform_populate(dev->of_node, of_default_bus_match_table,
>>
>> I think this isn't enough, AFAICT this needs IS_BUILTIN(CONFIG_BCMA)
>> && IS_ENABLED(CONFIG_OF), else it will break with OF=n and BCMA=y, as
>> of_default_bus_match_table is only defined/built-in when OF=y.
>
> I just tested and OF=n and BCMA=y case built succesfully for me. I think
> it's because of_default_bus_match_table is created even when CONFIG_OF
> is n. Can you double check, please?
>

Thinking more about it, I know/remember why it complies even if it
shouldn't: the inline prototype for !OF of of_platform_populate does not
use any of the arguments so the compiler optimises the reference out.
So unless you compile with -O0, you are fine.

I always forget that this works, so sorry for the noise.


Jonas

2015-07-26 11:57:37

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] bcma: fix linking problem with of_default_bus_match_table

Kalle Valo <[email protected]> writes:

> Stephen reported a build problem caused by commit cae761b5a6bd ("bcma: populate
> bus DT subnodes as platform_device-s"):
>
> ERROR: "of_default_bus_match_table" [drivers/bcma/bcma.ko] undefined!
>
> Rafał Miłecki suggested as a quick fix to use IS_BUILTIN() to workaround the
> issue. The downside is that this won't work when BCMA is compiled as a module,
> but we can live with that for now just to unblock the breakage.
>
> Reported-by: Stephen Rothwell <[email protected]>
> Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
> Signed-off-by: Kalle Valo <[email protected]>

I dropped this as Hauke sent a better patch.

--
Kalle Valo

2015-07-23 17:15:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] bcma: fix linking problem with of_default_bus_match_table

Jonas Gorski <[email protected]> writes:

>>> I think this isn't enough, AFAICT this needs IS_BUILTIN(CONFIG_BCMA)
>>> && IS_ENABLED(CONFIG_OF), else it will break with OF=n and BCMA=y, as
>>> of_default_bus_match_table is only defined/built-in when OF=y.
>>
>> I just tested and OF=n and BCMA=y case built succesfully for me. I think
>> it's because of_default_bus_match_table is created even when CONFIG_OF
>> is n. Can you double check, please?
>>
>
> Thinking more about it, I know/remember why it complies even if it
> shouldn't: the inline prototype for !OF of of_platform_populate does not
> use any of the arguments so the compiler optimises the reference out.
> So unless you compile with -O0, you are fine.

Thanks for checking. I think this is good enough for now and I'll apply
this once I get the report from kbuild.

--
Kalle Valo

2015-07-23 15:51:30

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] bcma: fix linking problem with of_default_bus_match_table

Jonas Gorski <[email protected]> writes:

> Hi,
>
> On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <[email protected]> wrote:
>
>> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
>> index 59128478a90f..6b7d54622058 100644
>> --- a/drivers/bcma/main.c
>> +++ b/drivers/bcma/main.c
>> @@ -410,7 +410,7 @@ int bcma_bus_register(struct bcma_bus *bus)
>> bcma_core_pci_early_init(&bus->drv_pci[0]);
>> }
>>
>> - if (bus->host_pdev) {
>> + if (IS_BUILTIN(CONFIG_BCMA) && bus->host_pdev) {
>> struct device *dev = &bus->host_pdev->dev;
>>
>> of_platform_populate(dev->of_node, of_default_bus_match_table,
>
> I think this isn't enough, AFAICT this needs IS_BUILTIN(CONFIG_BCMA)
> && IS_ENABLED(CONFIG_OF), else it will break with OF=n and BCMA=y, as
> of_default_bus_match_table is only defined/built-in when OF=y.

I just tested and OF=n and BCMA=y case built succesfully for me. I think
it's because of_default_bus_match_table is created even when CONFIG_OF
is n. Can you double check, please?

--
Kalle Valo

2015-07-23 10:03:15

by Jonas Gorski

[permalink] [raw]
Subject: Re: [PATCH] bcma: fix linking problem with of_default_bus_match_table

Hi,

On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <[email protected]> wrote:
> Stephen reported a build problem caused by commit cae761b5a6bd ("bcma: populate
> bus DT subnodes as platform_device-s"):
>
> ERROR: "of_default_bus_match_table" [drivers/bcma/bcma.ko] undefined!
>
> Rafał Miłecki suggested as a quick fix to use IS_BUILTIN() to workaround the
> issue. The downside is that this won't work when BCMA is compiled as a module,
> but we can live with that for now just to unblock the breakage.
>
> Reported-by: Stephen Rothwell <[email protected]>
> Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
> Signed-off-by: Kalle Valo <[email protected]>
> ---
> drivers/bcma/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
> index 59128478a90f..6b7d54622058 100644
> --- a/drivers/bcma/main.c
> +++ b/drivers/bcma/main.c
> @@ -410,7 +410,7 @@ int bcma_bus_register(struct bcma_bus *bus)
> bcma_core_pci_early_init(&bus->drv_pci[0]);
> }
>
> - if (bus->host_pdev) {
> + if (IS_BUILTIN(CONFIG_BCMA) && bus->host_pdev) {
> struct device *dev = &bus->host_pdev->dev;
>
> of_platform_populate(dev->of_node, of_default_bus_match_table,

I think this isn't enough, AFAICT this needs IS_BUILTIN(CONFIG_BCMA)
&& IS_ENABLED(CONFIG_OF), else it will break with OF=n and BCMA=y, as
of_default_bus_match_table is only defined/built-in when OF=y.


Jonas