Re: [PATCH] ASoC: rt5640: ifdef for ACPI module table

[Olof Johansson <olof@lixom.net>]

On Wed, Sep 18, 2013 at 04:38:42PM +0100, Mark Brown wrote:
> On Wed, Sep 18, 2013 at 08:13:14AM -0700, Olof Johansson wrote:
> > On Wed, Sep 18, 2013 at 2:41 AM, Mark Brown <broonie@kernel.org> wrote:
> > > On Tue, Sep 17, 2013 at 11:10:24PM -0700, Olof Johansson wrote:
> 
> > > Please send things to the advertised maintainer addresses.
> 
> > I sent it to the person signing off on the patch that this fixes up,
> > you should probably use your maintainer address for that.
> 
> In that case you should've also CCed the maintainer address, you should
> always CC maintainers on patches.

Sure, will do. But you should also consider using your maintainer address
for your maintainer work. Linaro will surely be credited for it anyway
in the oh-so-important statistics. :-)

> > > Shouldn't the fix for this be to do the same as we're doing for the OF
> > > tables and have an acpi_match_ptr()?  It seems like we're doing the same
> > > thing so we should be handling it in a similar fashion.
> 
> > There's already ACPI_PTR(), which is exactly why this warning shows
> > up. of_match_ptr() and ACPI_PTR() will evaluate to NULL if
> > CONFIG_OF/ACPI aren't set, which is the case here -- which in turn is
> > why there's a warning that the match table is defined but never used.
> 
> > So, the solution is either to not define the table, i.e. ifdef, or to
> > not use the ACPI_PTR() macro. It makes sense to not advertise the
> > driver as capable of probing on ACPI if ACPI is not enabled in the
> > kernel config, which in turn means ifdef.
> 
> > Unless I'm missing something here, of course. :)
> 
> We're supposed to be able to do this sort of stuff without the ifdef
> noise and have the compiler eliminate unreferenced static symbols
> automatically, though I can't see how of_match_ptr() actually does this
> now.  I don't immedately seem to be able to persuade Kconfig to turn
> off OF to verify though we do have some examples which nobody has
> complained about...

The problem doesn't show up when you build the driver as a module, because by
then the MODULE_DEVICE_TABLE() reference means the table is referenced. But
when you don't build as a module, MODULE_GENERIC_TABLE() reduces down to
nothing, thus causing this.

Only way to avoid is as I see it is to mark the table __maybe_unused,
which in this case doesn't look like much of a better option.

Most other drivers, for example those using pci or of module tables, have
Kconfig dependencies such that there's no need to ifdef. There are plenty of
examples of drivers ifdeffing on CONFIG_OF for those tables though.


-Olof