Re: [PATCH] iio: adc: ti-ads112c14: add gpio support

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 25 Sep 2026 16:50:47 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add support for using the AIN4/GPIO0 to AIN7/GPIO3 pins as GPIOs when
> the gpio-controller property is present.
> 
> Pins that are already used for something else according to the
> devicetree are excluded from the valid GPIO mask. This includes analog
> inputs and excitation current outputs used by channels, REFP/REFN when
> an external reference is used, the /FAULT and /DRDY interrupts and the
> external clock input.
> 
> The per-pin register field macros are replaced with parameterized ones
> so that they can be used with the GPIO offset.

Can we pull that out as a trivial precursor? Feels like it will
reduce the noise for the more interesting new stuff.

Otherwise, only thing is a suggestion on how to perhaps
couple using the pins with reserving them in the parse function.

Thanks,

Jonathan


> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> ---
>  drivers/iio/adc/ti-ads112c14.c | 198 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 180 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> index 3c877126b0be..1c0519716faa 100644
> --- a/drivers/iio/adc/ti-ads112c14.c
> +++ b/drivers/iio/adc/ti-ads112c14.c

>  #define   ADS112C14_DIGITAL_CFG_CODING			BIT(1)
>  
>  #define ADS112C14_REG_GPIO_CFG				0x0B
> -#define   ADS112C14_GPIO_CFG_GPIO3_CFG			GENMASK(7, 6)
> -#define   ADS112C14_GPIO_CFG_GPIO2_CFG			GENMASK(5, 4)
> -#define   ADS112C14_GPIO_CFG_GPIO1_CFG			GENMASK(3, 2)
> -#define   ADS112C14_GPIO_CFG_GPIO0_CFG			GENMASK(1, 0)
> +#define   ADS112C14_GPIO_CFG_GPIO_CFG(n)		(GENMASK(1, 0) << (2 * (n)))

Trivial but I'd have slightly preferred to have seen these
macros all done as a precursor patch.

>  #define     ADS112C14_GPIO_CFG_GPIO_CFG_DISABLED	  0
>  #define     ADS112C14_GPIO_CFG_GPIO_CFG_INPUT		  1
>  #define     ADS112C14_GPIO_CFG_GPIO_CFG_OUTPUT_PUSH_PULL  2
> @@ -147,10 +142,7 @@

> @@ -393,6 +390,135 @@ struct ads112c14_data {
>  						 ARRAY_SIZE(ads112c14_sys_mon_channels));
>  };
>  
> +static void ads112c14_reserve_gpio_for_ain(unsigned long *gpio_reserved_mask,
> +					   u32 ain)
> +{

See below - but maybe this should return ain.

> +	if (ain >= 4 && ain <= 7)
> +		set_bit(ain - 4, gpio_reserved_mask);
> +}


>  static irqreturn_t ads112c14_drdy_irq_handler(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> @@ -1913,7 +2039,8 @@ static int ads112c14_populate_idac_mag(u32 current_nA, u8 *idac_mag)
>  }
>  
>  static int ads112c14_parse_channels(struct iio_dev *indio_dev,
> -				    bool *need_avdd_ref, bool *need_ext_ref)
> +				    bool *need_avdd_ref, bool *need_ext_ref,
> +				    unsigned long *gpio_reserved_mask)
>  {
>  	struct ads112c14_data *data = iio_priv(indio_dev);
>  	struct device *dev = indio_dev->dev.parent;
> @@ -1981,6 +2108,8 @@ static int ads112c14_parse_channels(struct iio_dev *indio_dev,
>  			 * for single-ended channels when taking measurements.
>  			 */
>  			spec->channel2 = ADS112C14_MUX_CFG_AIN_GND;
> +
> +			ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[0]);
>  		} else if (fwnode_property_present(child, "diff-channels")) {
>  			ret = fwnode_property_read_u32_array(child, "diff-channels",
>  							     pair, ARRAY_SIZE(pair));
> @@ -1995,6 +2124,9 @@ static int ads112c14_parse_channels(struct iio_dev *indio_dev,
>  			spec->differential = 1;
>  			spec->channel = pair[0];
>  			spec->channel2 = pair[1];
> +
> +			ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[0]);
> +			ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[1]);
>  		} else {
>  			return dev_err_probe(dev, -EINVAL,
>  					     "channel node missing channel type property\n");
> @@ -2026,6 +2158,10 @@ static int ads112c14_parse_channels(struct iio_dev *indio_dev,
>  			measurement->idac1_mux = pair[0];
>  			measurement->idac2_mux = measurement->iadc_count > 1 ? pair[1] : 0;
>  
> +			ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[0]);
> +			if (measurement->iadc_count > 1)

Put this and the setting of measurement->iadc2_mux under the same if()
Currently the mix of styles are making the match up of the two harder to spot.
Hmm. I wonder if we can do something fun like.

			meaurement->idac1_mux =
				ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[0]);
			if (measurement->iadc_count > 1)
				meaurement->idac2_mux =
					ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[1]);

Even better if we give that helper and the mask shorter names.  resv probably enough for
reserve for instance.


> +				ads112c14_reserve_gpio_for_ain(gpio_reserved_mask, pair[1]);
> +