2014-02-03 08:47:32

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set

Hi Shimoda-san,

On Mon, 3 Feb 2014, Shimoda, Yoshihiro wrote:
> This patch fixes an issue that the following build error happens when
> the CONFIG_OF is not set:
>
> drivers/spi/spi-rspi.c: In function 'rspi_probe':
> drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
>
> Signed-off-by: Yoshihiro Shimoda <[email protected]>
> ---
> This patch is based on the latest origin/topic/rspi branch in the spi.git.
>
> drivers/spi/spi-rspi.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
> index 34ad4bc..e5cfc3d 100644
> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -1164,6 +1164,7 @@ static int rspi_parse_dt(struct device *dev, struct spi_master *master)
> return 0;
> }
> #else
> +#define rspi_of_match NULL
> static inline int rspi_parse_dt(struct device *dev, struct spi_master *master)
> {
> return -EINVAL;
> --
> 1.7.1

Thanks, obviously I missed that of_match_device() still uses the ID table
parameter if CONFIG_OF=n :-(

Below I have two alternative solutions:
1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
the platform_driver structure,
2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
inside of_match_device().

If 2 is accepted, drivers don't have to care about this anymore.

What do you think?

>From 060b8577e95441ee3b29e966a9fdd19b2a870bdf Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <[email protected]>
Date: Mon, 3 Feb 2014 09:22:06 +0100
Subject: [PATCH 1/2] spi: rspi: fix build error when CONFIG_OF is not set

If CONFIG_OF=n:

drivers/spi/spi-rspi.c: In function 'rspi_probe':
drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
drivers/spi/spi-rspi.c:1203:26: note: each undeclared identifier is reported only once for each function it appears in

Use of_match_ptr() to fix this.

Reported-by: Yoshihiro Shimoda <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/spi/spi-rspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 1c3aed63a2e0..df637184f6f0 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -1200,7 +1200,7 @@ static int rspi_probe(struct platform_device *pdev)
return -ENOMEM;
}

- of_id = of_match_device(rspi_of_match, &pdev->dev);
+ of_id = of_match_device(of_match_ptr(rspi_of_match), &pdev->dev);
if (of_id) {
ops = of_id->data;
ret = rspi_parse_dt(&pdev->dev, master);
--
1.7.9.5


>From 477ab825d43524959d68c3974e6e9536bd83bcde Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <[email protected]>
Date: Mon, 3 Feb 2014 09:27:40 +0100
Subject: [PATCH 2/2] [RFC] of/device: Nullify match table in
of_match_device() for CONFIG_OF=n

If the of_device_id table inside a device driver is protected by #ifdef
CONFIG_OF, the driver still has to provide a dummy declaration of the
table, or wrap it inside of_match_ptr(), when calling of_match_device()
in the CONFIG_OF=n case, else the driver fails to compile with e.g.

drivers/spi/spi-rspi.c: In function 'rspi_probe':
drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
drivers/spi/spi-rspi.c:1203:26: note: each undeclared identifier is reported only once for each function it appears in

Make of_match_device() nullify the table pointer if CONFIG_OF=n to fix
this.

Reported-by: Yoshihiro Shimoda <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
include/linux/of_device.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 82ce324fdce7..dac8bd2890ca 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -72,11 +72,13 @@ static inline int of_device_uevent_modalias(struct device *dev,

static inline void of_device_node_put(struct device *dev) { }

-static inline const struct of_device_id *of_match_device(
+static inline const struct of_device_id *__of_match_device(
const struct of_device_id *matches, const struct device *dev)
{
return NULL;
}
+#define of_match_device(matches, dev) \
+ __of_match_device(of_match_ptr(matches), (dev))

static inline struct device_node *of_cpu_device_node_get(int cpu)
{
--
1.7.9.5

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2014-02-03 15:17:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set

On Mon, Feb 03, 2014 at 09:47:25AM +0100, Geert Uytterhoeven wrote:

> Thanks, obviously I missed that of_match_device() still uses the ID table
> parameter if CONFIG_OF=n :-(

> Below I have two alternative solutions:
> 1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
> the platform_driver structure,
> 2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
> inside of_match_device().

> If 2 is accepted, drivers don't have to care about this anymore.

I think if we can fix it at the subsystem level that's clearly nicer
since it means nothing else will run into the same issue. Your patch
looks OK to me, I'd suggest submitting it.


Attachments:
(No filename) (696.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-03 21:45:19

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set

On Mon, Feb 3, 2014 at 2:47 AM, Geert Uytterhoeven <[email protected]> wrote:
> Hi Shimoda-san,
>
> On Mon, 3 Feb 2014, Shimoda, Yoshihiro wrote:
>> This patch fixes an issue that the following build error happens when
>> the CONFIG_OF is not set:
>>
>> drivers/spi/spi-rspi.c: In function 'rspi_probe':
>> drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
>>
>> Signed-off-by: Yoshihiro Shimoda <[email protected]>
>> ---
>> This patch is based on the latest origin/topic/rspi branch in the spi.git.
>>
>> drivers/spi/spi-rspi.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
>> index 34ad4bc..e5cfc3d 100644
>> --- a/drivers/spi/spi-rspi.c
>> +++ b/drivers/spi/spi-rspi.c
>> @@ -1164,6 +1164,7 @@ static int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> return 0;
>> }
>> #else
>> +#define rspi_of_match NULL
>> static inline int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> {
>> return -EINVAL;
>> --
>> 1.7.1
>
> Thanks, obviously I missed that of_match_device() still uses the ID table
> parameter if CONFIG_OF=n :-(
>
> Below I have two alternative solutions:
> 1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
> the platform_driver structure,
> 2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
> inside of_match_device().
>
> If 2 is accepted, drivers don't have to care about this anymore.
>
> What do you think?

I'll apply the 2nd one.

Rob

2014-02-04 06:02:11

by Yoshihiro Shimoda

[permalink] [raw]
Subject: Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set

Hi Geert-san,

(2014/02/03 17:47), Geert Uytterhoeven wrote:
> Hi Shimoda-san,
>
> On Mon, 3 Feb 2014, Shimoda, Yoshihiro wrote:
>> This patch fixes an issue that the following build error happens when
>> the CONFIG_OF is not set:
>>
>> drivers/spi/spi-rspi.c: In function 'rspi_probe':
>> drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
>>
>> Signed-off-by: Yoshihiro Shimoda <[email protected]>
>> ---
>> This patch is based on the latest origin/topic/rspi branch in the spi.git.
>>
>> drivers/spi/spi-rspi.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
>> index 34ad4bc..e5cfc3d 100644
>> --- a/drivers/spi/spi-rspi.c
>> +++ b/drivers/spi/spi-rspi.c
>> @@ -1164,6 +1164,7 @@ static int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> return 0;
>> }
>> #else
>> +#define rspi_of_match NULL
>> static inline int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> {
>> return -EINVAL;
>> --
>> 1.7.1
>
> Thanks, obviously I missed that of_match_device() still uses the ID table
> parameter if CONFIG_OF=n :-(
>
> Below I have two alternative solutions:
> 1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
> the platform_driver structure,
> 2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
> inside of_match_device().
>
> If 2 is accepted, drivers don't have to care about this anymore.
>
> What do you think?

Thank you for the reply.
I think the 2nd one is a nice idea.
If I applied it without my patch, the build error disappeared.

Best regards,
Yoshihiro Shimoda