Re: [PATCH 2/2] gpio: adg1712: add driver support

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2025-11-11 at 12:05 +0100, Linus Walleij wrote:
> On Mon, Nov 10, 2025 at 1:32 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> > On Mon, 2025-11-10 at 11:30 +0100, Linus Walleij wrote:
> > > Hi Antoniu,
> > > 
> > > thanks for your patch!
> > > 
> > > On Fri, Oct 31, 2025 at 5:08 PM Antoniu Miclaus
> > > <antoniu.miclaus@analog.com> wrote:
> > > 
> > > > Add driver support for the ADG1712, which contains four independent
> > > > single-pole/single-throw (SPST) switches and operates with a
> > > > low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> > > > dual supply range from ±1.08V to ±2.75V.
> > > > 
> > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > 
> > > So tying into the binding discussion:
> > > 
> > > GPIO means "general purpose input/output".
> > > 
> > > I am really confused as whether this is:
> > > 
> > > - General purpose - seems to be for the purpose of switching
> > >   currents and nothing else.
> > > 
> > > - Input/Output - It's switching something else and not inputting
> > >   or outputting anything and this makes the driver look strange.
> > > 
> > > 
> > 
> > Not the first time a part like this pops up [1]. At the time, the final
> > conclusion was to go with gpiolib. Naturally you can think otherwise now :)
> 
> I think we might wanna go with gpiolib for the Linux internals, maybe
> we want to add some kind of awareness or flag in gpiolib that this is
> a switch and not an output we can control the level of?
> 
> I could think of this:
> 
> - Make .get() and .set() in struct gpio_chip return -ENOTIMP
>   no getting and setting these "lines" because we really cannot
>   control that, these lines will have the level of whatever is on
>   the line we are switching.
> 
> - Implement .set_config() and implement the generic pin
>   control property PIN_CONFIG_OUTPUT_ENABLE as 1
>   to switch "on" and 0 for switch "off".
>   See include/linux/pinctrl/pinconf-generic.h
> 
> This makes it possible to use the gpiolib in a way that is
> non-ambiguous.
> 

The above makes sense to me. I'll let Antoniu take it from here and check if 
the above fits the usecases he is aware of. Not sure if it makes sense for a piece
of HW like this but if the usecase is for userspace to control the on/off states,
then I guess we would need .get() and .set(). Or some kind of "frontend" driver
making use of the consumer helpers.

Thanks!
- Nuno Sá

> We might want to add consumer helpers in
> include/linux/gpio/consumer.h such as:
> 
> #include <linux/pinctrl/pinconf-generic.h>
> 
> int gpiod_switch_enable(struct gpio_desc *desc)
> {
>    unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 1);
> 
>    return gpiod_set_config(desc, cfg);
> }
> 
> int gpiod_switch_disable(struct gpio_desc *desc)
> {
>    unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);
> 
>    return gpiod_set_config(desc, cfg);
> }
> 
> See e.g. rtd_gpio_set_config() in drivers/gpio/gpio-rtd.c for
> an example of how the GPIO driver can unpack and handle
> generic .set_config() options like this.
> 
> The binding however needs to be something separate like a proper switch binding
> I think or we will confuse other operating systems.
> 
> Yours,
> Linus Walleij