Re: [PATCH v5 03/21] dt-bindings: pinctrl: Add starfive,jhb100-sys0-pinctrl

[Krzysztof Kozlowski <krzk@kernel.org>]

On 29/08/2026 05:08, Changhuang Liang wrote:
> Hi, Rob, Krzysztof, Conor
> 
>> On Mon, Aug 17, 2026 at 02:45:22PM +0200, Linus Walleij wrote:
>>> On Fri, Aug 14, 2026 at 1:22 PM Changhuang Liang
>>> <changhuang.liang@starfivetech.com> wrote:
>>>
>>>>> You still did not explain the cells - neither in reply nor in
>>>>> commit - thus start with that. Why do you need it? Multiple
>>>>> controllers have groupped pins or GPIOs in registers, so explanation
>> "every 32 GPIOs correspond to one register"
>>>>> tells me nothing. Why would that matter how many GPIOs is in one
>> register?
>>>>
>>>> Since every 32 GPIOs correspond to one register, Linus suggested at the
>> time that I could try using:
>>>> gpios = <&gpio [instance] offset flags>; gpio-ranges = <&pinctrl
>>>> [gpio_instance] gpio_offset pin_offset count>;
>>>>
>>>> The advantage of using this is that in a GPIO controller with more
>>>> than 32 GPIOs, every 32 GPIOs can be treated as one GPIO bank. This
>>>> way, in the driver, converting pin_offset to gpio_offset does not require a
>> large number of /32 and %32 operations.
>>>>
>>>> the following is Linus's original suggestion at the time
>>>>
>> https://lore.kernel.org/all/CAD++jL=tvDBWJHRGGJw4nnox_kcJKn5hwvwz9zx
>>>> adMRedyvRhw@mail.gmail.com/
>>>
>>> Just copy this explanation into the commit so it is clear why you need
>>> three cells for this.
>>
>> "Can use gpiolib" is not really justification. Neither is avoiding a shift and/or
>> mask. I suppose there's some h/w attribute here to justify it?
> 
> I'll go ahead and send out a new version per Linus's suggestion. In the meantime, 
> if anyone has any additional thoughts or further feedback, I'd be more than happy 
> to incorporate them. Please don't hesitate to let me know!

You got feedback... having things organized per registers does not make
them a bank.

Best regards,
Krzysztof