Messages in this thread | | | Date | Tue, 19 Apr 2022 13:50:01 +0200 | Subject | Re: [PATCH 1/2] dt-binding: Add cypress,cy8c95x0 binding | From | Krzysztof Kozlowski <> |
| |
On 19/04/2022 09:15, Patrick Rudolph wrote: > Added device tree binding documentation for > Cypress CY8C95x0 I2C pin-controller. >
Thank you for your patch. There is something to discuss/improve.
(...)
> + reg: > + maxItems: 1 > + > + gpio-controller: true > + > + '#gpio-cells': > + description: > + The first cell is the pin number and the second cell is used > + to specify optional parameters. > + const: 2 > + > + gpio-reserved-ranges: true > + > + interrupts: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + description: > + Specifies the pin number and flags, as defined in > + include/dt-bindings/interrupt-controller/irq.h
Skip description, it's obvious.
> + const: 2 > + > + ngpios: > + minimum: 1 > + maximum: 60 > + > + gpio-line-names: > + minItems: 1 > + maxItems: 60 > + > + vdd-supply: > + description: > + Optional power supply. > + > +patternProperties: > + '-pins$': > + type: object > + description: > + Pinctrl node's client devices use subnodes for desired pin configuration. > + Client device subnodes use below standard properties. > + $ref: "/schemas/pinctrl/pincfg-node.yaml" > + > + properties: > + pins: > + description: > + List of gpio pins affected by the properties specified in this > + subnode. > + items: > + oneOf:
Why do you need oneOf here?
> + - pattern: "^gp([0-7][0-7])$" > + minItems: 1 > + maxItems: 60 > + > + function: > + enum: [ gpio, pwm ] > +
No need for blank line. In other cases you put description before the property constraints, so do it consistently here as well.
> + description: > + Specify the alternative function to be configured for the specified > + pins. > + > + bias-pull-down: true > + > + bias-pull-up: true > + > + bias-disable: true > + > + output-high: true > + > + output-low: true > + > + drive-push-pull: true > + > + drive-open-drain: true > + > + drive-open-source: true > + > + required: > + - pins > + - function > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-controller > + - '#interrupt-cells' > + - gpio-controller > + - '#gpio-cells'
no allOf referencing pinctrl.yaml? Include it unless there is some reason not to.
Best regards, Krzysztof
| |