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

Hi, Krzysztof

Thanks for the review.

> 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
> >>> <[email protected]> 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
> >>>> [email protected]/
> >>>
> >>> 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.

Okay, I'll revert to the initial approach of using:
gpios = <&gpio offset flags>; 
gpio-ranges = <&pinctrl gpio_offset pin_offset count>;

Best Regards,
Changhuang
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv