Messages in this thread | | | Date | Tue, 28 Nov 2023 16:29:53 +0800 | Subject | Re: [PATCH v2 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control | From | Jacky Huang <> |
| |
Dear Krzysztof,
Thanks for your review.
On 2023/11/28 下午 03:34, Krzysztof Kozlowski wrote: > On 28/11/2023 07:11, Jacky Huang wrote: >> From: Jacky Huang <ychuang3@nuvoton.com> >> >> Add documentation to describe nuvoton ma35d1 pin control and GPIO. >> >> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >> --- > Your changelog said: > >> - Remove ma35d1-pinfunc.h which is unused after update definition of > 'nuvoton,pins'. > > You forgot to add: > > " - Do not test the bindings before sending" > > I assume none of the driver changes compile either.
It's my mistake. I forgot to remove 'ma35d1-pinfunc.h' from my local copy, and as a consequence, the 'dt_binding_check' did not catch this error. I will fix this. >> .../pinctrl/nuvoton,ma35d1-pinctrl.yaml | 189 ++++++++++++++++++ >> 1 file changed, 189 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> new file mode 100644 >> index 000000000000..84287293a726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> @@ -0,0 +1,189 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,ma35d1-pinctrl.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Nuvoton MA35D1 pin control and GPIO >> + >> +maintainers: >> + - Shan-Chun Hung <schung@nuvoton.com> >> + >> +properties: >> + compatible: >> + enum: >> + - nuvoton,ma35d1-pinctrl >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 1 >> + >> + nuvoton,sys: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle of the system-management node. >> + >> + ranges: true >> + >> +allOf: >> + - $ref: pinctrl.yaml# > allOf goes before additionalProperties. > >> + >> +required: >> + - compatible >> + - nuvoton,sys > This goes after patternProperties
I will fix the above two as:
allOf: - $ref: pinctrl.yaml#
required: - compatible - nuvoton,sys
additionalProperties: false
examples:
>> + >> +patternProperties: >> + "^gpio@[0-9a-f]+$": >> + type: object >> + additionalProperties: false >> + properties: >> + gpio-controller: true >> + >> + '#gpio-cells': >> + const: 2 >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + interrupt-controller: true >> + >> + '#interrupt-cells': >> + const: 2 >> + >> + interrupts: >> + description: >> + The interrupt outputs to sysirq. >> + maxItems: 1 >> + >> + required: >> + - gpio-controller >> + - '#gpio-cells' >> + - reg >> + - clocks >> + - interrupt-controller >> + - '#interrupt-cells' >> + - interrupts >> + >> + "^pin-[a-z0-9]+$": >> + type: object >> + description: >> + A pinctrl node should contain at least one subnodes representing the >> + pinctrl groups available on the machine. Each subnode will list the >> + pins it needs, and how they should be configured, with regard to muxer >> + configuration, pullups, drive strength, input enable/disable and input >> + schmitt. >> + >> + allOf: > Drop allOf, just $ref
OK, I will fix it.
>> + - $ref: pincfg-node.yaml# >> + >> + properties: >> + bias-disable: true > Drop this and other "true", why do you need them here?
We are following the conventions used in other pinctrl documents, such as 'realtek,rtd1315e-pinctrl.yaml' and 'xlnx,zynqmp-pinctrl.yaml'.
After comparing various pinctrl documents, I noticed that they all express it as 'bias-disable: true'. Therefore, may I keep the current format?
>> + >> + bias-pull-down: true >> + >> + bias-pull-up: true >> + >> + power-source: >> + description: | >> + Valid arguments are described as below: >> + 0: power supply of 1.8V >> + 1: power supply of 3.3V >> + enum: [0, 1] >> + >> + drive-strength-microamp: >> + oneOf: >> + - enum: [ 2900, 4400, 5800, 7300, 8600, 10100, 11500, 13000 ] >> + description: 1.8V I/O driving strength >> + - enum: [ 17100, 25600, 34100, 42800, 48000, 56000, 77000, 82000 ] >> + description: 3.3V I/O driving strength >> + >> + input-enable: true >> + >> + input-schmitt-enable: true >> + >> + unevaluatedProperties: false > > > Best regards, > Krzysztof >
Best Regards, Jacky Huang
| |