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

[Johan Jonker]

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.

===

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