[PATCH v2 04/10] pinctrl: rockchip: Add pinmux status related ops

[Quentin Schulz]

Hi Jonas,

On 8/3/24 12:56 AM, Jonas Karlman wrote:
> Add get_pins_count(), get_pin_name() and get_pin_muxing() ops to support
> the pinmux status cmd.
> 
>    => pinmux dev pinctrl
>    dev: pinctrl
>    => pinmux status
>    GPIO0_A0  : gpio
>    GPIO0_A1  : func-1
>    GPIO0_A2  : gpio
>    GPIO0_A3  : gpio
>    GPIO0_A4  : func-1
>    GPIO0_A5  : gpio
>    GPIO0_A6  : gpio
>    GPIO0_A7  : func-1
>    GPIO0_B0  : gpio
>    GPIO0_B1  : func-1
>    GPIO0_B2  : func-1
>    GPIO0_B3  : gpio
>    [...]
> 
> The change to use ENOENT for unrouted pins also help hide a "Error -22"
> message for unrouted pins using the gpio status -a cmd.
> 
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang at rock-chips.com>
> ---
> v2: Collect r-b tag
> ---
>   .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 50 ++++++++++++++++++-
>   1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> index 8ede74da40c9..345e0abdf5d1 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> @@ -126,7 +126,7 @@ static int rockchip_get_mux(struct rockchip_pin_bank *bank, int pin)
>   
>   	if (bank->iomux[iomux_num].type & IOMUX_UNROUTED) {
>   		debug("pin %d is unrouted\n", pin);
> -		return -EINVAL;
> +		return -ENOENT;

Rather ENODEV or ENXIO according to the comments in 
https://elixir.bootlin.com/u-boot/v2024.07/source/include/linux/errno.h?

>   	}
>   
>   	if (bank->iomux[iomux_num].type & IOMUX_GPIO_ONLY)
> @@ -193,6 +193,32 @@ static struct rockchip_pin_bank *rockchip_pin_to_bank(struct udevice *dev,
>   	return NULL;
>   }
>   
> +static int rockchip_pinctrl_get_pins_count(struct udevice *dev)
> +{
> +	struct rockchip_pinctrl_priv *priv = dev_get_priv(dev);
> +	struct rockchip_pin_ctrl *ctrl = priv->ctrl;
> +
> +	return ctrl->nr_pins;
> +}
> +
> +static const char *rockchip_pinctrl_get_pin_name(struct udevice *dev,
> +						 unsigned int selector)
> +{
> +	static char name[PINNAME_SIZE];
> +	struct rockchip_pin_bank *bank;
> +	unsigned int index;
> +
> +	bank = rockchip_pin_to_bank(dev, selector);
> +	if (!bank)
> +		return NULL;
> +
> +	index = selector - bank->pin_base;
> +	snprintf(name, sizeof(name), "GPIO%u_%c%u",
> +		 bank->bank_num, 'A' + (index / 8), index % 8);
> +

So before I forget, I think we have an issue on PX30 (unrelated to this 
patch) because GPIO0 is said to have 32 pins but it doesn't, it has 24, 
c.f. the TRM where GPIO0 doesn't have a D "sub-bank". I'll have to check 
the Linux kernel driver(s) too, but considering we've been using sysfs 
for setting GPIOs on PX30 for a while in the kernel, I assume somehow 
this is handled properly (or just luck).

I'm also wondering if the name shouldn't rather be the one coming from 
gpio-line-names too? This probably is more for the gpio command than the 
pinctrl one though I assume? I guess we could also tackle this in the 
future if someone would like to have this instead.

Cheers,
Quentin