Re: [PATCH v5 03/21] dt-bindings: pinctrl: Add starfive,jhb100-sys0-pinctrl
Changhuang Liang <[email protected]>
| Newsgroups | org.kernel.vger.linux-gpio,org.infradead.lists.linux-riscv,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <ZQ4PR01MB12022F6D6F71542033CF857BF2DA2@ZQ4PR01MB1202.CHNPR01.prod.partner.outlook.cn> |
Hi, Krzysztof Thanks for the feedback. > On Fri, Aug 14, 2026 at 01:46:03AM +0000, Changhuang Liang wrote: > > Hi, Rob, Conor, Linus > > > > > HI, Rob > > > > > > Thanks for the review. > > > > > > > On Mon, Aug 10, 2026 at 09:51:07AM +0000, Changhuang Liang wrote: > > > > > Hi, Rob > > > > > > > > > > > On Sun, 09 Aug 2026 23:36:32 -0700, Changhuang Liang wrote: > > > > > > > Add pinctrl bindings for StarFive JHB100 SoC System-0(sys0) > > > > > > > pinctrl controller. > > > > > > > > > > > > > > Signed-off-by: Changhuang Liang > > > > > > > <[email protected]> > > > > > > > --- > > > > > > > .../pinctrl/starfive,jhb100-sys0-pinctrl.yaml | 164 > > > > ++++++++++++++++++ > > > > > > > .../pinctrl/starfive,jhb100-pinctrl.h | 17 ++ > > > > > > > 2 files changed, 181 insertions(+) create mode 100644 > > > > > > > Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sy > > > > > > > s0-p > > > > > > > in > > > > > > > ctrl > > > > > > > .yaml create mode 100644 > > > > > > > include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h > > > > > > > > > > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > > > > > > > yamllint warnings/errors: > > > > > > > > > > > > dtschema/dtc warnings/errors: > > > > > > Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0 > > > > > > -pin > > > > > > ct > > > > > > rl.exa > > > > > > mple.dtb: pinctrl@13080000 (starfive,jhb100-sys0-pinctrl): > > > > > > gpio-ranges:1: [4] is too short > > > > > > from schema $id: > http://devicetree.org/schemas/gpio/gpio.yaml > > > > > > > > > > > > doc reference errors (make refcheckdocs): > > > > > > > > > > > > See > > > > > > https://patchwork.kernel.org/project/devicetree/patch/20260810 > > > > > > 0636 > > > > > > 50 > > > > > > .23 [email protected] > > > > > > > > > > > > The base for the series is generally the latest rc1. A > > > > > > different dependency should be noted in *this* patch. > > > > > > > > > > > > If you already ran 'make dt_binding_check' and didn't see the > > > > > > above error(s), then make sure 'yamllint' is installed and > > > > > > dt-schema is up to > > > > > > date: > > > > > > > > > > > > pip3 install dtschema --upgrade > > > > > > > > > > > > Please check and re-submit after running the above command > yourself. > > > > > > Note that DT_SCHEMA_FILES can be set to your schema file to > > > > > > speed up checking your schema. However, it must be unset to > > > > > > test all examples with your schema. > > > > > > > > > > Hi Rob, > > > > > > > > > > Regarding this link: > > > > > > > > > > > > > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schem > > > a > > > > s > > > > > /gpio/gpio.yaml#L71 > > > > > > > > > > It seems that the configuration to support #gpio-cells = <3>; > > > > > has not been > > > > added here yet, is that correct? > > > > > > > > Shrug. Based on the description, gpio-ranges doesn't depend the # of > cells. > > > > > > > > Why do you need 3 cells? You haven't described the cell content. > > > > > > For the pinctrl of JHB100, every 32 GPIOs correspond to one > > > register, so it uses 3-cell #gpio-cells and 4-cell gpio-ranges, which helps > simplify the driver code. > > > > > > I referred to this generic implementation: > > > https://elixir.bootlin.com/linux/v7.2-rc6/source/drivers/gpio/gpioli > > > b-of.c#L93 > > > 6 > > > > > > So the content of gpio cells and gpio-ranges is as follows: > > > gpios = <&gpio [instance] offset flags>; gpio-ranges = <&pinctrl > > > [gpio_instance] gpio_offset pin_offset count>; > > > > > > I'm not sure whether this error is caused by a mismatch with the > > > definition in > > > schemas/gpio/gpio.yaml: > > > > > > gpio-ranges: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > items: > > > items: > > > - description: pin controller phandle > > > - description: GPIO controller offset > > > - description: pin controller offset > > > - description: number of pins > > > > > > Do you have any suggestions for fixing this? Thank you very much. > > > > > > Best Regards, > > > Changhuang > > > > For these configurations: > > gpios = <&gpio [instance] offset flags>; gpio-ranges = <&pinctrl > > [gpio_instance] gpio_offset pin_offset count>; > > > > I tried using spacemit,k1-gpio for comparison. Currently, the gpio-ranges > configuration under K1 is as follows, and it does not report any errors: > > > > gpio: gpio@d4019000 { > > compatible = "spacemit,k1-gpio"; > > gpio-ranges = <&pinctrl 0 0 0 32>, > > <&pinctrl 1 0 32 32>, > > <&pinctrl 2 0 64 32>, > > <&pinctrl 3 0 96 32>; }; > > > > Then I reduced it by removing one set of entries, changing it to: > > > > gpio: gpio@d4019000 { > > compatible = "spacemit,k1-gpio"; > > gpio-ranges = <&pinctrl 0 0 0 32>, > > <&pinctrl 1 0 32 32>, > > <&pinctrl 2 0 64 32>; }; > > > > At this point, I see a similar error: > > > > /linux/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: gpio@d4019000 > (spacemit,k1-gpio): gpio-ranges:3: [0, 64, 32] is too short > > from schema $id: http://devicetree.org/schemas/gpio/gpio.yaml > > DTC [C] arch/riscv/boot/dts/spacemit/k1-milkv-jupiter.dtb > > > > My understanding is that the first configuration has 5×4 elements, but the > dt-schema seems to treat it as 4×5 elements. > > In that case, it still aligns with 4 elements per group, so no error > > is reported. However, when I changed it to 5×3 elements, it no longer aligns > with 4 elements per group, which is why the error appears. > > > > I'm not sure if my understanding is correct. Does anyone have any good > > suggestions on how to fix this error? I'm currently unsure about what the > next steps should be." > > 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_kcJKn5hwvwz9zxadMRedyvRhw@mail.gmail.com/ Best Regards, Changhuang