[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdVP9tEpQpGNe9pupeDHinm_vzRsc7HAfk-d5wm8bXk3Gg@mail.gmail.com>
Date: Wed, 3 Feb 2021 08:45:22 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Stephen Boyd <swboyd@...omium.org>, Rob Herring <robh@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, kernel test robot <lkp@...el.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Arnd Bergmann <arnd@...db.de>, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v2] ASoC: da7218: Drop CONFIG_OF ifdef
Hi Arnd,
On Tue, Feb 2, 2021 at 9:50 PM Arnd Bergmann <arnd@...nel.org> wrote:
> On Tue, Feb 2, 2021 at 9:06 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > On Tue, Feb 2, 2021 at 8:20 PM Stephen Boyd <swboyd@...omium.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@...el.com>
> > > Cc: Geert Uytterhoeven <geert+renesas@...der.be>
> > > Acked-by: Arnd Bergmann <arnd@...db.de>
> > > Cc: Mark Brown <broonie@...nel.org>
> > > Signed-off-by: Stephen Boyd <swboyd@...omium.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@...ux-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
Powered by blists - more mailing lists