linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: accel: fxls8962af: fix i2c dependency
@ 2021-07-21 15:13 Arnd Bergmann
  2021-07-21 15:50 ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2021-07-21 15:13 UTC (permalink / raw)
  To: Jonathan Cameron, Sean Nyekjaer
  Cc: Arnd Bergmann, Lars-Peter Clausen, Andy Shevchenko,
	Linus Walleij, Stephan Gerhold, Hans de Goede, Tomas Melin,
	Mike Looijmans, Alexandru Ardelean, linux-iio, linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-21 15:13 [PATCH] iio: accel: fxls8962af: fix i2c dependency Arnd Bergmann
@ 2021-07-21 15:50 ` Andy Shevchenko
  2021-07-21 16:12   ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-07-21 15:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jonathan Cameron, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-21 15:50 ` Andy Shevchenko
@ 2021-07-21 16:12   ` Arnd Bergmann
  2021-07-21 17:33     ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2021-07-21 16:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-21 16:12   ` Arnd Bergmann
@ 2021-07-21 17:33     ` Andy Shevchenko
  2021-07-21 18:40       ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-07-21 17:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jonathan Cameron, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-21 17:33     ` Andy Shevchenko
@ 2021-07-21 18:40       ` Arnd Bergmann
  2021-07-24 15:16         ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2021-07-21 18:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-21 18:40       ` Arnd Bergmann
@ 2021-07-24 15:16         ` Jonathan Cameron
  2021-07-24 17:00           ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2021-07-24 15:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andy Shevchenko, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, Linux Kernel Mailing List

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iio: accel: fxls8962af: fix i2c dependency
  2021-07-24 15:16         ` Jonathan Cameron
@ 2021-07-24 17:00           ` Arnd Bergmann
  0 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2021-07-24 17:00 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Andy Shevchenko, Sean Nyekjaer, Arnd Bergmann,
	Lars-Peter Clausen, Linus Walleij, Stephan Gerhold,
	Hans de Goede, Tomas Melin, Mike Looijmans, Alexandru Ardelean,
	linux-iio, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-24 17:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 15:13 [PATCH] iio: accel: fxls8962af: fix i2c dependency Arnd Bergmann
2021-07-21 15:50 ` Andy Shevchenko
2021-07-21 16:12   ` Arnd Bergmann
2021-07-21 17:33     ` Andy Shevchenko
2021-07-21 18:40       ` Arnd Bergmann
2021-07-24 15:16         ` Jonathan Cameron
2021-07-24 17:00           ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).