When CONFIG_OF is disabled, gcc-9 produces a warning about the
wfx_sdio_of_match[] array having a declaration without a dimension:
drivers/staging/wfx/bus_sdio.c:159:34: error: array 'wfx_sdio_of_match' assumed to have one element [-Werror]
159 | static const struct of_device_id wfx_sdio_of_match[];
| ^~~~~~~~~~~~~~~~~
Move the proper declaration up and out of the #ifdef instead.
Fixes: a7a91ca5a23d ("staging: wfx: add infrastructure for new driver")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/staging/wfx/bus_sdio.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index dedc3ff58d3e..c2e4bd1e3b0a 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -156,7 +156,13 @@ static const struct hwbus_ops wfx_sdio_hwbus_ops = {
.align_size = wfx_sdio_align_size,
};
-static const struct of_device_id wfx_sdio_of_match[];
+static const struct of_device_id wfx_sdio_of_match[] = {
+ { .compatible = "silabs,wfx-sdio" },
+ { .compatible = "silabs,wf200" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
+
static int wfx_sdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
{
@@ -248,15 +254,6 @@ static const struct sdio_device_id wfx_sdio_ids[] = {
};
MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids);
-#ifdef CONFIG_OF
-static const struct of_device_id wfx_sdio_of_match[] = {
- { .compatible = "silabs,wfx-sdio" },
- { .compatible = "silabs,wf200" },
- { },
-};
-MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
-#endif
-
struct sdio_driver wfx_sdio_driver = {
.name = "wfx-sdio",
.id_table = wfx_sdio_ids,
@@ -264,6 +261,6 @@ struct sdio_driver wfx_sdio_driver = {
.remove = wfx_sdio_remove,
.drv = {
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(wfx_sdio_of_match),
+ .of_match_table = wfx_sdio_of_match,
}
};
--
2.26.0
On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote:
>
> When CONFIG_OF is disabled, gcc-9 produces a warning about the
> wfx_sdio_of_match[] array having a declaration without a dimension:
>
> drivers/staging/wfx/bus_sdio.c:159:34: error: array 'wfx_sdio_of_match' assumed to have one element [-Werror]
> 159 | static const struct of_device_id wfx_sdio_of_match[];
> | ^~~~~~~~~~~~~~~~~
>
> Move the proper declaration up and out of the #ifdef instead.
>
> Fixes: a7a91ca5a23d ("staging: wfx: add infrastructure for new driver")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/staging/wfx/bus_sdio.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
> index dedc3ff58d3e..c2e4bd1e3b0a 100644
> --- a/drivers/staging/wfx/bus_sdio.c
> +++ b/drivers/staging/wfx/bus_sdio.c
> @@ -156,7 +156,13 @@ static const struct hwbus_ops wfx_sdio_hwbus_ops = {
> .align_size = wfx_sdio_align_size,
> };
>
> -static const struct of_device_id wfx_sdio_of_match[];
> +static const struct of_device_id wfx_sdio_of_match[] = {
> + { .compatible = "silabs,wfx-sdio" },
> + { .compatible = "silabs,wf200" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
I suggest to keep the '#ifdef CONFIG_OF' around this definition. If
CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL
and it should compile.
> +
> static int wfx_sdio_probe(struct sdio_func *func,
> const struct sdio_device_id *id)
> {
> @@ -248,15 +254,6 @@ static const struct sdio_device_id wfx_sdio_ids[] = {
> };
> MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids);
>
> -#ifdef CONFIG_OF
> -static const struct of_device_id wfx_sdio_of_match[] = {
> - { .compatible = "silabs,wfx-sdio" },
> - { .compatible = "silabs,wf200" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
> -#endif
> -
> struct sdio_driver wfx_sdio_driver = {
> .name = "wfx-sdio",
> .id_table = wfx_sdio_ids,
> @@ -264,6 +261,6 @@ struct sdio_driver wfx_sdio_driver = {
> .remove = wfx_sdio_remove,
> .drv = {
> .owner = THIS_MODULE,
> - .of_match_table = of_match_ptr(wfx_sdio_of_match),
> + .of_match_table = wfx_sdio_of_match,
... and obviously keep of_match_ptr() here.
> }
> };
--
J?r?me Pouiller
On Wed, Apr 29, 2020 at 6:04 PM Jerome Pouiller
<[email protected]> wrote:
> On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote:
> >
> > -static const struct of_device_id wfx_sdio_of_match[];
> > +static const struct of_device_id wfx_sdio_of_match[] = {
> > + { .compatible = "silabs,wfx-sdio" },
> > + { .compatible = "silabs,wf200" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
>
> I suggest to keep the '#ifdef CONFIG_OF' around this definition. If
> CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL
> and it should compile.
I would generally always go for fewer #ifdef instead of more when the result
is the same. Are you worried about wasting 600 bytes of object code size for
the array on systems that need this driver but not CONFIG_OF, or something
else?
Arnd
On Wednesday 29 April 2020 22:34:56 CEST Arnd Bergmann wrote:
>
> On Wed, Apr 29, 2020 at 6:04 PM Jerome Pouiller
> <[email protected]> wrote:
> > On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote:
> > >
> > > -static const struct of_device_id wfx_sdio_of_match[];
> > > +static const struct of_device_id wfx_sdio_of_match[] = {
> > > + { .compatible = "silabs,wfx-sdio" },
> > > + { .compatible = "silabs,wf200" },
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
> >
> > I suggest to keep the '#ifdef CONFIG_OF' around this definition. If
> > CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL
> > and it should compile.
>
> I would generally always go for fewer #ifdef instead of more when the result
> is the same. Are you worried about wasting 600 bytes of object code size for
> the array on systems that need this driver but not CONFIG_OF, or something
> else?
I am not very concerned about the size of the object. However, I think
that all the modules should apply the same policy regarding the device
tables. With a few greps, I found 3954 struct of_device_id. About 500 are
inside #ifdef and about 1000 use of_match_ptr().
Should we consider that the structs of_device_id have to be defined even
if CONFIG_OF is not defined? And In this case, should we drop
of_match_ptr()?
Or in contrary, when kernel is compiled without CONFIG_OF, no modules
should contains OF entries in its device table?
--
J?r?me Pouiller
On Thu, Apr 30, 2020 at 10:42 AM Jerome Pouiller
<[email protected]> wrote:
> On Wednesday 29 April 2020 22:34:56 CEST Arnd Bergmann wrote:
> > On Wed, Apr 29, 2020 at 6:04 PM Jerome Pouiller
> > <[email protected]> wrote:
> > > On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote:
> > > >
> > > > -static const struct of_device_id wfx_sdio_of_match[];
> > > > +static const struct of_device_id wfx_sdio_of_match[] = {
> > > > + { .compatible = "silabs,wfx-sdio" },
> > > > + { .compatible = "silabs,wf200" },
> > > > + { },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
> > >
> > > I suggest to keep the '#ifdef CONFIG_OF' around this definition. If
> > > CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL
> > > and it should compile.
> >
> > I would generally always go for fewer #ifdef instead of more when the result
> > is the same. Are you worried about wasting 600 bytes of object code size for
> > the array on systems that need this driver but not CONFIG_OF, or something
> > else?
>
> I am not very concerned about the size of the object. However, I think
> that all the modules should apply the same policy regarding the device
> tables. With a few greps, I found 3954 struct of_device_id. About 500 are
> inside #ifdef and about 1000 use of_match_ptr().
>
> Should we consider that the structs of_device_id have to be defined even
> if CONFIG_OF is not defined? And In this case, should we drop
> of_match_ptr()?
>
> Or in contrary, when kernel is compiled without CONFIG_OF, no modules
> should contains OF entries in its device table?
I think the drivers that use an #ifdef here just do so for historic reasons.
In the linux-2.6 days, this caused build failures, but just leaving them
defined has worked for a long time.
Arnd