When gp0 or gp1 is not taken as an interrupt, expose them as gpo if
gpio-contoller is set in the devicetree.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/iio/adc/ad4062.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)

diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
index 3df7dbf29ae4a..203b06276431f 100644
--- a/drivers/iio/adc/ad4062.c
+++ b/drivers/iio/adc/ad4062.c
@@ -10,6 +10,7 @@
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/gpio/driver.h>
 #include <linux/i3c/device.h>
 #include <linux/i3c/master.h>
 #include <linux/iio/buffer.h>
@@ -85,8 +86,11 @@
 #define AD4062_MAX_AVG		0xB
 #define AD4062_MON_VAL_MAX_GAIN		1999970
 #define AD4062_MON_VAL_MIDDLE_POINT	0x8000
+#define AD4062_GP_DISABLED	0x0
 #define AD4062_GP_INTR		0x1
 #define AD4062_GP_DRDY		0x2
+#define AD4062_GP_STATIC_LOW	0x5
+#define AD4062_GP_STATIC_HIGH	0x6
 #define AD4062_INTR_EN_NEITHER	0x0
 #define AD4062_INTR_EN_EITHER	0x3
 #define AD4062_TCONV_NS		270
@@ -635,12 +639,14 @@ static int ad4062_request_irq(struct iio_dev *indio_dev)
 	if (ret == -EPROBE_DEFER) {
 		return ret;
 	} else if (ret < 0) {
+		st->gpo_irq[0] = false;
 		ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
 					 AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN,
 					 AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN);
 		if (ret)
 			return ret;
 	} else {
+		st->gpo_irq[0] = true;
 		ret = devm_request_threaded_irq(dev, ret, NULL,
 						ad4062_irq_handler_thresh,
 						IRQF_ONESHOT, indio_dev->name,
@@ -1263,6 +1269,130 @@ static int ad4062_regulators_get(struct ad4062_state *st, bool *ref_sel)
 	return 0;
 }
 
+static int ad4062_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int ad4062_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct ad4062_state *st = gpiochip_get_data(gc);
+	unsigned int reg_val = value ? AD4062_GP_STATIC_HIGH : AD4062_GP_STATIC_LOW;
+
+	if (st->gpo_irq[offset])
+		return -ENODEV;
+
+	if (offset)
+		return regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
+					  AD4062_REG_GP_CONF_MODE_MSK_1,
+					  FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, reg_val));
+	else
+		return regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
+					  AD4062_REG_GP_CONF_MODE_MSK_0,
+					  FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_0, reg_val));
+}
+
+static int ad4062_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct ad4062_state *st = gpiochip_get_data(gc);
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD4062_REG_GP_CONF, &reg_val);
+	if (ret)
+		return 0;
+
+	if (st->gpo_irq[offset])
+		return -ENODEV;
+
+	if (offset)
+		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_1, reg_val);
+	else
+		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_0, reg_val);
+
+	return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
+}
+
+static void ad4062_gpio_disable(void *data)
+{
+	struct ad4062_state *st = data;
+	u8 val = FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_0, AD4062_GP_DISABLED) |
+		 FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_DISABLED);
+
+	regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
+			   AD4062_REG_GP_CONF_MODE_MSK_1 | AD4062_REG_GP_CONF_MODE_MSK_0,
+			   val);
+}
+
+static int ad4062_gpio_init_valid_mask(struct gpio_chip *gc,
+				       unsigned long *valid_mask,
+				       unsigned int ngpios)
+{
+	struct ad4062_state *st = gpiochip_get_data(gc);
+
+	bitmap_zero(valid_mask, ngpios);
+
+	if (!st->gpo_irq[0])
+		set_bit(0, valid_mask);
+	if (!st->gpo_irq[1])
+		set_bit(1, valid_mask);
+
+	return 0;
+}
+
+static int ad4062_gpio_init(struct ad4062_state *st)
+{
+	struct device *dev = &st->i3cdev->dev;
+	struct gpio_chip *gc;
+	u8 val, mask;
+	int ret;
+
+	if ((st->gpo_irq[0] && st->gpo_irq[1]) ||
+	    !device_property_read_bool(dev, "gpio-controller"))
+		return 0;
+
+	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
+	if (!gc)
+		return -ENOMEM;
+
+	val = 0;
+	mask = 0;
+	if (!st->gpo_irq[0]) {
+		mask |= AD4062_REG_GP_CONF_MODE_MSK_0;
+		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_0, AD4062_GP_STATIC_LOW);
+	}
+	if (!st->gpo_irq[1]) {
+		mask |= AD4062_REG_GP_CONF_MODE_MSK_1;
+		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_STATIC_LOW);
+	}
+
+	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
+				 mask, val);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, ad4062_gpio_disable, st);
+	if (ret)
+		return ret;
+
+	gc->parent = dev;
+	gc->label = st->chip->name;
+	gc->owner = THIS_MODULE;
+	gc->base = -1;
+	gc->ngpio = 2;
+	gc->init_valid_mask = ad4062_gpio_init_valid_mask;
+	gc->get_direction = ad4062_gpio_get_direction;
+	gc->set = ad4062_gpio_set;
+	gc->get = ad4062_gpio_get;
+	gc->can_sleep = true;
+
+	ret = devm_gpiochip_add_data(dev, gc, st);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to register GPIO chip\n");
+
+	return 0;
+}
+
 static const struct i3c_device_id ad4062_id_table[] = {
 	I3C_DEVICE(AD4062_I3C_VENDOR, ad4060_chip_info.prod_id, &ad4060_chip_info),
 	I3C_DEVICE(AD4062_I3C_VENDOR, ad4062_chip_info.prod_id, &ad4062_chip_info),
@@ -1351,6 +1481,10 @@ static int ad4062_probe(struct i3c_device *i3cdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");
 
+	ret = ad4062_gpio_init(st);
+	if (ret)
+		return ret;
+
 	INIT_WORK(&st->trig_conv, ad4062_trigger_work);
 
 	return devm_iio_device_register(dev, indio_dev);

-- 
2.51.1

On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:
> When gp0 or gp1 is not taken as an interrupt, expose them as gpo if

GPO

> gpio-contoller is set in the devicetree.

Why can't gpio-regmap be used?

...

> +static int ad4062_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct ad4062_state *st = gpiochip_get_data(gc);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, AD4062_REG_GP_CONF, &reg_val);
> +	if (ret)
> +		return 0;

> +	if (st->gpo_irq[offset])
> +		return -ENODEV;

Consider using valid_mask instead (.init_valid_mask() callback).
Hmm... And it seems it's in place. I didn't get what is here then and
why we need to do it after accessing the HW? If there are side-effects
they must be described.

> +	if (offset)
> +		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_1, reg_val);
> +	else
> +		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_0, reg_val);
> +
> +	return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;

	return !!(reg_val == AD4062_GP_STATIC_HIGH);

also will work.

> +}

> +static int ad4062_gpio_init_valid_mask(struct gpio_chip *gc,
> +				       unsigned long *valid_mask,
> +				       unsigned int ngpios)
> +{
> +	struct ad4062_state *st = gpiochip_get_data(gc);
> +
> +	bitmap_zero(valid_mask, ngpios);
> +
> +	if (!st->gpo_irq[0])
> +		set_bit(0, valid_mask);
> +	if (!st->gpo_irq[1])
> +		set_bit(1, valid_mask);

Why atomic bit set:s?


> +	return 0;
> +}
> +
> +static int ad4062_gpio_init(struct ad4062_state *st)
> +{
> +	struct device *dev = &st->i3cdev->dev;
> +	struct gpio_chip *gc;
> +	u8 val, mask;
> +	int ret;

> +	if ((st->gpo_irq[0] && st->gpo_irq[1]) ||
> +	    !device_property_read_bool(dev, "gpio-controller"))
> +		return 0;

Do you need this? valid_mask should take care of this.

> +	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
> +	if (!gc)
> +		return -ENOMEM;
> +
> +	val = 0;
> +	mask = 0;
> +	if (!st->gpo_irq[0]) {
> +		mask |= AD4062_REG_GP_CONF_MODE_MSK_0;
> +		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_0, AD4062_GP_STATIC_LOW);
> +	}
> +	if (!st->gpo_irq[1]) {
> +		mask |= AD4062_REG_GP_CONF_MODE_MSK_1;
> +		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_STATIC_LOW);
> +	}
> +
> +	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
> +				 mask, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, ad4062_gpio_disable, st);
> +	if (ret)
> +		return ret;
> +
> +	gc->parent = dev;
> +	gc->label = st->chip->name;
> +	gc->owner = THIS_MODULE;
> +	gc->base = -1;
> +	gc->ngpio = 2;
> +	gc->init_valid_mask = ad4062_gpio_init_valid_mask;
> +	gc->get_direction = ad4062_gpio_get_direction;
> +	gc->set = ad4062_gpio_set;
> +	gc->get = ad4062_gpio_get;
> +	gc->can_sleep = true;
> +
> +	ret = devm_gpiochip_add_data(dev, gc, st);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to register GPIO chip\n");
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:
> > When gp0 or gp1 is not taken as an interrupt, expose them as gpo if
Hi Andy,
> 
> GPO
Ack.
> 
> > gpio-contoller is set in the devicetree.
> 
> Why can't gpio-regmap be used?
> 
Because the device register values (0x5, 0x6) does not fit the gpio-regmap.
It writes the mask for high and 0 for low.
But low is 01[01] and
    high   01[10]

A different series would need to extend the gpio-regmap ops, but if you
implement your custom reg read/write, then you save at most ~5 lines...
I will add that to the commit message.
> ...
> 
> > +static int ad4062_gpio_get(struct gpio_chip *gc, unsigned int offset)
> > +{
> > +	struct ad4062_state *st = gpiochip_get_data(gc);
> > +	unsigned int reg_val;
> > +	int ret;
> > +
> > +	ret = regmap_read(st->regmap, AD4062_REG_GP_CONF, &reg_val);
> > +	if (ret)
> > +		return 0;
Should have been
  		return ret;
> 
> > +	if (st->gpo_irq[offset])
> > +		return -ENODEV;
> 
> Consider using valid_mask instead (.init_valid_mask() callback).
> Hmm... And it seems it's in place. I didn't get what is here then and
> why we need to do it after accessing the HW? If there are side-effects
> they must be described.
> 
True, this is not necessary the valid mask does the same.
> > +	if (offset)
> > +		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_1, reg_val);
> > +	else
> > +		reg_val = FIELD_GET(AD4062_REG_GP_CONF_MODE_MSK_0, reg_val);
> > +
> > +	return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> 
> 	return !!(reg_val == AD4062_GP_STATIC_HIGH);
> 
> also will work.
>
 	return reg_val == AD4062_GP_STATIC_HIGH;
> > +}
> 
> > +static int ad4062_gpio_init_valid_mask(struct gpio_chip *gc,
> > +				       unsigned long *valid_mask,
> > +				       unsigned int ngpios)
> > +{
> > +	struct ad4062_state *st = gpiochip_get_data(gc);
> > +
> > +	bitmap_zero(valid_mask, ngpios);
> > +
> > +	if (!st->gpo_irq[0])
> > +		set_bit(0, valid_mask);
> > +	if (!st->gpo_irq[1])
> > +		set_bit(1, valid_mask);
> 
> Why atomic bit set:s?
> 
Not needed, will use

	if (!st->gpo_irq[0])
		*valid_mask |= BIT(0);
	if (!st->gpo_irq[1])
		*valid_mask |= BIT(1);

> 
> > +	return 0;
> > +}
> > +
> > +static int ad4062_gpio_init(struct ad4062_state *st)
> > +{
> > +	struct device *dev = &st->i3cdev->dev;
> > +	struct gpio_chip *gc;
> > +	u8 val, mask;
> > +	int ret;
> 
> > +	if ((st->gpo_irq[0] && st->gpo_irq[1]) ||
> > +	    !device_property_read_bool(dev, "gpio-controller"))
> > +		return 0;
> 
> Do you need this? valid_mask should take care of this.
> 
True, this is not necessary.
> > +	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
> > +	if (!gc)
> > +		return -ENOMEM;
> > +
> > +	val = 0;
> > +	mask = 0;
> > +	if (!st->gpo_irq[0]) {
> > +		mask |= AD4062_REG_GP_CONF_MODE_MSK_0;
> > +		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_0, AD4062_GP_STATIC_LOW);
> > +	}
> > +	if (!st->gpo_irq[1]) {
> > +		mask |= AD4062_REG_GP_CONF_MODE_MSK_1;
> > +		val |= FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_STATIC_LOW);
> > +	}
> > +
> > +	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
> > +				 mask, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_add_action_or_reset(dev, ad4062_gpio_disable, st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	gc->parent = dev;
> > +	gc->label = st->chip->name;
> > +	gc->owner = THIS_MODULE;
> > +	gc->base = -1;
> > +	gc->ngpio = 2;
> > +	gc->init_valid_mask = ad4062_gpio_init_valid_mask;
> > +	gc->get_direction = ad4062_gpio_get_direction;
> > +	gc->set = ad4062_gpio_set;
> > +	gc->get = ad4062_gpio_get;
> > +	gc->can_sleep = true;
> > +
> > +	ret = devm_gpiochip_add_data(dev, gc, st);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Unable to register GPIO chip\n");
> > +
> > +	return 0;
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Best regards,
Jorge

On Wed, Nov 26, 2025 at 04:55:41PM +0100, Jorge Marques wrote:
> On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:

...

> > Why can't gpio-regmap be used?
> > 
> Because the device register values (0x5, 0x6) does not fit the gpio-regmap.
> It writes the mask for high and 0 for low.
> But low is 01[01] and
>     high   01[10]
> 
> A different series would need to extend the gpio-regmap ops, but if you
> implement your custom reg read/write, then you save at most ~5 lines...
> I will add that to the commit message.

OK.

...

> > > +	return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> > 
> > 	return !!(reg_val == AD4062_GP_STATIC_HIGH);
> > 
> > also will work.
> >
>  	return reg_val == AD4062_GP_STATIC_HIGH;

Hmm... This will include implicit bool->int. The !! guarantees values 0 or 1,
but I don't remember about implicit bool->int case.

...

> > > +static int ad4062_gpio_init_valid_mask(struct gpio_chip *gc,
> > > +				       unsigned long *valid_mask,
> > > +				       unsigned int ngpios)
> > > +{
> > > +	struct ad4062_state *st = gpiochip_get_data(gc);
> > > +
> > > +	bitmap_zero(valid_mask, ngpios);
> > > +
> > > +	if (!st->gpo_irq[0])
> > > +		set_bit(0, valid_mask);
> > > +	if (!st->gpo_irq[1])
> > > +		set_bit(1, valid_mask);
> > 
> > Why atomic bit set:s?
> > 
> Not needed, will use

Note, bitops are xxx_bit() -- atomic, __xxx_bit() -- non-atomic,
that's what I had in mind.

> 	if (!st->gpo_irq[0])
> 		*valid_mask |= BIT(0);
> 	if (!st->gpo_irq[1])
> 		*valid_mask |= BIT(1);

Can't it be rather something like

	for (unsigned int i = 0; i < ...; i++)
		__assign_bit(i, valid_mask, st->gpo_irq[i]);

?
This shorter and does the same independently on the length of the bitmask
(and effectively the array size of gpo_irq)

> > > +	return 0;
> > > +}

-- 
With Best Regards,
Andy Shevchenko

On Thu, Nov 27, 2025 at 11:20:54AM +0200, Andy Shevchenko wrote:
Hi Andy,
> On Wed, Nov 26, 2025 at 04:55:41PM +0100, Jorge Marques wrote:
> > On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> > > On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:
> 
> ...
> 
> > > > +	return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> > > 
> > > 	return !!(reg_val == AD4062_GP_STATIC_HIGH);
> > > 
> > > also will work.
> > >
> >  	return reg_val == AD4062_GP_STATIC_HIGH;
> 
> Hmm... This will include implicit bool->int. The !! guarantees values 0 or 1,
> but I don't remember about implicit bool->int case.
> 
> ...
I don't think the implicit bool->int is an issue, grepping `return .* == .*;`
matches a few methods that return int.
Experimenting with the _Bool type (gcc 15, clang 19, any std version),

	int main()
	{
	    int a = 1;
	    int b = 2;
	
	    return (_Bool)(a == b);
	}

with
gcc -Wall -W -pedantic -std=c23 -c test.c
clang -Wall -Wextra -Wbool-conversion -std=c11 -O2 test.c

also doesn't raise warnings.

> 
> > > > +static int ad4062_gpio_init_valid_mask(struct gpio_chip *gc,
> > > > +				       unsigned long *valid_mask,
> > > > +				       unsigned int ngpios)
> > > > +{
> > > > +	struct ad4062_state *st = gpiochip_get_data(gc);
> > > > +
> > > > +	bitmap_zero(valid_mask, ngpios);
> > > > +
> > > > +	if (!st->gpo_irq[0])
> > > > +		set_bit(0, valid_mask);
> > > > +	if (!st->gpo_irq[1])
> > > > +		set_bit(1, valid_mask);
> > > 
> > > Why atomic bit set:s?
> > > 
> > Not needed, will use
> 
> Note, bitops are xxx_bit() -- atomic, __xxx_bit() -- non-atomic,
> that's what I had in mind.
> 
> > 	if (!st->gpo_irq[0])
> > 		*valid_mask |= BIT(0);
> > 	if (!st->gpo_irq[1])
> > 		*valid_mask |= BIT(1);
> 
> Can't it be rather something like
> 
> 	for (unsigned int i = 0; i < ...; i++)
> 		__assign_bit(i, valid_mask, st->gpo_irq[i]);
> 
> ?
> This shorter and does the same independently on the length of the bitmask
> (and effectively the array size of gpo_irq)
> 
Sure, just
 		__assign_bit(i, valid_mask, !st->gpo_irq[i]);

"Set as valid gpo if not used as irq"
> > > > +	return 0;
> > > > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
Best Regards,
Jorge

On Thu, Dec 4, 2025 at 11:38 PM Jorge Marques <gastmaier@gmail.com> wrote:
> On Thu, Nov 27, 2025 at 11:20:54AM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 26, 2025 at 04:55:41PM +0100, Jorge Marques wrote:
> > > On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:

...

> > > > > +       return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> > > >
> > > >   return !!(reg_val == AD4062_GP_STATIC_HIGH);
> > > >
> > > > also will work.
> > > >
> > >     return reg_val == AD4062_GP_STATIC_HIGH;
> >
> > Hmm... This will include implicit bool->int. The !! guarantees values 0 or 1,
> > but I don't remember about implicit bool->int case.

> I don't think the implicit bool->int is an issue, grepping `return .* == .*;`
> matches a few methods that return int.

Yes, the Q here is the value of true _always_ be promoted to 1?

> Experimenting with the _Bool type (gcc 15, clang 19, any std version),
>
>         int main()
>         {
>             int a = 1;
>             int b = 2;
>
>             return (_Bool)(a == b);
>         }
>
> with
> gcc -Wall -W -pedantic -std=c23 -c test.c
> clang -Wall -Wextra -Wbool-conversion -std=c11 -O2 test.c
>
> also doesn't raise warnings.

Of course, because before even looking into warnings the entire code
degrades to return 0. I.o.w., the test case is not correct. But don't
hurry up to fix it, you won't get warnings anyway, it's all about C
standard and not about (in)correctness of the code. See above.

-- 
With Best Regards,
Andy Shevchenko

On Fri, Dec 05, 2025 at 12:21:31AM +0200, Andy Shevchenko wrote:
> On Thu, Dec 4, 2025 at 11:38 PM Jorge Marques <gastmaier@gmail.com> wrote:
> > On Thu, Nov 27, 2025 at 11:20:54AM +0200, Andy Shevchenko wrote:
> > > On Wed, Nov 26, 2025 at 04:55:41PM +0100, Jorge Marques wrote:
> > > > On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> > > > > On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:
> 
> ...
> 
> > > > > > +       return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> > > > >
> > > > >   return !!(reg_val == AD4062_GP_STATIC_HIGH);
> > > > >
> > > > > also will work.
> > > > >
> > > >     return reg_val == AD4062_GP_STATIC_HIGH;
> > >
> > > Hmm... This will include implicit bool->int. The !! guarantees values 0 or 1,
> > > but I don't remember about implicit bool->int case.
> 
> > I don't think the implicit bool->int is an issue, grepping `return .* == .*;`
> > matches a few methods that return int.
> 
> Yes, the Q here is the value of true _always_ be promoted to 1?
> 
Hi Andy,

The relational operator result has type int (c99 6.5.9 Equality
operators); and when any scalar value is converted to _Bool, the result
is 0 if the value compares equal to 0; otherwise, the result is 1 (c99
6.3.1.2).
https://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf

No conversion warnings even when forcing _Bool type.
There are many usages like this, for example:

drivers/iio/accel/adxl313_core.c @ int adxl313_is_act_inact_ac()
drivers/iio/light/opt4060.c @ int opt4060_read_event_config()
drivers/iio/light/tsl2772.c @ int tsl2772_device_id_verify()
lib/zstd/compress/zstd_fast.c @ int ZSTD_match4Found_branch()

I cannot find many legitimate usage of relational operator with the
double negation.
  git ls-files | xargs grep -s 'return !!' | grep '=='

> > Experimenting with the _Bool type (gcc 15, clang 19, any std version),
> >
> >         int main()
> >         {
> >             int a = 1;
> >             int b = 2;
> >
> >             return (_Bool)(a == b);
> >         }
> >
> > with
> > gcc -Wall -W -pedantic -std=c23 -c test.c
> > clang -Wall -Wextra -Wbool-conversion -std=c11 -O2 test.c
> >
> > also doesn't raise warnings.
> 
> Of course, because before even looking into warnings the entire code
> degrades to return 0. I.o.w., the test case is not correct. But don't
> hurry up to fix it, you won't get warnings anyway, it's all about C
> standard and not about (in)correctness of the code. See above.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
Best Regards,
Jorge

On Fri, Dec 5, 2025 at 1:53 PM Jorge Marques <gastmaier@gmail.com> wrote:
> On Fri, Dec 05, 2025 at 12:21:31AM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 4, 2025 at 11:38 PM Jorge Marques <gastmaier@gmail.com> wrote:
> > > On Thu, Nov 27, 2025 at 11:20:54AM +0200, Andy Shevchenko wrote:
> > > > On Wed, Nov 26, 2025 at 04:55:41PM +0100, Jorge Marques wrote:
> > > > > On Mon, Nov 24, 2025 at 12:40:37PM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Nov 24, 2025 at 10:18:08AM +0100, Jorge Marques wrote:

...

> > > > > > > +       return reg_val == AD4062_GP_STATIC_HIGH ? 1 : 0;
> > > > > >
> > > > > >   return !!(reg_val == AD4062_GP_STATIC_HIGH);
> > > > > >
> > > > > > also will work.
> > > > > >
> > > > >     return reg_val == AD4062_GP_STATIC_HIGH;
> > > >
> > > > Hmm... This will include implicit bool->int. The !! guarantees values 0 or 1,
> > > > but I don't remember about implicit bool->int case.
> >
> > > I don't think the implicit bool->int is an issue, grepping `return .* == .*;`
> > > matches a few methods that return int.
> >
> > Yes, the Q here is the value of true _always_ be promoted to 1?
>
> The relational operator result has type int (c99 6.5.9 Equality
> operators); and when any scalar value is converted to _Bool, the result
> is 0 if the value compares equal to 0; otherwise, the result is 1 (c99
> 6.3.1.2).
> https://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf

Okay, thanks for checking this!
So, let's go with a simplified variant then.

> No conversion warnings even when forcing _Bool type.
> There are many usages like this, for example:
>
> drivers/iio/accel/adxl313_core.c @ int adxl313_is_act_inact_ac()
> drivers/iio/light/opt4060.c @ int opt4060_read_event_config()
> drivers/iio/light/tsl2772.c @ int tsl2772_device_id_verify()
> lib/zstd/compress/zstd_fast.c @ int ZSTD_match4Found_branch()
>
> I cannot find many legitimate usage of relational operator with the
> double negation.
>   git ls-files | xargs grep -s 'return !!' | grep '=='

-- 
With Best Regards,
Andy Shevchenko

On Mon, Nov 24, 2025 at 10:19 AM Jorge Marques <jorge.marques@analog.com> wrote:

> When gp0 or gp1 is not taken as an interrupt, expose them as gpo if
> gpio-contoller is set in the devicetree.
>
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>

It looks to me like it will work!

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij