[PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[Stephen Boyd]

This reverts commit a06cd8cf97a3 ("ASoC: da7218: skip of_device_id table
when !CONFIG_OF") because we want to make of_match_device() stop using
of_match_ptr() internally, confusing compilers and causing ifdef
pollution.

Reported-by: kernel test robot <lkp@intel.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

Changes from v1:
 * Dropped of_match_ptr() in driver too

 sound/soc/codecs/da7218.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
index 2bfafbe9e3dc..83cace9d0139 100644
--- a/sound/soc/codecs/da7218.c
+++ b/sound/soc/codecs/da7218.c
@@ -2278,14 +2278,12 @@ static irqreturn_t da7218_irq_thread(int irq, void *data)
  * DT
  */
 
-#ifdef CONFIG_OF
 static const struct of_device_id da7218_of_match[] = {
 	{ .compatible = "dlg,da7217", .data = (void *) DA7217_DEV_ID },
 	{ .compatible = "dlg,da7218", .data = (void *) DA7218_DEV_ID },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7218_of_match);
-#endif
 
 static inline int da7218_of_get_id(struct device *dev)
 {
@@ -3311,7 +3309,7 @@ MODULE_DEVICE_TABLE(i2c, da7218_i2c_id);
 static struct i2c_driver da7218_i2c_driver = {
 	.driver = {
 		.name = "da7218",
-		.of_match_table = of_match_ptr(da7218_of_match),
+		.of_match_table = da7218_of_match,
 	},
 	.probe		= da7218_i2c_probe,
 	.id_table	= da7218_i2c_id,

base-commit: 19c329f6808995b142b3966301f217c831e7cf31
-- 
https://chromeos.dev

Re: [PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[Geert Uytterhoeven]

Hi Stephen,

On Tue, Feb 2, 2021 at 8:20 PM Stephen Boyd <swboyd@chromium.org> wrote:
> This reverts commit a06cd8cf97a3 ("ASoC: da7218: skip of_device_id table
> when !CONFIG_OF") because we want to make of_match_device() stop using
> of_match_ptr() internally, confusing compilers and causing ifdef
> pollution.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Thanks for your patch!

> ---
>
> Changes from v1:
>  * Dropped of_match_ptr() in driver too
>
>  sound/soc/codecs/da7218.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> index 2bfafbe9e3dc..83cace9d0139 100644
> --- a/sound/soc/codecs/da7218.c
> +++ b/sound/soc/codecs/da7218.c
> @@ -2278,14 +2278,12 @@ static irqreturn_t da7218_irq_thread(int irq, void *data)
>   * DT
>   */
>
> -#ifdef CONFIG_OF
>  static const struct of_device_id da7218_of_match[] = {
>         { .compatible = "dlg,da7217", .data = (void *) DA7217_DEV_ID },
>         { .compatible = "dlg,da7218", .data = (void *) DA7218_DEV_ID },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, da7218_of_match);
> -#endif
>
>  static inline int da7218_of_get_id(struct device *dev)
>  {
> @@ -3311,7 +3309,7 @@ MODULE_DEVICE_TABLE(i2c, da7218_i2c_id);
>  static struct i2c_driver da7218_i2c_driver = {
>         .driver = {
>                 .name = "da7218",
> -               .of_match_table = of_match_ptr(da7218_of_match),
> +               .of_match_table = da7218_of_match,

This does mean the compiler can no longer optimize the table away
in the CONFIG_OF=n case. Is that intentional?

>         },
>         .probe          = da7218_i2c_probe,
>         .id_table       = da7218_i2c_id,
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[Arnd Bergmann]

On Tue, Feb 2, 2021 at 9:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Feb 2, 2021 at 8:20 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > This reverts commit a06cd8cf97a3 ("ASoC: da7218: skip of_device_id table
> > when !CONFIG_OF") because we want to make of_match_device() stop using
> > of_match_ptr() internally, confusing compilers and causing ifdef
a> > pollution.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>
> Thanks for your patch!
>
> > ---
> >
> > Changes from v1:
> >  * Dropped of_match_ptr() in driver too
> >
> >  sound/soc/codecs/da7218.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> > index 2bfafbe9e3dc..83cace9d0139 100644
> > --- a/sound/soc/codecs/da7218.c
> > +++ b/sound/soc/codecs/da7218.c
> > @@ -2278,14 +2278,12 @@ static irqreturn_t da7218_irq_thread(int irq, void *data)
> >   * DT
> >   */
> >
> > -#ifdef CONFIG_OF
> >  static const struct of_device_id da7218_of_match[] = {
> >         { .compatible = "dlg,da7217", .data = (void *) DA7217_DEV_ID },
> >         { .compatible = "dlg,da7218", .data = (void *) DA7218_DEV_ID },
> >         { }
> >  };
> >  MODULE_DEVICE_TABLE(of, da7218_of_match);
> > -#endif
> >
> >  static inline int da7218_of_get_id(struct device *dev)
> >  {
> > @@ -3311,7 +3309,7 @@ MODULE_DEVICE_TABLE(i2c, da7218_i2c_id);
> >  static struct i2c_driver da7218_i2c_driver = {
> >         .driver = {
> >                 .name = "da7218",
> > -               .of_match_table = of_match_ptr(da7218_of_match),
> > +               .of_match_table = da7218_of_match,
>
> This does mean the compiler can no longer optimize the table away
> in the CONFIG_OF=n case. Is that intentional?

It already couldn't do that because of the other reference.

In general, I don't see much value in optimizing the match table
away regardless, since most drivers that have one do not work
without a devicetree anyway. It seems that this could in theory
work using a locally defined platform device, but I don't see any
such users in the mainline kernel.

       Arnd

Re: [PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[Geert Uytterhoeven]

Hi Arnd,

On Tue, Feb 2, 2021 at 9:50 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Tue, Feb 2, 2021 at 9:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Feb 2, 2021 at 8:20 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > This reverts commit a06cd8cf97a3 ("ASoC: da7218: skip of_device_id table
> > > when !CONFIG_OF") because we want to make of_match_device() stop using
> > > of_match_ptr() internally, confusing compilers and causing ifdef
> a> > pollution.
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > Cc: Mark Brown <broonie@kernel.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >
> > Thanks for your patch!
> >
> > > ---
> > >
> > > Changes from v1:
> > >  * Dropped of_match_ptr() in driver too
> > >
> > >  sound/soc/codecs/da7218.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> > > index 2bfafbe9e3dc..83cace9d0139 100644
> > > --- a/sound/soc/codecs/da7218.c
> > > +++ b/sound/soc/codecs/da7218.c
> > > @@ -2278,14 +2278,12 @@ static irqreturn_t da7218_irq_thread(int irq, void *data)
> > >   * DT
> > >   */
> > >
> > > -#ifdef CONFIG_OF
> > >  static const struct of_device_id da7218_of_match[] = {
> > >         { .compatible = "dlg,da7217", .data = (void *) DA7217_DEV_ID },
> > >         { .compatible = "dlg,da7218", .data = (void *) DA7218_DEV_ID },
> > >         { }
> > >  };
> > >  MODULE_DEVICE_TABLE(of, da7218_of_match);
> > > -#endif
> > >
> > >  static inline int da7218_of_get_id(struct device *dev)
> > >  {
> > > @@ -3311,7 +3309,7 @@ MODULE_DEVICE_TABLE(i2c, da7218_i2c_id);
> > >  static struct i2c_driver da7218_i2c_driver = {
> > >         .driver = {
> > >                 .name = "da7218",
> > > -               .of_match_table = of_match_ptr(da7218_of_match),
> > > +               .of_match_table = da7218_of_match,
> >
> > This does mean the compiler can no longer optimize the table away
> > in the CONFIG_OF=n case. Is that intentional?
>
> It already couldn't do that because of the other reference.

But that will be fixed by the conversion to of_device_get_match_data(),
right?

> In general, I don't see much value in optimizing the match table
> away regardless, since most drivers that have one do not work
> without a devicetree anyway. It seems that this could in theory
> work using a locally defined platform device, but I don't see any
> such users in the mainline kernel.

For platform devices, limited to one SoC or SoC family, I agree.

Given this is an i2c driver with a MODULE_DEVICE_TABLE listing multiple
entries, I wouldn't be surprised if there are out-of-tree users, using
i2c_register_board_info() or echo > /sys/.../new_device.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef

[Mark Brown]

On Tue, 2 Feb 2021 11:20:16 -0800, Stephen Boyd wrote:
> This reverts commit a06cd8cf97a3 ("ASoC: da7218: skip of_device_id table
> when !CONFIG_OF") because we want to make of_match_device() stop using
> of_match_ptr() internally, confusing compilers and causing ifdef
> pollution.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: da7218: Drop CONFIG_OF ifdef
      commit: 0dd4d3e8c3a8ab63e9368e01d7839afad7b804c7

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark