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

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

[John Keeping]

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.

When a sequence number is given explicitly via an alias it makes sense
to use this for the bank number (and aliases can be added via
-u-boot.dtsi when they are not present upstream), so make this the
preferred scheme for assigning a bank index, falling back to the current
method if no alias is defined.

Signed-off-by: John Keeping <john@metanate.com>
---
I'm not sure if it would be better just to use dev_seq(dev)
unconditionally here.  If no aliases are defined, then the device tree
nodes are in the right order for all Rockchip SoCs anyway so the
sequence numbers will be sensible.

But this feels like the change with less risk of unintentionally
introducing a regression.

 drivers/gpio/rk_gpio.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
index 68f30157a9..b2f6d0e925 100644
--- a/drivers/gpio/rk_gpio.c
+++ b/drivers/gpio/rk_gpio.c
@@ -151,8 +151,13 @@ 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);
+
+	ret = dev_read_alias_seq(dev, &priv->bank);
+	if (ret) {
+		end = strrchr(dev->name, '@');
+		priv->bank = trailing_strtoln(dev->name, end);
+	}
+
 	priv->name[0] = 'A' + priv->bank;
 	uc_priv->bank_name = priv->name;
 
-- 
2.37.1

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

[Simon Glass]

Hi John,

On Tue, 26 Jul 2022 at 10:25, John Keeping <john@metanate.com> 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.
>
> When a sequence number is given explicitly via an alias it makes sense
> to use this for the bank number (and aliases can be added via
> -u-boot.dtsi when they are not present upstream), so make this the
> preferred scheme for assigning a bank index, falling back to the current
> method if no alias is defined.
>
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> I'm not sure if it would be better just to use dev_seq(dev)
> unconditionally here.  If no aliases are defined, then the device tree
> nodes are in the right order for all Rockchip SoCs anyway so the
> sequence numbers will be sensible.

Well dev_seq() should now match the alias numbers if provided, so it
seems better.

>
> But this feels like the change with less risk of unintentionally
> introducing a regression.
>
>  drivers/gpio/rk_gpio.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
> index 68f30157a9..b2f6d0e925 100644
> --- a/drivers/gpio/rk_gpio.c
> +++ b/drivers/gpio/rk_gpio.c
> @@ -151,8 +151,13 @@ 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);
> +
> +       ret = dev_read_alias_seq(dev, &priv->bank);
> +       if (ret) {
> +               end = strrchr(dev->name, '@');
> +               priv->bank = trailing_strtoln(dev->name, end);
> +       }
> +
>         priv->name[0] = 'A' + priv->bank;
>         uc_priv->bank_name = priv->name;
>
> --
> 2.37.1
>

Regards,
Simon

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

[John Keeping]

Hi Simon,

On Tue, Jul 26, 2022 at 01:53:44PM -0600, Simon Glass wrote:
> On Tue, 26 Jul 2022 at 10:25, John Keeping <john@metanate.com> 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.
> >
> > When a sequence number is given explicitly via an alias it makes sense
> > to use this for the bank number (and aliases can be added via
> > -u-boot.dtsi when they are not present upstream), so make this the
> > preferred scheme for assigning a bank index, falling back to the current
> > method if no alias is defined.
> >
> > Signed-off-by: John Keeping <john@metanate.com>
> > ---
> > I'm not sure if it would be better just to use dev_seq(dev)
> > unconditionally here.  If no aliases are defined, then the device tree
> > nodes are in the right order for all Rockchip SoCs anyway so the
> > sequence numbers will be sensible.
> 
> Well dev_seq() should now match the alias numbers if provided, so it
> seems better.

Right, when there are aliases dev_seq() is definitely right, but
currently the bank number is derived from a node like:

	gpio0: gpio0@ff750000 {

where the name is "gpio0@..." and it's that zero that is used to
determine the bank.

I don't think all boards define aliases for the GPIOs, so I thought it
might be better to preserve the existing node-name scheme when no alias
is set.  But there seems to be quite a push towards standardised node
names upstream so eventually updating the device trees from upstream
will break this scheme.

Having looked around a bit more, it seems that of the drivers generating
a bank name with 'A'+index, while PIC32 uses the same node name scheme
as Rockchip, both octeon and mvebu use dev_seq(), so it does seem
sensible to just switch to dev_seq() as the right way to do this.


Regards,
John