Re: [PATCH v2 2/3] pinctrl: starfive: jh7110: add support for PAD_INTERNAL_* for GPI

[Icenowy Zheng <uwu@icenowy.me>]

在 2025-04-24星期四的 14:20 +0800，Icenowy Zheng写道：
> The JH7110 SoC's both pin controller support routing GPI signals to
> internal fixed low/high level.
> 
> As we allocated two special "pin" numbers for these situations
> (PAD_INTERNAL_{LOW,HIGH}), add special handling code for these
> "pins".
> The DOEn/DOUT/FUNCTION fields are ignored and the internal input
> signal
> specified by the DIN field is routed to fixed low/high level.

Oops today I found that this patchset has some problem -- the GPIOMUX
macro masks bits 7 and 8 in GPIO number, and it maps
GPI_SYS_USB_OVERCURRENT to GPIO63 instead of PAD_INTERNAL_HIGH instead.

When I fixed this, I got

```
[    9.865841] starfive-jh7110-sys-pinctrl 13040000.pinctrl: pin 255 is
not registered so it cannot be requested
[    9.875814] starfive-jh7110-sys-pinctrl 13040000.pinctrl: error -
EINVAL: pin-255 (soc:usb@10100000)
[    9.884882] starfive-jh7110-sys-pinctrl 13040000.pinctrl: error -
EINVAL: could not request pin 255 (non-existing) from group usb0-
0.overcurrent-pins on device 13040000.pinctrl
```

Weird problem, how could I solve this?

> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>  .../starfive/pinctrl-starfive-jh7110.c        | 41 +++++++++++++++--
> --
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
> b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
> index 1d0d6c224c104..fb18c7974ec86 100644
> --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
> @@ -291,6 +291,24 @@ void jh7110_set_gpiomux(struct jh7110_pinctrl
> *sfp, unsigned int pin,
>  }
>  EXPORT_SYMBOL_GPL(jh7110_set_gpiomux);
>  
> +static void jh7110_set_gpi(struct jh7110_pinctrl *sfp, u32 gpi, u32
> val)
> +{
> +       u32 offset, shift;
> +       u32 reg_val;
> +       const struct jh7110_pinctrl_soc_info *info = sfp->info;
> +
> +       offset = 4 * (gpi / 4);
> +       shift  = 8 * (gpi % 4);
> +
> +       reg_val = readl_relaxed(sfp->base +
> +                               info->gpi_reg_base + offset);
> +       reg_val &= info->gpi_mask << shift;
> +       reg_val |= (val & info->gpi_mask) << shift;
> +
> +       writel_relaxed(reg_val, sfp->base +
> +                               info->gpi_reg_base + offset);
> +}
> +
>  static int jh7110_set_mux(struct pinctrl_dev *pctldev,
>                           unsigned int fsel, unsigned int gsel)
>  {
> @@ -307,14 +325,23 @@ static int jh7110_set_mux(struct pinctrl_dev
> *pctldev,
>         pinmux = group->data;
>         for (i = 0; i < group->grp.npins; i++) {
>                 u32 v = pinmux[i];
> +               u32 pin = jh7110_pinmux_pin(v);
>  
> -               if (info->jh7110_set_one_pin_mux)
> -                       info->jh7110_set_one_pin_mux(sfp,
> -                                       jh7110_pinmux_pin(v),
> -                                       jh7110_pinmux_din(v),
> -                                       jh7110_pinmux_dout(v),
> -                                       jh7110_pinmux_doen(v),
> -                                       jh7110_pinmux_function(v));
> +               switch (pin) {
> +               case PAD_INTERNAL_LOW:
> +               case PAD_INTERNAL_HIGH:
> +                       jh7110_set_gpi(sfp, jh7110_pinmux_din(v),
> +                                      pin == PAD_INTERNAL_HIGH);
> +                       break;
> +               default:
> +                       if (info->jh7110_set_one_pin_mux)
> +                               info->jh7110_set_one_pin_mux(sfp,
> +                                               jh7110_pinmux_pin(v),
> +                                               jh7110_pinmux_din(v),
> +                                               jh7110_pinmux_dout(v)
> ,
> +                                               jh7110_pinmux_doen(v)
> ,
> +                                               jh7110_pinmux_functio
> n(v));
> +               }
>         }
>  
>         return 0;