[PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

The bd79124 has ADC inputs which can be muxed to be GPIOs. The driver
supports this by registering a GPIO-chip for channels which aren't used
as ADC.

The Kconfig entry does not handle the dependency to GPIOLIB, which
causes errors:

ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/iio/adc/rohm-bd79124.ko] undefined!
ERROR: modpost: "gpiochip_get_data" [drivers/iio/adc/rohm-bd79124.ko] undefined!

at linking phase if GPIOLIB is not configured to be used.

Fix this by adding dependency to the GPIOLIB.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202508131533.5sSkq80B-lkp@intel.com/
Fixes: 3f57a3b9ab74 ("iio: adc: Support ROHM BD79124 ADC")
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---

I am somewhat curious why the failure occurs only at the linking phase?
Wouldn't it either be better to have these functions
devm_gpiochip_add_data_with_key() and gpiochip_get_data() only declared
when the CONFIG_GPIOLIB is y/m, to get errors already during
compilation, or provide stubs?
---
 drivers/iio/adc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 6de2abad0197..24f2572c487e 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1300,7 +1300,7 @@ config RN5T618_ADC
 
 config ROHM_BD79124
 	tristate "Rohm BD79124 ADC driver"
-	depends on I2C
+	depends on I2C && GPIOLIB
 	select REGMAP_I2C
 	select IIO_ADC_HELPER
 	help
-- 
2.50.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Bartosz Golaszewski]

On Wed, 13 Aug 2025 11:16:06 +0200, Matti Vaittinen
<mazziesaccount@gmail.com> said:
> The bd79124 has ADC inputs which can be muxed to be GPIOs. The driver
> supports this by registering a GPIO-chip for channels which aren't used
> as ADC.
>
> The Kconfig entry does not handle the dependency to GPIOLIB, which
> causes errors:
>
> ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/iio/adc/rohm-bd79124.ko] undefined!
> ERROR: modpost: "gpiochip_get_data" [drivers/iio/adc/rohm-bd79124.ko] undefined!
>
> at linking phase if GPIOLIB is not configured to be used.
>
> Fix this by adding dependency to the GPIOLIB.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202508131533.5sSkq80B-lkp@intel.com/
> Fixes: 3f57a3b9ab74 ("iio: adc: Support ROHM BD79124 ADC")
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

> ---
>
> I am somewhat curious why the failure occurs only at the linking phase?
> Wouldn't it either be better to have these functions
> devm_gpiochip_add_data_with_key() and gpiochip_get_data() only declared
> when the CONFIG_GPIOLIB is y/m, to get errors already during
> compilation, or provide stubs?

Providing stubs is not correct for sure - a GPIO provider must always pull
in the relevant infrastructure over Kconfig. As for the former: it seems it's
a common pattern for the headers containing the "provider" part of the
subystem API, you'd get the same issue with regulators or pinctrl.

I don't have a good answer, I'd just apply this as it's not a common issue
from what I can tell.

Bartosz

> ---
>  drivers/iio/adc/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 6de2abad0197..24f2572c487e 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1300,7 +1300,7 @@ config RN5T618_ADC
>
>  config ROHM_BD79124
>  	tristate "Rohm BD79124 ADC driver"
> -	depends on I2C
> +	depends on I2C && GPIOLIB
>  	select REGMAP_I2C
>  	select IIO_ADC_HELPER
>  	help
> --
> 2.50.1
>
>

Re: [PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Andy Shevchenko]

On Wed, Aug 13, 2025 at 11:40 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Wed, 13 Aug 2025 11:16:06 +0200, Matti Vaittinen
> <mazziesaccount@gmail.com> said:
> > The bd79124 has ADC inputs which can be muxed to be GPIOs. The driver
> > supports this by registering a GPIO-chip for channels which aren't used
> > as ADC.
> >
> > The Kconfig entry does not handle the dependency to GPIOLIB, which
> > causes errors:
> >
> > ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/iio/adc/rohm-bd79124.ko] undefined!
> > ERROR: modpost: "gpiochip_get_data" [drivers/iio/adc/rohm-bd79124.ko] undefined!
> >
> > at linking phase if GPIOLIB is not configured to be used.
> >
> > Fix this by adding dependency to the GPIOLIB.

...

> > I am somewhat curious why the failure occurs only at the linking phase?
> > Wouldn't it either be better to have these functions
> > devm_gpiochip_add_data_with_key() and gpiochip_get_data() only declared
> > when the CONFIG_GPIOLIB is y/m, to get errors already during
> > compilation, or provide stubs?
>
> Providing stubs is not correct for sure - a GPIO provider must always pull
> in the relevant infrastructure over Kconfig.

I disagree with this statement. The (provided) resource can be
optional and hence the stubs are a way to go.

> As for the former: it seems it's
> a common pattern for the headers containing the "provider" part of the
> subystem API, you'd get the same issue with regulators or pinctrl.
>
> I don't have a good answer, I'd just apply this as it's not a common issue
> from what I can tell.

If the GPIO functionality is optional (not the main one), the user
should be able to compile it conditionally, in such a case it's either
an ifdeffery in the code, or separate module with its own stubs.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Bartosz Golaszewski]

On Wed, Aug 13, 2025 at 12:07 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Aug 13, 2025 at 11:40 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > On Wed, 13 Aug 2025 11:16:06 +0200, Matti Vaittinen
> > <mazziesaccount@gmail.com> said:
> > > The bd79124 has ADC inputs which can be muxed to be GPIOs. The driver
> > > supports this by registering a GPIO-chip for channels which aren't used
> > > as ADC.
> > >
> > > The Kconfig entry does not handle the dependency to GPIOLIB, which
> > > causes errors:
> > >
> > > ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/iio/adc/rohm-bd79124.ko] undefined!
> > > ERROR: modpost: "gpiochip_get_data" [drivers/iio/adc/rohm-bd79124.ko] undefined!
> > >
> > > at linking phase if GPIOLIB is not configured to be used.
> > >
> > > Fix this by adding dependency to the GPIOLIB.
>
> ...
>
> > > I am somewhat curious why the failure occurs only at the linking phase?
> > > Wouldn't it either be better to have these functions
> > > devm_gpiochip_add_data_with_key() and gpiochip_get_data() only declared
> > > when the CONFIG_GPIOLIB is y/m, to get errors already during
> > > compilation, or provide stubs?
> >
> > Providing stubs is not correct for sure - a GPIO provider must always pull
> > in the relevant infrastructure over Kconfig.
>
> I disagree with this statement. The (provided) resource can be
> optional and hence the stubs are a way to go.
>
> > As for the former: it seems it's
> > a common pattern for the headers containing the "provider" part of the
> > subystem API, you'd get the same issue with regulators or pinctrl.
> >
> > I don't have a good answer, I'd just apply this as it's not a common issue
> > from what I can tell.
>
> If the GPIO functionality is optional (not the main one), the user
> should be able to compile it conditionally, in such a case it's either
> an ifdeffery in the code, or separate module with its own stubs.
>

Honestly, it makes much more sense to factor out that optional
functionality into its own compilation unit that can be left out
completely for !CONFIG_GPIOLIB with a single internal registration
function being stubbed within the driver.

Bartosz

Re: [PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Andy Shevchenko]

On Wed, Aug 13, 2025 at 02:17:44PM +0200, Bartosz Golaszewski wrote:
> On Wed, Aug 13, 2025 at 12:07 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Aug 13, 2025 at 11:40 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > On Wed, 13 Aug 2025 11:16:06 +0200, Matti Vaittinen
> > > <mazziesaccount@gmail.com> said:

...

> > > As for the former: it seems it's
> > > a common pattern for the headers containing the "provider" part of the
> > > subystem API, you'd get the same issue with regulators or pinctrl.
> > >
> > > I don't have a good answer, I'd just apply this as it's not a common issue
> > > from what I can tell.
> >
> > If the GPIO functionality is optional (not the main one), the user
> > should be able to compile it conditionally, in such a case it's either
> > an ifdeffery in the code, or separate module with its own stubs.
> 
> Honestly, it makes much more sense to factor out that optional
> functionality into its own compilation unit that can be left out
> completely for !CONFIG_GPIOLIB with a single internal registration
> function being stubbed within the driver.

That's what I suggested under "separate module with its own stubs" above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: bd79124: Add GPIOLIB dependency

[Jonathan Cameron]

On Wed, 13 Aug 2025 15:27:58 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Aug 13, 2025 at 02:17:44PM +0200, Bartosz Golaszewski wrote:
> > On Wed, Aug 13, 2025 at 12:07 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:  
> > > On Wed, Aug 13, 2025 at 11:40 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:  
> > > > On Wed, 13 Aug 2025 11:16:06 +0200, Matti Vaittinen
> > > > <mazziesaccount@gmail.com> said:  
> 
> ...
> 
> > > > As for the former: it seems it's
> > > > a common pattern for the headers containing the "provider" part of the
> > > > subystem API, you'd get the same issue with regulators or pinctrl.
> > > >
> > > > I don't have a good answer, I'd just apply this as it's not a common issue
> > > > from what I can tell.  
> > >
> > > If the GPIO functionality is optional (not the main one), the user
> > > should be able to compile it conditionally, in such a case it's either
> > > an ifdeffery in the code, or separate module with its own stubs.  
> > 
> > Honestly, it makes much more sense to factor out that optional
> > functionality into its own compilation unit that can be left out
> > completely for !CONFIG_GPIOLIB with a single internal registration
> > function being stubbed within the driver.  
> 
> That's what I suggested under "separate module with its own stubs" above.
> 

I agree that's the long term ideal. For now I'm going to queue this fix
up and mark it for stable.  We can tidy up and make it optional again
as a follow up.

Jonathan