[PATCH v1 1/6] rockchip: gpio: rk_gpio: use ROCKCHIP_GPIOS_PER_BANK as divider

[Jonas Karlman]

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 <jbx6244 at gmail.com> 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/20230318235651.826148-3-jonas@kwiboo.se/

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 <jbx6244 at gmail.com>
>>> ---
>>>  drivers/gpio/rk_gpio.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>>>
>>> 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