Message ID | 20220710102110.39748-2-tmaimon77@gmail.com |
---|---|
State | New |
Headers | show |
Series | pinctrl: nuvoton: add pinmux and GPIO driver for NPCM8XX | expand |
Hi Krzysztof, Thanks for your comments. On Tue, 12 Jul 2022 at 12:48, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 10/07/2022 12:21, Tomer Maimon wrote: > > Added device tree binding documentation for Nuvoton Arbel BMC NPCM8XX > > pinmux and GPIO controller. > > > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> > > --- > > .../pinctrl/nuvoton,npcm845-pinctrl.yaml | 205 ++++++++++++++++++ > > 1 file changed, 205 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > > new file mode 100644 > > index 000000000000..6395ef2bf5b3 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > > @@ -0,0 +1,205 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pinctrl/nuvoton,npcm845-pinctrl.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton NPCM845 Pin Controller and GPIO > > + > > +maintainers: > > + - Tomer Maimon <tmaimon77@gmail.com> > > + > > +description: > > + The Nuvoton BMC NPCM8XX Pin Controller multi-function routed through > > + the multiplexing block, Each pin supports GPIO functionality (GPIOx) > > + and multiple functions that directly connect the pin to different > > + hardware blocks. > > + > > +properties: > > + compatible: > > + const: nuvoton,npcm845-pinctrl > > + > > + ranges: > > + maxItems: 1 > > ranges without reg? Does it even work? Did you test the bindings? The ranges related to GPIO node reg I did test the pin controller document and it passed. bash-4.2$ make ARCH=arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dts DTC Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb CHECK Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb Did I need to run anything else than dt_binding_check for testing the document? > > > + > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 1 > > + > > +patternProperties: > > + "^gpio@": > > + type: object > > + > > + description: > > + Eight GPIO banks that each contain between 32 GPIOs. > > + > > + properties: > > + > > No blank line. O.K. > > > + gpio-controller: true > > + > > + '#gpio-cells': > > + const: 2 > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + gpio-ranges: > > + maxItems: 1 > > + > > + required: > > + - gpio-controller > > + - '#gpio-cells' > > + - reg > > + - interrupts > > + - gpio-ranges > > + > > + "-pin": > > + $ref: pinmux-node.yaml# > > Shouldn't this be under bank? Do you mean after the group and function properties? The -pin shouldn't use for the group property naming? > > > + > > + properties: > > + groups: > > + description: > > + One or more groups of pins to mux to a certain function > > + items: > > + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, > > + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, > > + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, > > + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, > > + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, > > + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, > > + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, > > + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, > > + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, > > + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, > > + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, > > + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, > > + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, > > + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, > > + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, > > + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, > > + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, > > + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, > > + hgpio4, hgpio5, hgpio6, hgpio7 ] > > + > > + function: > > + description: > > + The function that a group of pins is muxed to > > + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, > > + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, > > + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, > > + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, > > + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, > > + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, > > + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, > > + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, > > + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, > > + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, > > + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, > > + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, > > + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, > > + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, > > + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, > > + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, > > + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, > > + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, > > + hgpio4, hgpio5, hgpio6, hgpio7 ] > > + > > + dependencies: > > + groups: [ function ] > > + function: [ groups ] > > + > > + additionalProperties: false > > + > > + "^pin": > > This is almost the same as previous property. Confusing and I think it > does not work. if I remove it I get the following error: pinctrl@f0800000: 'pin34-slew' does not match any of the regexes: '-pin', '^gpio@', 'pinctrl-[0-9]+' Can you advise what I should do? > > > + $ref: pincfg-node.yaml# > > + > > + properties: > > + pins: > > + description: > > + A list of pins to configure in certain ways, such as enabling > > + debouncing > > + > > + bias-disable: true > > + > > + bias-pull-up: true > > + > > + bias-pull-down: true > > + > > + input-enable: true > > + > > + output-low: true > > + > > + output-high: true > > + > > + drive-push-pull: true > > + > > + drive-open-drain: true > > + > > + input-debounce: > > + description: > > + Debouncing periods in microseconds, one period per interrupt > > + bank found in the controller > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 1 > > + maxItems: 4 > > + > > + slew-rate: > > + description: | > > + 0: Low rate > > + 1: High rate > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + > > + drive-strength: > > + enum: [ 0, 1, 2, 4, 8, 12 ] > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - ranges > > + - '#address-cells' > > + - '#size-cells' > > Missing allOf with ref to pinctrl.yaml. Do you mean adding allOf: - $ref: "pinctrl.yaml#" > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/gpio/gpio.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + pinctrl: pinctrl@f0800000 { > > + compatible = "nuvoton,npcm845-pinctrl"; > > + ranges = <0x0 0x0 0xf0010000 0x8000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + gpio0: gpio@f0010000 { > > + gpio-controller; > > + #gpio-cells = <2>; > > + reg = <0x0 0xB0>; > > + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; > > + gpio-ranges = <&pinctrl 0 0 32>; > > + }; > > + > > + fanin0_pin: fanin0-pin { > > + groups = "fanin0"; > > + function = "fanin0"; > > + }; > > + > > + pin34_slew: pin34-slew { > > and how does it pass your checks? Yes > > Did you test the bindings? > > > > Best regards, > Krzysztof Best regards, Tomer
Hi Krzysztof, Thanks for your clarifications. On Tue, 12 Jul 2022 at 16:45, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 12/07/2022 15:29, Tomer Maimon wrote: > > Hi Krzysztof, > > > > Thanks for your comments. > > > > On Tue, 12 Jul 2022 at 12:48, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 10/07/2022 12:21, Tomer Maimon wrote: > >>> Added device tree binding documentation for Nuvoton Arbel BMC NPCM8XX > >>> pinmux and GPIO controller. > >>> > >>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> > >>> --- > >>> .../pinctrl/nuvoton,npcm845-pinctrl.yaml | 205 ++++++++++++++++++ > >>> 1 file changed, 205 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>> new file mode 100644 > >>> index 000000000000..6395ef2bf5b3 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>> @@ -0,0 +1,205 @@ > >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,npcm845-pinctrl.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Nuvoton NPCM845 Pin Controller and GPIO > >>> + > >>> +maintainers: > >>> + - Tomer Maimon <tmaimon77@gmail.com> > >>> + > >>> +description: > >>> + The Nuvoton BMC NPCM8XX Pin Controller multi-function routed through > >>> + the multiplexing block, Each pin supports GPIO functionality (GPIOx) > >>> + and multiple functions that directly connect the pin to different > >>> + hardware blocks. > >>> + > >>> +properties: > >>> + compatible: > >>> + const: nuvoton,npcm845-pinctrl > >>> + > >>> + ranges: > >>> + maxItems: 1 > >> > >> ranges without reg? Does it even work? Did you test the bindings? > > The ranges related to GPIO node reg > > But you do not allow here a 'reg', do you? So how can you have an unit > address in pinctrl node? I allow the reg unit address in the GPIO node. This is why reg is in the GPIO node as follow: compatible = "nuvoton,npcm845-pinctrl"; ranges = <0x0 0x0 0xf0010000 0x8000>; #address-cells = <1>; #size-cells = <1>; status = "okay"; gpio0: gpio@f0010000 { gpio-controller; #gpio-cells = <2>; reg = <0x0 0xB0>; interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; gpio-ranges = <&pinctrl 0 0 32>; }; gpio1: gpio@f0011000 { gpio-controller; #gpio-cells = <2>; reg = <0x1000 0xB0>; interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>; gpio-ranges = <&pinctrl 0 32 32>; }; gpio2: gpio@f0012000 { gpio-controller; #gpio-cells = <2>; reg = <0x2000 0xB0>; interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; gpio-ranges = <&pinctrl 0 64 32>; }; ... Is it problematic? > > > > > I did test the pin controller document and it passed. > > bash-4.2$ make ARCH=arm64 dt_binding_check > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > > LINT Documentation/devicetree/bindings > > CHKDT Documentation/devicetree/bindings/processed-schema.json > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > DTEX Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dts > > DTC Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb > > CHECK Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb > > Did I need to run anything else than dt_binding_check for testing the document? > > Indeed it will pass, because you do not have reg in pinctrl node. But > your dts won't pass make dtbs W=1 After running make ARCH=arm64 dtbs W=1 I don't see warning related to pinctrl bash-4.2$ make ARCH=arm64 dtbs W=1 DTC arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dtb arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5: Warning (unit_address_vs_reg): /ahb/apb: node has a reg or ranges property, but no unit name arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts:20.9-22.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5: Warning (simple_bus_reg): /ahb/apb: simple-bus unit address format error, expected "f0000000" arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:56.35-61.5: Warning (unique_unit_address): /ahb/reset-controller@f0801000: duplicate unit-address (also used in node /ahb/clock-controller@f0801000) I did got warning but it dont related to the pinctrl, Maybe I didn't run the test correct? > > > >> > >>> + > >>> + '#address-cells': > >>> + const: 1 > >>> + > >>> + '#size-cells': > >>> + const: 1 > >>> + > >>> +patternProperties: > >>> + "^gpio@": > >>> + type: object > >>> + > >>> + description: > >>> + Eight GPIO banks that each contain between 32 GPIOs. > >>> + > >>> + properties: > >>> + > >> > >> No blank line. > > O.K. > >> > >>> + gpio-controller: true > >>> + > >>> + '#gpio-cells': > >>> + const: 2 > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + interrupts: > >>> + maxItems: 1 > >>> + > >>> + gpio-ranges: > >>> + maxItems: 1 > >>> + > >>> + required: > >>> + - gpio-controller > >>> + - '#gpio-cells' > >>> + - reg > >>> + - interrupts > >>> + - gpio-ranges > >>> + > >>> + "-pin": > >>> + $ref: pinmux-node.yaml# > >> > >> Shouldn't this be under bank? > > Do you mean after the group and function properties? > > The -pin shouldn't use for the group property naming? > > Hm, I guess it's fine, I actually don't remember the recommendation for > gpio banks in relation to pinmux nodes. > > >> > >>> + > >>> + properties: > >>> + groups: > >>> + description: > >>> + One or more groups of pins to mux to a certain function > >>> + items: > >>> + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, > >>> + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, > >>> + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, > >>> + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, > >>> + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, > >>> + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, > >>> + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, > >>> + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, > >>> + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, > >>> + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, > >>> + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, > >>> + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, > >>> + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, > >>> + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, > >>> + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, > >>> + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, > >>> + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, > >>> + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, > >>> + hgpio4, hgpio5, hgpio6, hgpio7 ] > >>> + > >>> + function: > >>> + description: > >>> + The function that a group of pins is muxed to > >>> + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, > >>> + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, > >>> + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, > >>> + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, > >>> + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, > >>> + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, > >>> + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, > >>> + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, > >>> + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, > >>> + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, > >>> + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, > >>> + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, > >>> + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, > >>> + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, > >>> + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, > >>> + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, > >>> + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, > >>> + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, > >>> + hgpio4, hgpio5, hgpio6, hgpio7 ] > >>> + > >>> + dependencies: > >>> + groups: [ function ] > >>> + function: [ groups ] > >>> + > >>> + additionalProperties: false > >>> + > >>> + "^pin": > >> > >> This is almost the same as previous property. Confusing and I think it > >> does not work. > > if I remove it I get the following error: > > pinctrl@f0800000: 'pin34-slew' does not match any of the regexes: > > '-pin', '^gpio@', 'pinctrl-[0-9]+' > > Can you advise what I should do? > > Ah, the pattern is indeed different - you start with pin. Anyway it's > confusing to have cfg starting with pin and mux ending in pin. How > "pin-pin" would work? :) > > Use maybe similar pattern, so start with mux for mux and pin for cfg. > Look at wpcm450 pinctrl. It indeed confusing, I will work with different naming. > > > >> > >>> + $ref: pincfg-node.yaml# > >>> + > >>> + properties: > >>> + pins: > >>> + description: > >>> + A list of pins to configure in certain ways, such as enabling > >>> + debouncing > >>> + > >>> + bias-disable: true > >>> + > >>> + bias-pull-up: true > >>> + > >>> + bias-pull-down: true > >>> + > >>> + input-enable: true > >>> + > >>> + output-low: true > >>> + > >>> + output-high: true > >>> + > >>> + drive-push-pull: true > >>> + > >>> + drive-open-drain: true > >>> + > >>> + input-debounce: > >>> + description: > >>> + Debouncing periods in microseconds, one period per interrupt > >>> + bank found in the controller > >>> + $ref: /schemas/types.yaml#/definitions/uint32-array > >>> + minItems: 1 > >>> + maxItems: 4 > >>> + > >>> + slew-rate: > >>> + description: | > >>> + 0: Low rate > >>> + 1: High rate > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + enum: [0, 1] > >>> + > >>> + drive-strength: > >>> + enum: [ 0, 1, 2, 4, 8, 12 ] > >>> + > >>> + additionalProperties: false > >>> + > >>> +required: > >>> + - compatible > >>> + - ranges > >>> + - '#address-cells' > >>> + - '#size-cells' > >> > >> Missing allOf with ref to pinctrl.yaml. > > Do you mean adding > > allOf: > > - $ref: "pinctrl.yaml#" > > Yes. > > > > Best regards, > Krzysztof Best regards, Tomer
Hi Krzysztof, Thanks for your clarifications. On Tue, 12 Jul 2022 at 23:44, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 12/07/2022 20:44, Tomer Maimon wrote: > > Hi Krzysztof, > > > > Thanks for your clarifications. > > > > On Tue, 12 Jul 2022 at 16:45, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 12/07/2022 15:29, Tomer Maimon wrote: > >>> Hi Krzysztof, > >>> > >>> Thanks for your comments. > >>> > >>> On Tue, 12 Jul 2022 at 12:48, Krzysztof Kozlowski > >>> <krzysztof.kozlowski@linaro.org> wrote: > >>>> > >>>> On 10/07/2022 12:21, Tomer Maimon wrote: > >>>>> Added device tree binding documentation for Nuvoton Arbel BMC NPCM8XX > >>>>> pinmux and GPIO controller. > >>>>> > >>>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> > >>>>> --- > >>>>> .../pinctrl/nuvoton,npcm845-pinctrl.yaml | 205 ++++++++++++++++++ > >>>>> 1 file changed, 205 insertions(+) > >>>>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>>>> new file mode 100644 > >>>>> index 000000000000..6395ef2bf5b3 > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>>>> @@ -0,0 +1,205 @@ > >>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > >>>>> +%YAML 1.2 > >>>>> +--- > >>>>> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,npcm845-pinctrl.yaml# > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>>> + > >>>>> +title: Nuvoton NPCM845 Pin Controller and GPIO > >>>>> + > >>>>> +maintainers: > >>>>> + - Tomer Maimon <tmaimon77@gmail.com> > >>>>> + > >>>>> +description: > >>>>> + The Nuvoton BMC NPCM8XX Pin Controller multi-function routed through > >>>>> + the multiplexing block, Each pin supports GPIO functionality (GPIOx) > >>>>> + and multiple functions that directly connect the pin to different > >>>>> + hardware blocks. > >>>>> + > >>>>> +properties: > >>>>> + compatible: > >>>>> + const: nuvoton,npcm845-pinctrl > >>>>> + > >>>>> + ranges: > >>>>> + maxItems: 1 > >>>> > >>>> ranges without reg? Does it even work? Did you test the bindings? > >>> The ranges related to GPIO node reg > >> > >> But you do not allow here a 'reg', do you? So how can you have an unit > >> address in pinctrl node? > > I allow the reg unit address in the GPIO node. > > This is why reg is in the GPIO node as follow: > > > > compatible = "nuvoton,npcm845-pinctrl"; > > ranges = <0x0 0x0 0xf0010000 0x8000>; > > #address-cells = <1>; > > #size-cells = <1>; > > status = "okay"; > > gpio0: gpio@f0010000 { > > gpio-controller; > > #gpio-cells = <2>; > > reg = <0x0 0xB0>; > > interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; > > gpio-ranges = <&pinctrl 0 0 32>; > > }; > > gpio1: gpio@f0011000 { > > gpio-controller; > > #gpio-cells = <2>; > > reg = <0x1000 0xB0>; > > interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>; > > gpio-ranges = <&pinctrl 0 32 32>; > > }; > > gpio2: gpio@f0012000 { > > gpio-controller; > > #gpio-cells = <2>; > > reg = <0x2000 0xB0>; > > interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; > > gpio-ranges = <&pinctrl 0 64 32>; > > }; > > ... > > Is it problematic? > > > It seems not, looks ok because of ranges, although it is a bit confusing > that your pinctrl unit address is 0xf0800000 but ranges is 0xf0010000. The reason the pinctrl address 0xf0800000 because the pin mux is handled by the GCR registers and the ranges related to the GPIO. > > >> > >>> > >>> I did test the pin controller document and it passed. > >>> bash-4.2$ make ARCH=arm64 dt_binding_check > >>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml > >>> LINT Documentation/devicetree/bindings > >>> CHKDT Documentation/devicetree/bindings/processed-schema.json > >>> SCHEMA Documentation/devicetree/bindings/processed-schema.json > >>> DTEX Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dts > >>> DTC Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb > >>> CHECK Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb > >>> Did I need to run anything else than dt_binding_check for testing the document? > >> > >> Indeed it will pass, because you do not have reg in pinctrl node. But > >> your dts won't pass make dtbs W=1 > > After running make ARCH=arm64 dtbs W=1 I don't see warning related to pinctrl > > bash-4.2$ make ARCH=arm64 dtbs W=1 > > DTC arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dtb > > arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5: > > Warning (unit_address_vs_reg): /ahb/apb: node has a reg or ranges > > property, but no unit name > > arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts:20.9-22.4: Warning > > (unit_address_vs_reg): /memory: node has a reg or ranges property, but > > no unit name > > arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5: > > Warning (simple_bus_reg): /ahb/apb: simple-bus unit address format > > error, expected "f0000000" > > arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:56.35-61.5: > > Warning (unique_unit_address): /ahb/reset-controller@f0801000: > > duplicate unit-address (also used in node > > /ahb/clock-controller@f0801000) > > I did got warning but it dont related to the pinctrl, Maybe I didn't > > run the test correct? > > Looks correct, indeed. > > Best regards, > Krzysztof Best regards, Tomer
diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml new file mode 100644 index 000000000000..6395ef2bf5b3 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml @@ -0,0 +1,205 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nuvoton,npcm845-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NPCM845 Pin Controller and GPIO + +maintainers: + - Tomer Maimon <tmaimon77@gmail.com> + +description: + The Nuvoton BMC NPCM8XX Pin Controller multi-function routed through + the multiplexing block, Each pin supports GPIO functionality (GPIOx) + and multiple functions that directly connect the pin to different + hardware blocks. + +properties: + compatible: + const: nuvoton,npcm845-pinctrl + + ranges: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + +patternProperties: + "^gpio@": + type: object + + description: + Eight GPIO banks that each contain between 32 GPIOs. + + properties: + + gpio-controller: true + + '#gpio-cells': + const: 2 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + gpio-ranges: + maxItems: 1 + + required: + - gpio-controller + - '#gpio-cells' + - reg + - interrupts + - gpio-ranges + + "-pin": + $ref: pinmux-node.yaml# + + properties: + groups: + description: + One or more groups of pins to mux to a certain function + items: + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, + hgpio4, hgpio5, hgpio6, hgpio7 ] + + function: + description: + The function that a group of pins is muxed to + enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, + smb5b, smb5c, lkgpo0, pspi2, jm1, jm2, smb4den, smb4b, + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, + smb22, smb23, smb4d, smb14, smb5, smb4, smb3, spi0cs1, + spi0cs2, spi0cs3, smb3c, smb3b, bmcuart0a, uart1, jtag2, + bmcuart1, uart2, bmcuart0b, r1err, r1md, r1oen, r2oen, + rmii3, r3oen, smb3d, fanin0, fanin1, fanin2, fanin3, fanin4, + fanin5, fanin6, fanin7, fanin8, fanin9, fanin10, fanin11, + fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, + r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg1, + rg1mdio, rg2, ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, + smb0, smb1, smb2, smb2c, smb2b, smb1c, smb1b, smb8, smb9, + smb10, smb11, sd1, sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, + pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, + serirq, lpcclk, scipme, sci, smb6, smb7, spi1, faninx, r1, + spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, + smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, + smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, + hgpio4, hgpio5, hgpio6, hgpio7 ] + + dependencies: + groups: [ function ] + function: [ groups ] + + additionalProperties: false + + "^pin": + $ref: pincfg-node.yaml# + + properties: + pins: + description: + A list of pins to configure in certain ways, such as enabling + debouncing + + bias-disable: true + + bias-pull-up: true + + bias-pull-down: true + + input-enable: true + + output-low: true + + output-high: true + + drive-push-pull: true + + drive-open-drain: true + + input-debounce: + description: + Debouncing periods in microseconds, one period per interrupt + bank found in the controller + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 1 + maxItems: 4 + + slew-rate: + description: | + 0: Low rate + 1: High rate + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + + drive-strength: + enum: [ 0, 1, 2, 4, 8, 12 ] + + additionalProperties: false + +required: + - compatible + - ranges + - '#address-cells' + - '#size-cells' + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/gpio/gpio.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + pinctrl: pinctrl@f0800000 { + compatible = "nuvoton,npcm845-pinctrl"; + ranges = <0x0 0x0 0xf0010000 0x8000>; + #address-cells = <1>; + #size-cells = <1>; + + gpio0: gpio@f0010000 { + gpio-controller; + #gpio-cells = <2>; + reg = <0x0 0xB0>; + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; + gpio-ranges = <&pinctrl 0 0 32>; + }; + + fanin0_pin: fanin0-pin { + groups = "fanin0"; + function = "fanin0"; + }; + + pin34_slew: pin34-slew { + pins = "GPIO34/I3C4_SDA"; + bias-disable; + }; + }; + }; +
Added device tree binding documentation for Nuvoton Arbel BMC NPCM8XX pinmux and GPIO controller. Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> --- .../pinctrl/nuvoton,npcm845-pinctrl.yaml | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml