diff mbox

bcma: fix linking problem with of_default_bus_match_table

Message ID 1437633104-23373-1-git-send-email-kvalo@codeaurora.org (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show

Commit Message

Kalle Valo July 23, 2015, 6:31 a.m. UTC
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 <sfr@canb.auug.org.au>
Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/bcma/main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jonas Gorski July 23, 2015, 10:02 a.m. UTC | #1
Hi,

On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <kvalo@codeaurora.org> 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 <sfr@canb.auug.org.au>
> Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo July 23, 2015, 3:51 p.m. UTC | #2
Jonas Gorski <jogo@openwrt.org> writes:

> Hi,
>
> On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <kvalo@codeaurora.org> 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?
Jonas Gorski July 23, 2015, 4:04 p.m. UTC | #3
On 23.07.2015 17:51, Kalle Valo wrote:
> Jonas Gorski <jogo@openwrt.org> writes:
> 
>> Hi,
>>
>> On Thu, Jul 23, 2015 at 8:31 AM, Kalle Valo <kvalo@codeaurora.org> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo July 23, 2015, 5:15 p.m. UTC | #4
Jonas Gorski <jogo@openwrt.org> 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 July 26, 2015, 11:57 a.m. UTC | #5
Kalle Valo <kvalo@codeaurora.org> 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 <sfr@canb.auug.org.au>
> Fixes: cae761b5a6bd ("bcma: populate bus DT subnodes as platform_device-s")
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

I dropped this as Hauke sent a better patch.
diff mbox

Patch

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,