On Tue, Jan 17, 2023 at 08:52:22PM +0100, Johan Jonker wrote: > > > 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.sch...@theobroma-systems.com> > > Cc: Johan Jonker <jbx6...@gmail.com> > > Signed-off-by: John Keeping <j...@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); > > 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.
True, but those scripts are going to be board specific and already depend on board data. The name will be consistent per board. > === > 2: > For example: rk3288-evb-u-boot.dtsi > > What happens in tpl or spl with only gpio3 and gpio8 in dt-plat.c. Is the bank name used in tpl/spl at all? There's no script support there and it could only be used in board code. > 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-jbx6...@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. Again this is board specific - if a board wants this scheme of GPIO naming it can enable aliases and rely on that. I have been thinking of another option which is a bigger change to the bank name - it doesn't have to be a letter, could we use the register address as the bank name? So the GPIO would be something like ff750000.1 (although I don't know how the naming scheme works, so having a separator may not work). What do you think of that idea? Regards, John