Re: [PATCH] rockchip: derive GPIO bank from alias if available

[Johan Jonker <jbx6244@gmail.com>]

On 1/17/23 19:15, John Keeping wrote:
> Upstream device trees now use standard node names like "gpio@ff..." but
> the rk_gpio driver expects a name like "gpio0@ff..." (note the index
> before the @).
> 
> This is not a change that can be made in a -u-boot.dtsi file, so
> updating to the latest upstream device trees requires updating the
> driver.
> 
> Other GPIO drivers already use the sequence number to derive the bank.
> This can be set explicitly using an alias in the device tree (which can
> be added in a -u-boot.dtsi file) but Rockchip GPIO banks are defined in
> order in the device tree anyway so when no aliases are supplied the
> indexes should still be correct.
> 
> Note that the bank name is only used by the `gpio` command or
> potentially by board code via dm_gpio_lookup_name() and related
> functions; it is not used by driver code which will lookup GPIOs from
> the device tree by phandle.  No Rockchip platforms make any use of
> dm_gpio_lookup_name() or `gpio` in the boot scripts.
> 
> Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Cc: Johan Jonker <jbx6244@gmail.com>
> Signed-off-by: John Keeping <john@metanate.com>
> ---
>  drivers/gpio/rk_gpio.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
> index 68f30157a9..8568f10cd0 100644
> --- a/drivers/gpio/rk_gpio.c
> +++ b/drivers/gpio/rk_gpio.c
> @@ -142,7 +142,6 @@ static int rockchip_gpio_probe(struct udevice *dev)
>  {
>  	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>  	struct rockchip_gpio_priv *priv = dev_get_priv(dev);
> -	char *end;
>  	int ret;
>  
>  	priv->regs = dev_read_addr_ptr(dev);
> @@ -151,9 +150,8 @@ static int rockchip_gpio_probe(struct udevice *dev)
>  		return ret;
>  
>  	uc_priv->gpio_count = ROCKCHIP_GPIOS_PER_BANK;
> -	end = strrchr(dev->name, '@');
> -	priv->bank = trailing_strtoln(dev->name, end);
> -	priv->name[0] = 'A' + priv->bank;

> +
> +	priv->name[0] = 'A' + dev_seq(dev);

Hi,

Some comments. Have a look if it's useful.
===
1:
Tested with rk3066a mk808 in only full u-boot.

The gpio banks have a number gap.

	aliases {
		gpio0 = &gpio0;
		gpio1 = &gpio1;
		gpio2 = &gpio2;
		gpio3 = &gpio3;
		gpio4 = &gpio4;

		gpio6 = &gpio6;
	};

With aliases gpio6 is called G.
Without gpio6 is called F.
There's no name consistency for command scripts.
===
2:
For example: rk3288-evb-u-boot.dtsi

What happens in tpl or spl with only gpio3 and gpio8 in dt-plat.c.

Or with SPL FDT blob:

[v1,01/17] rockchip: spl: fix reloc gd and FDT blob pointer
http://patchwork.ozlabs.org/project/uboot/patch/20220508150825.21711-2-jbx6244@gmail.com/

Not in use in mainline. Just saying there's more possible in SPL/TPL.
===
The use of aliases is more of something of an additional thing that we not always can count on that is there.

Johan

 

>  	uc_priv->bank_name = priv->name;
>  
>  	return 0;