From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linusw@kernel.org, brgl@kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org,
Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Add GPIO set_config
Date: Wed, 25 Mar 2026 12:48:43 +0200 [thread overview]
Message-ID: <3fc7c513-2f15-461b-b83c-29c3bccde609@tuxon.dev> (raw)
In-Reply-To: <CAMuHMdUEjMs9TgGR=vMG4Sd_XtnaW+D5Vrb0VBCZdb35TtLXEw@mail.gmail.com>
Hi, Geert,
On 3/24/26 19:28, Geert Uytterhoeven wrote:
> Hi Claudiu,
>
> On Mon, 16 Mar 2026 at 11:19, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>> On 3/13/26 15:15, Geert Uytterhoeven wrote:
>>> On Wed, 18 Feb 2026 at 16:19, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> Add GPIO set_config to allow setting GPIO specific functionalities.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> 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
Thank you for looking to this. I haven't experimented this with my RZ/G2L desk
board but looking on other RZ/G2L board that I have access to (not on my desk) I
can see similar errors even on RZ/G2L. I'll look into it.
Thank you,
Claudiu
>
> 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
>
prev parent reply other threads:[~2026-03-25 10:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-18 15:19 [PATCH] pinctrl: renesas: rzg2l: Add GPIO set_config Claudiu
2026-02-19 9:21 ` Bartosz Golaszewski
2026-02-19 19:04 ` Lad, Prabhakar
2026-02-24 9:08 ` Linus Walleij
2026-03-13 13:15 ` Geert Uytterhoeven
2026-03-16 10:19 ` claudiu beznea
2026-03-24 17:28 ` Geert Uytterhoeven
2026-03-24 19:02 ` Geert Uytterhoeven
2026-03-25 10:48 ` Claudiu Beznea [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3fc7c513-2f15-461b-b83c-29c3bccde609@tuxon.dev \
--to=claudiu.beznea@tuxon.dev \
--cc=brgl@kernel.org \
--cc=claudiu.beznea.uj@bp.renesas.com \
--cc=geert@linux-m68k.org \
--cc=linusw@kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).