Hi Johan, On 2023-03-19 12:34, Johan Jonker wrote: > > > On 3/18/23 21:20, Simon Glass wrote: >> Hi Johan, >> >> On Thu, 16 Mar 2023 at 10:46, Johan Jonker <[email protected]> wrote: >>> >>> The current divider to calculate the bank ID can change. >>> Use a constant ROCKCHIP_GPIOS_PER_BANK as fixed divider. >> > >> What is the motivation for this patch? > > The gpio-ranges property format: > > gpio-ranges = <[pin controller phandle], [GPIO controller offset], > [pin controller offset], [number of pins]>; > > 1: Given the Rockchip TRM not all gpio-banks have 32 pins per bank. > 2: The "gpio-ranges" syntax allows multiple items with variable number of > pins.
Is there a reason why gpio-ranges is used to determine bank id? In the series at [1], add support for rk35xx gpio banks, sent yesterday I changed this to rely on the dev alias id, same as in the linux driver. Any reason why using the device alias id won't work for the same purpose in u-boot? [1] https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ Regards, Jonas > > === > > Theoretical example: > > gpio-ranges = <&pinctrl 0 32 32>; // 32/32 => bank 1 > > vs. > > gpio-ranges = <&pinctrl 16 48 16>, // 48/16 => bank 3 vs. 48/32 => bank 1 > <&pinctrl 0 32 16>; // 32/16 => bank 2 vs. 32/32 => bank 1 > > Both descriptions are valid. > The number of pins in the second example is reduced to 16 per item. > Using that as divider will give a wrong bank number. > Use a constant instead. > For the Rockchip situation simple parsing the first item is enough the know > it's bank number for now. > To do it correct one could parse a list of gpio-range items, but that makes > it more complicated. > > From gpio.txt: > Each offset runs from 0 to N. It is perfectly fine to pile any number of > ranges with just one pin-to-GPIO line mapping if the ranges are concocted, but > in practice these ranges are often lumped in discrete sets. > >> >>> >>> Signed-off-by: Johan Jonker <[email protected]> >>> --- >>> drivers/gpio/rk_gpio.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> Reviewed-by: Simon Glass <[email protected]> >> >>> >>> diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c >>> index f7ad4d68..0a2acf18 100644 >>> --- a/drivers/gpio/rk_gpio.c >>> +++ b/drivers/gpio/rk_gpio.c >>> @@ -160,7 +160,7 @@ static int rockchip_gpio_probe(struct udevice *dev) >>> 0, &args); >>> if (!ret || ret != -ENOENT) { >>> uc_priv->gpio_count = args.args[2]; >>> - priv->bank = args.args[1] / args.args[2]; >>> + priv->bank = args.args[1] / ROCKCHIP_GPIOS_PER_BANK; >>> } else { >>> uc_priv->gpio_count = ROCKCHIP_GPIOS_PER_BANK; >>> end = strrchr(dev->name, '@'); >>> -- >>> 2.20.1 >>> >> >> Regards, >> SImon
