Re: [PATCH] pinctrl: renesas: rzg2l: Add GPIO set_config

[Geert Uytterhoeven]

Hi Claudiu,

On Mon, 16 Mar 2026 at 11:19, claudiu beznea <claudiu.beznea@xxxxxxxxx> wrote:
> On 3/13/26 15:15, Geert Uytterhoeven wrote:
> > On Wed, 18 Feb 2026 at 16:19, Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>
> >> Add GPIO set_config to allow setting GPIO specific functionalities.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> >> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> >> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> >> @@ -1848,6 +1848,25 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
> >>          rzg2l_gpio_direction_input(chip, offset);
> >>   }
> >>
> >> +static int rzg2l_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> >> +                                unsigned long config)
> >> +{
> >> +       switch (pinconf_to_config_param(config)) {
> >> +       case PIN_CONFIG_BIAS_DISABLE:
> >> +       case PIN_CONFIG_BIAS_PULL_UP:
> >> +       case PIN_CONFIG_BIAS_PULL_DOWN:
> >> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> >> +       case PIN_CONFIG_DRIVE_PUSH_PULL:
> >> +       case PIN_CONFIG_SLEW_RATE:
> >> +       case PIN_CONFIG_DRIVE_STRENGTH:
> >> +       case PIN_CONFIG_DRIVE_STRENGTH_UA:
> >> +       case PIN_CONFIG_POWER_SOURCE:
> >
> > Shouldn't you handle all types that are supported by
> > rzg2l_pinctrl_pinconf_[gs]et()?
> >
> > The following are missing:
> > PIN_CONFIG_INPUT_ENABLE
> > PIN_CONFIG_OUTPUT_ENABLE
> > PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS
> > PIN_CONFIG_INPUT_SCHMITT_ENABLE
> > RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE
>
> I'll add these as well.

Apparently you can't just add RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE
to the switch statement, as gcc requires all case statements to use values
that are actually defined in the enum:

    drivers/pinctrl/renesas/pinctrl-rzg2l.c:2072:9: error: case value
‘128’ not in enumerated type ‘enum pin_config_param’ [-Werror=switch]

As the documentation states this range is meant for custom
configurations:

     * @PIN_CONFIG_END: this is the last enumerator for pin configurations, if
     *      you need to pass in custom configurations to the pin controller, use
     *      PIN_CONFIG_END+1 as the base offset.
     * @PIN_CONFIG_MAX: this is the maximum configuration value that can be
     *      presented using the packed format.

I fixed that by replacing the enum by u8 in the conversion macros:

    --- a/include/linux/pinctrl/pinconf-generic.h
    +++ b/include/linux/pinctrl/pinconf-generic.h
    @@ -173,9 +173,9 @@ enum pin_config_param {
      * upper 24 bits.
      */

    -static inline enum pin_config_param
pinconf_to_config_param(unsigned long config)
    +static inline u8 pinconf_to_config_param(unsigned long config)
     {
    -       return (enum pin_config_param) (config & 0xffUL);
    +       return config & 0xffUL;
     }

     static inline u32 pinconf_to_config_argument(unsigned long config)
    @@ -183,8 +183,7 @@ static inline u32
pinconf_to_config_argument(unsigned long config)
            return (u32) ((config >> 8) & 0xffffffUL);
     }

    -static inline unsigned long pinconf_to_config_packed(enum
pin_config_param param,
    -                                                    u32 argument)
    +static inline unsigned long pinconf_to_config_packed(u8 param,
u32 argument)
     {
            return PIN_CONF_PACKED(param, argument);
     }

Probably a few more should be updated, too?

> >> @@ -2819,6 +2838,7 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
> >>        chip->direction_output = rzg2l_gpio_direction_output;
> >>        chip->get = rzg2l_gpio_get;
> >>        chip->set = rzg2l_gpio_set;
> >> +       chip->set_config = rzg2l_gpio_set_config;
> >>        chip->label = name;
> >>        chip->parent = pctrl->dev;
> >>        chip->owner = THIS_MODULE;

This change breaks pin control and GPIO on RZ/Five:

    -pinctrl-rzg2l 11030000.pinctrl: pinctrl-rzg2l support registered
    +gpio gpiochip0: (11030000.pinctrl): setup of own GPIO can0_stb failed
    +requesting hog GPIO can0_stb (chip 11030000.pinctrl, offset 18) failed, -95
    +gpiochip_add_data_with_key: GPIOs 512..743 (11030000.pinctrl)
failed to register, -95
    +pinctrl-rzg2l 11030000.pinctrl: error -EOPNOTSUPP: failed to add
GPIO controller
    +pinctrl-rzg2l 11030000.pinctrl: error -EOPNOTSUPP: failed to add GPIO chip
    +pinctrl-rzg2l 11030000.pinctrl: probe with driver pinctrl-rzg2l
failed with error -95

For the can0_stb hog, rzg2l_gpio_set_config() is called with offset 18 and
config 0x115 (PIN_CONFIG_PERSIST_STATE = 1).

Just adding PIN_CONFIG_PERSIST_STATE to the switch doesn't help,
as pinctrl_gpio_set_config() also returns -EOPNOTSUPP.
Ignoring PIN_CONFIG_PERSIST_STATE helps a bit, but the next call
uses config 0x8, and pinctrl_gpio_set_config() now returns -EINVAL,
but the pin controller now gets registered?...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds