| Message ID | 20230827203612.173562-2-tmaimon77@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | pinctrl: nuvoton: add pinmux and GPIO driver for NPCM8XX | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 27/08/2023 22:36, 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> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > + '^pin': > + $ref: pincfg-node.yaml# > + > + properties: > + pins: > + description: > + A list of pins to configure in certain ways, such as enabling > + debouncing What pin names are allowed? > + > + 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 > + > +allOf: > + - $ref: pinctrl.yaml# > + > +required: > + - compatible > + - ranges > + - '#address-cells' > + - '#size-cells' > + - nuvoton,sysgcr > + > +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@f0800260 { Nothing improved here. Test your DTS. This is being reported - I checked. > + compatible = "nuvoton,npcm845-pinctrl"; > + ranges = <0x0 0x0 0xf0010000 0x8000>; > + #address-cells = <1>; > + #size-cells = <1>; > + nuvoton,sysgcr = <&gcr>; > + > + gpio0: gpio@0 { > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x0 0xB0>; Keep lowercase hex. Best regards, Krzysztof
Hi Krzysztof, Thanks for your comments On Mon, 28 Aug 2023 at 10:10, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 27/08/2023 22:36, 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> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > > > + '^pin': > > + $ref: pincfg-node.yaml# > > + > > + properties: > > + pins: > > + description: > > + A list of pins to configure in certain ways, such as enabling > > + debouncing > > What pin names are allowed? Do you mean to describe all the allowed pin items? for example: items: pattern: 'GPIO0/IOX1_DI/SMB6C_SDA/SMB18_SDA|GPIO1/IOX1_LD/SMB6C_SCL/SMB18_SCL' or items: pattern: '^GPIO([0-9]|[0-9][0-9]|[1-2][0-4][0-9]|25[0-6])$' is good enough? > > > + > > + 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 > > + > > +allOf: > > + - $ref: pinctrl.yaml# > > + > > +required: > > + - compatible > > + - ranges > > + - '#address-cells' > > + - '#size-cells' > > + - nuvoton,sysgcr > > + > > +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@f0800260 { > > Nothing improved here. Test your DTS. This is being reported - I checked. what do you suggest since the pinctrl doesn't have a reg parameter, maybe pinctrl: pinctrl@0? BTW, I have run both dt_binding_check and W=1 dtbs_check, and didn't see an issue related to the pinctrl: pinctrl@f0800260, do I need to add another flag to see the issue? > > > + compatible = "nuvoton,npcm845-pinctrl"; > > + ranges = <0x0 0x0 0xf0010000 0x8000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + nuvoton,sysgcr = <&gcr>; > > + > > + gpio0: gpio@0 { > > + gpio-controller; > > + #gpio-cells = <2>; > > + reg = <0x0 0xB0>; > > Keep lowercase hex. will be done in V7 > > > Best regards, > Krzysztof > Best regards, Tomer
On 28/08/2023 12:26, Tomer Maimon wrote: > Hi Krzysztof, > > Thanks for your comments > > On Mon, 28 Aug 2023 at 10:10, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 27/08/2023 22:36, 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> >>> Reviewed-by: Rob Herring <robh@kernel.org> >>> --- >> >> >>> + '^pin': >>> + $ref: pincfg-node.yaml# >>> + >>> + properties: >>> + pins: >>> + description: >>> + A list of pins to configure in certain ways, such as enabling >>> + debouncing >> >> What pin names are allowed? > Do you mean to describe all the allowed pin items? > for example: > items: > pattern: > 'GPIO0/IOX1_DI/SMB6C_SDA/SMB18_SDA|GPIO1/IOX1_LD/SMB6C_SCL/SMB18_SCL' > or > items: > pattern: '^GPIO([0-9]|[0-9][0-9]|[1-2][0-4][0-9]|25[0-6])$' > > is good enough? Something like this. Whichever is correct. >> >>> + >>> + bias-disable: true >>> + >>> +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@f0800260 { >> >> Nothing improved here. Test your DTS. This is being reported - I checked. > what do you suggest since the pinctrl doesn't have a reg parameter, > maybe pinctrl: pinctrl@0? It has ranges, so yes @0 looks correct here. Which leds to second question - how pinctrl could have @0? It's already taken by SoC! So your DTS here - unit address and ranges - are clearly wrong. > BTW, I have run both dt_binding_check and W=1 dtbs_check, and didn't > see an issue related to the pinctrl: pinctrl@f0800260, do I need to > add another flag to see the issue? Did you read my message last time? I said - it's about DTS, not the binding. Best regards, Krzysztof
On 28/08/2023 12:36, Krzysztof Kozlowski wrote: > On 28/08/2023 12:26, Tomer Maimon wrote: >> Hi Krzysztof, >> >> Thanks for your comments >> >> On Mon, 28 Aug 2023 at 10:10, Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org> wrote: >>> >>> On 27/08/2023 22:36, 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> >>>> Reviewed-by: Rob Herring <robh@kernel.org> >>>> --- >>> >>> >>>> + '^pin': >>>> + $ref: pincfg-node.yaml# >>>> + >>>> + properties: >>>> + pins: >>>> + description: >>>> + A list of pins to configure in certain ways, such as enabling >>>> + debouncing >>> >>> What pin names are allowed? >> Do you mean to describe all the allowed pin items? >> for example: >> items: >> pattern: >> 'GPIO0/IOX1_DI/SMB6C_SDA/SMB18_SDA|GPIO1/IOX1_LD/SMB6C_SCL/SMB18_SCL' >> or >> items: >> pattern: '^GPIO([0-9]|[0-9][0-9]|[1-2][0-4][0-9]|25[0-6])$' >> >> is good enough? > > Something like this. Whichever is correct. > >>> >>>> + >>>> + bias-disable: true >>>> + > >>>> +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@f0800260 { >>> >>> Nothing improved here. Test your DTS. This is being reported - I checked. >> what do you suggest since the pinctrl doesn't have a reg parameter, >> maybe pinctrl: pinctrl@0? > > It has ranges, so yes @0 looks correct here. Wait, your address according to ranges is 0xf0010000, not 0x0, not 0xf0800260... > Which leds to second > question - how pinctrl could have @0? It's already taken by SoC! So your > DTS here - unit address and ranges - are clearly wrong. > > >> BTW, I have run both dt_binding_check and W=1 dtbs_check, and didn't >> see an issue related to the pinctrl: pinctrl@f0800260, do I need to >> add another flag to see the issue? > > Did you read my message last time? I said - it's about DTS, not the binding. Best regards, Krzysztof
Hi Krzysztof, Thanks for your clarifications On Mon, 28 Aug 2023 at 13:39, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 28/08/2023 12:36, Krzysztof Kozlowski wrote: > > On 28/08/2023 12:26, Tomer Maimon wrote: > >> Hi Krzysztof, > >> > >> Thanks for your comments > >> > >> On Mon, 28 Aug 2023 at 10:10, Krzysztof Kozlowski > >> <krzysztof.kozlowski@linaro.org> wrote: > >>> > >>> On 27/08/2023 22:36, 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> > >>>> Reviewed-by: Rob Herring <robh@kernel.org> > >>>> --- > >>> > >>> > >>>> + '^pin': > >>>> + $ref: pincfg-node.yaml# > >>>> + > >>>> + properties: > >>>> + pins: > >>>> + description: > >>>> + A list of pins to configure in certain ways, such as enabling > >>>> + debouncing > >>> > >>> What pin names are allowed? > >> Do you mean to describe all the allowed pin items? > >> for example: > >> items: > >> pattern: > >> 'GPIO0/IOX1_DI/SMB6C_SDA/SMB18_SDA|GPIO1/IOX1_LD/SMB6C_SCL/SMB18_SCL' > >> or > >> items: > >> pattern: '^GPIO([0-9]|[0-9][0-9]|[1-2][0-4][0-9]|25[0-6])$' > >> > >> is good enough? > > > > Something like this. Whichever is correct. > > > >>> > >>>> + > >>>> + bias-disable: true > >>>> + > > > >>>> +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@f0800260 { > >>> > >>> Nothing improved here. Test your DTS. This is being reported - I checked. > >> what do you suggest since the pinctrl doesn't have a reg parameter, > >> maybe pinctrl: pinctrl@0? > > > > It has ranges, so yes @0 looks correct here. > > Wait, your address according to ranges is 0xf0010000, not 0x0, not > 0xf0800260... I will modify it to pinctrl: pinctrl@f0010000 > > > > Which leds to second > > question - how pinctrl could have @0? It's already taken by SoC! So your > > DTS here - unit address and ranges - are clearly wrong. > > > > > >> BTW, I have run both dt_binding_check and W=1 dtbs_check, and didn't > >> see an issue related to the pinctrl: pinctrl@f0800260, do I need to > >> add another flag to see the issue? > > > > Did you read my message last time? I said - it's about DTS, not the binding. yes, understood doesn't the dtbs_check check the DTS? > > Best regards, > Krzysztof > Best regards, Tomer
On 28/08/2023 13:43, Tomer Maimon wrote: >> >>> Which leds to second >>> question - how pinctrl could have @0? It's already taken by SoC! So your >>> DTS here - unit address and ranges - are clearly wrong. >>> >>> >>>> BTW, I have run both dt_binding_check and W=1 dtbs_check, and didn't >>>> see an issue related to the pinctrl: pinctrl@f0800260, do I need to >>>> add another flag to see the issue? >>> >>> Did you read my message last time? I said - it's about DTS, not the binding. > yes, understood doesn't the dtbs_check check the DTS? Yes, it does, and the first error is being reported (just like several others for your DTS...). The second about gpio node, I think is not. Best regards, Krzysztof
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..a895c60e3eb4 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml @@ -0,0 +1,214 @@ +# 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 + + nuvoton,sysgcr: + $ref: /schemas/types.yaml#/definitions/phandle + description: a phandle to access GCR registers. + +patternProperties: + '^gpio@': + type: object + + description: + Eight GPIO banks that each contain 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 + + '-mux$': + $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, pspi, jm1, jm2, smb4den, smb4b, + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, + smb22, smb23, smb23b, smb4d, smb14, smb5, smb4, smb3, + spi0cs1, spi0cs2, spi0cs3, spi1cs0, spi1cs1, spi1cs2, + spi1cs3, spi1cs23, smb3c, smb3b, bmcuart0a, uart1, jtag2, + bmcuart1, uart2, sg1mdio, 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, 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, smi, 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, pspi, jm1, jm2, smb4den, smb4b, + smb4c, smb15, smb16, smb17, smb18, smb19, smb20, smb21, + smb22, smb23, smb23b, smb4d, smb14, smb5, smb4, smb3, + spi0cs1, spi0cs2, spi0cs3, spi1cs0, spi1cs1, spi1cs2, + spi1cs3, spi1cs23, smb3c, smb3b, bmcuart0a, uart1, jtag2, + bmcuart1, uart2, sg1mdio, 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, 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, smi, 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 + +allOf: + - $ref: pinctrl.yaml# + +required: + - compatible + - ranges + - '#address-cells' + - '#size-cells' + - nuvoton,sysgcr + +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@f0800260 { + compatible = "nuvoton,npcm845-pinctrl"; + ranges = <0x0 0x0 0xf0010000 0x8000>; + #address-cells = <1>; + #size-cells = <1>; + nuvoton,sysgcr = <&gcr>; + + gpio0: gpio@0 { + 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-mux { + groups = "fanin0"; + function = "fanin0"; + }; + + pin34_slew: pin34-slew { + pins = "GPIO34/I3C4_SDA"; + bias-disable; + }; + }; + };