[PATCH] iio: accel: fxls8962af: fix i2c dependency

[PATCH] iio: accel: fxls8962af: fix i2c dependency

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

With CONFIG_SPI=y and CONFIG_I2C=m, building fxls8962af into vmlinux
causes a link error against the I2C module:

aarch64-linux-ld: drivers/iio/accel/fxls8962af-core.o: in function `fxls8962af_fifo_flush':
fxls8962af-core.c:(.text+0x3a0): undefined reference to `i2c_verify_client'

Work around it by adding a Kconfig dependency that forces the SPI driver
to be a loadable module whenever I2C is a module.

Fixes: af959b7b96b8 ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I'm not overly happy with the fix either, but couldn't think of
a better idea. If someone provide a different fix, please ignore
mine.
---
 drivers/iio/accel/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 0e56ace61103..8d8b1ba42ff8 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -231,6 +231,7 @@ config DMARD10
 
 config FXLS8962AF
 	tristate
+	depends on I2C || !I2C # cannot be built-in for modular I2C
 
 config FXLS8962AF_I2C
 	tristate "NXP FXLS8962AF/FXLS8964AF Accelerometer I2C Driver"
@@ -247,6 +248,7 @@ config FXLS8962AF_I2C
 config FXLS8962AF_SPI
 	tristate "NXP FXLS8962AF/FXLS8964AF Accelerometer SPI Driver"
 	depends on SPI
+	depends on I2C || !I2C
 	select FXLS8962AF
 	select REGMAP_SPI
 	help
-- 
2.29.2

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Andy Shevchenko]

On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> With CONFIG_SPI=y and CONFIG_I2C=m, building fxls8962af into vmlinux
> causes a link error against the I2C module:
>
> aarch64-linux-ld: drivers/iio/accel/fxls8962af-core.o: in function `fxls8962af_fifo_flush':
> fxls8962af-core.c:(.text+0x3a0): undefined reference to `i2c_verify_client'
>
> Work around it by adding a Kconfig dependency that forces the SPI driver
> to be a loadable module whenever I2C is a module.

...

>  config FXLS8962AF
>         tristate
> +       depends on I2C || !I2C # cannot be built-in for modular I2C

Can you enlighten me how this will not be a no-op?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Arnd Bergmann]

On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > With CONFIG_SPI=y and CONFIG_I2C=m, building fxls8962af into vmlinux
> > causes a link error against the I2C module:
> >
> > aarch64-linux-ld: drivers/iio/accel/fxls8962af-core.o: in function `fxls8962af_fifo_flush':
> > fxls8962af-core.c:(.text+0x3a0): undefined reference to `i2c_verify_client'
> >
> > Work around it by adding a Kconfig dependency that forces the SPI driver
> > to be a loadable module whenever I2C is a module.
>
> ...
>
> >  config FXLS8962AF
> >         tristate
> > +       depends on I2C || !I2C # cannot be built-in for modular I2C
>
> Can you enlighten me how this will not be a no-op?

This part does nothing, it only causes a warning when FXLS8962AF
gets selected =y when I2C=m.

The important bit is the other hunk that adds the same dependency
to the FXLS8962AF_SPI symbol, which enforces that either I2C
is completely disabled, or treated as a dependency that prevents
the user from setting FXLS8962AF_SPI=y when that would cause
a link failure.

The effect is similar to a 'depends on SND_SOC_I2C_AND_SPI',
except we only need it on the SPI symbol here because the SPI
core cannot be in a module itself.

        Arnd

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Andy Shevchenko]

On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <arnd@kernel.org> wrote:

...

> > >  config FXLS8962AF
> > >         tristate
> > > +       depends on I2C || !I2C # cannot be built-in for modular I2C
> >
> > Can you enlighten me how this will not be a no-op?
>
> This part does nothing, it only causes a warning when FXLS8962AF
> gets selected =y when I2C=m.

This is something new to me. But shouldn't the other chunk guarantee
that warning won't happen?

> The important bit is the other hunk that adds the same dependency
> to the FXLS8962AF_SPI symbol, which enforces that either I2C
> is completely disabled, or treated as a dependency that prevents
> the user from setting FXLS8962AF_SPI=y when that would cause
> a link failure.

This part I understand and neither object to nor comment on.

> The effect is similar to a 'depends on SND_SOC_I2C_AND_SPI',
> except we only need it on the SPI symbol here because the SPI
> core cannot be in a module itself.

I see. Thanks for elaboration.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Arnd Bergmann]

On Wed, Jul 21, 2021 at 7:34 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> ...
>
> > > >  config FXLS8962AF
> > > >         tristate
> > > > +       depends on I2C || !I2C # cannot be built-in for modular I2C
> > >
> > > Can you enlighten me how this will not be a no-op?
> >
> > This part does nothing, it only causes a warning when FXLS8962AF
> > gets selected =y when I2C=m.
>
> This is something new to me. But shouldn't the other chunk guarantee
> that warning won't happen?

Correct, it works without that, but if that fails after something changes,
this version would provide better diagnostics than the FXLS8962AF
core driver causing a link failure, and I found it documents better
why the other driver needs the dependency.

Let me know if you prefer me to resend the patch without this hunk.

      Arnd

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Jonathan Cameron]

On Wed, 21 Jul 2021 20:40:30 +0200
Arnd Bergmann <arnd@kernel.org> wrote:

> On Wed, Jul 21, 2021 at 7:34 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <arnd@kernel.org> wrote:  
> > > On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:  
> > > > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <arnd@kernel.org> wrote:  
> >
> > ...
> >  
> > > > >  config FXLS8962AF
> > > > >         tristate
> > > > > +       depends on I2C || !I2C # cannot be built-in for modular I2C  
> > > >
> > > > Can you enlighten me how this will not be a no-op?  
> > >
> > > This part does nothing, it only causes a warning when FXLS8962AF
> > > gets selected =y when I2C=m.  
> >
> > This is something new to me. But shouldn't the other chunk guarantee
> > that warning won't happen?  
> 
> Correct, it works without that, but if that fails after something changes,
> this version would provide better diagnostics than the FXLS8962AF
> core driver causing a link failure, and I found it documents better
> why the other driver needs the dependency.
> 
> Let me know if you prefer me to resend the patch without this hunk.
> 
>       Arnd

Hi Arnd,

I didn't think of this particularly combination when we dealt with
last build issue the workaround brought in.  I've applied this to the
fixes-togreg branch of iio.git as an immediately solution, but longer
term we should think about just using a function pointer to allow us
to move this into the i2c specific module.  If we do that we can
drop this complex build logic later.

Thanks,

Jonathan

Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency

[Arnd Bergmann]

On Sat, Jul 24, 2021 at 5:15 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 21 Jul 2021 20:40:30 +0200 Arnd Bergmann <arnd@kernel.org> wrote:

> I didn't think of this particularly combination when we dealt with
> last build issue the workaround brought in.  I've applied this to the
> fixes-togreg branch of iio.git as an immediately solution, but longer
> term we should think about just using a function pointer to allow us
> to move this into the i2c specific module.  If we do that we can
> drop this complex build logic later.

Ok, that sounds good to me, thanks!

       Arnd