Re: [PATCH 2/2] gpio: Support ON Semi CAT9532

[Linus Walleij]

Hi Alban,

thanks for your patch!

overall if it should be possible to use the same component as a LED dimmer
then some DT property flag such as onn,gpio-mode; or so is needed to make
sure only the GPIO mode driver is probed?

I'm not sure about how to flag this in DT, check with the binding maintainers.

On Tue, Sep 15, 2026 at 11:21 AM Alban Bedel <alban.bedel@xxxxxxxxxx> wrote:

> Add support for the CAT9532 LED dimmer from On Semiconductor. Altought
> marketed as an LED dimmer it is a bit limited in this regard as it
> only has 2 PWM for 16 pins. As explained in the datasheet it can also
> be used as an open-drain GPIO controller, and that's how it is used on
> the hardware I have at hand. This chip doesn't really map to what the
> gpio-regmap driver expect so it is implemented as it own driver. This
> approch also allow to later add support for the LED dimmer if desired.
>
> Signed-off-by: Alban Bedel <alban.bedel@xxxxxxxxxx>
(...)

> +static int cat9532_gpio_direction_input(struct gpio_chip *chip,
> +                                       unsigned int offset)
> +{
> +       struct cat9532 *cat9532 = container_of(chip, struct cat9532, gpio);
> +       unsigned int reg = CAT9532_REG_LS0 + (offset >> 2);
> +       unsigned int shift = (offset & 3) << 1;
> +       int err;
> +
> +       err = regmap_update_bits(cat9532->regmap, reg,
> +                                CAT9532_LED_OUT_MASK << shift,
> +                                CAT9532_LED_OUT_HIZ << shift);
> +       if (err < 0)
> +               return err;
> +
> +       scoped_guard(spinlock, &cat9532->lock)
> +               cat9532->direction &= ~BIT(offset);
> +
> +       return 0;
> +}
> +
> +static int cat9532_gpio_get(struct gpio_chip *chip,
> +                           unsigned int offset)
> +{
> +       struct cat9532 *cat9532 = container_of(chip, struct cat9532, gpio);
> +       unsigned int reg = CAT9532_REG_INPUT0 + (offset >> 3);
> +       unsigned int mask = BIT(offset & 7);
> +       unsigned int val;
> +       int err;
> +
> +       err = regmap_read(cat9532->regmap, reg, &val);
> +       if (err < 0)
> +               return err;
> +
> +       return !!(val & mask);
> +}
> +
> +static int cat9532_gpio_set(struct gpio_chip *chip,
> +                           unsigned int offset, int value)
> +{
> +       struct cat9532 *cat9532 = container_of(chip, struct cat9532, gpio);
> +       unsigned int reg = CAT9532_REG_LS0 + (offset >> 2);
> +       unsigned int shift = (offset & 3) << 1;
> +       unsigned int val;
> +
> +       val = value ? CAT9532_LED_OUT_HIZ : CAT9532_LED_OUT_LOW;
> +       return regmap_update_bits(cat9532->regmap, reg,
> +                                 CAT9532_LED_OUT_MASK << shift,
> +                                 val << shift);
> +}

So clearly all GPIO lines are open drain, requiring an external
pull-up resistor.

I would handle things this way:

1. Implement .set_config for this gpio_chip.
2. In the .set_config() callback, reject anything except PIN_CONFIG_OPEN_DRAIN
   pinconf_to_config_param(config) != PIN_CONFIG_OPEN_DRAIN return -EINVAL;
3. If setting open drain succeeds, mark the line offset as configured
in a bitmap
4. Do not accept any get/set calls until the line has been configured as
  open drain.

This forces users to always properly configure the line as open drain before
usage.

Maybe it is possible to also add restrictions in the YAML DT bindings
to enforce consumers to set open drain, I'm not sure. But we can at least
enforce it at runtime.

> +       chip = &cat9532->gpio;
> +       chip->label = "cat9532";
> +       chip->parent = &client->dev;
> +       chip->owner = THIS_MODULE;
> +       chip->can_sleep = true;
> +       chip->base = -1;
> +       chip->ngpio = 16;

This is right, no using ngpio :)

Yours,
Linus Walleij