From: Arnd Bergmann <[email protected]>
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 <[email protected]>
---
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
On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> 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
On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <[email protected]> wrote:
> >
> > From: Arnd Bergmann <[email protected]>
> >
> > 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
On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <[email protected]> wrote:
> On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <[email protected]> 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
On Wed, Jul 21, 2021 at 7:34 PM Andy Shevchenko
<[email protected]> wrote:
> On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <[email protected]> wrote:
> > On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> > <[email protected]> wrote:
> > > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <[email protected]> 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
On Wed, 21 Jul 2021 20:40:30 +0200
Arnd Bergmann <[email protected]> wrote:
> On Wed, Jul 21, 2021 at 7:34 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Wed, Jul 21, 2021 at 7:12 PM Arnd Bergmann <[email protected]> wrote:
> > > On Wed, Jul 21, 2021 at 5:52 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > > On Wed, Jul 21, 2021 at 6:13 PM Arnd Bergmann <[email protected]> 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
On Sat, Jul 24, 2021 at 5:15 PM Jonathan Cameron <[email protected]> wrote:
> On Wed, 21 Jul 2021 20:40:30 +0200 Arnd Bergmann <[email protected]> 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