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

[Johan Jonker]

On 3/20/23 02:32, Kever Yang wrote:
> Hi Johan,
> 
> On 2023/3/19 19: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.
> 

> Could you share which TRM did you find gpio-banks is not 32 pins?

px2==rk3066:
Rockchip PX2 TRM V1.0.pdf page 20
GPIO
6 groups of GPIO (GPIO0~GPIO4, GPIO6) , 32 GPIOs per group in
GPIO0~GPIO4, and 16 GPIOs in GPIO6, totally have 176 GPIOs

gpio6 is incomplete, mostly only A+B registers but never 32 pin.

===
rk3288-chapter-01-introduction.pdf page 15
GPIO
Totally 160 GPIOs

gpio0 only A+B+C 

gpio8 only A+B

===
rk3066 :
16 : https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/rockchip/pinctrl-rk3066.c#L83

rk3288:
24: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/rockchip/pinctrl-rk3288.c#L185
16: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/rockchip/pinctrl-rk3288.c#L213
> 
> The design should be 32 pins per bank for all the SoCs, although some banks may not have full 32 pins,


> 
> but all the pin ID should follow the rules with 32pin per bank.

That's correct. This patch does not break that formula. Just don't use args[2].

number of pins != divider

> 
> 
> Thanks,
> 
> - Kever
> 
>> 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