lkml.org 
[lkml]   [2022]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 1/2] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
From
On 12/05/2022 11:41, Chris Packham wrote:
> Convert the existing device tree binding to YAML format.
>
> The old binding listed the interrupt-controller and related properties
> as required but there are sufficiently many existing usages without it
> that the YAML binding does not make the interrupt properties required.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>
> Notes:
> Changes in v3:
> - Correct indent in example
> - Move offset and marvell,pwm-offset to separate patch
> - Correct some documentation cross references

Thank you for your patch. There is something to discuss/improve.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
> new file mode 100644
> index 000000000000..2d95ef707f53
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-mvebu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell EBU GPIO controller
> +
> +maintainers:
> + - Thierry Reding <thierry.reding@gmail.com>
> + - Lee Jones <lee.jones@linaro.org>

These should be rather platform or driver maintainers, not subsystem
folks. Unless it happens that Thierry and Lee are for platform?

> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - marvell,armada-8k-gpio
> + - marvell,orion-gpio
> +
> + - items:
> + - enum:
> + - marvell,mv78200-gpio
> + - marvell,armada-370-gpio
> + - marvell,armadaxp-gpio
> + - const: marvell,orion-gpio
> +
> + reg:
> + description: |
> + Address and length of the register set for the device. Not used for
> + marvell,armada-8k-gpio.
> +
> + For the "marvell,armadaxp-gpio" variant a second entry is expected for
> + the per-cpu registers. For other variants second entry can be provided,
> + for the PWM function using the GPIO Blink Counter on/off registers.
> + minItems: 1
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: gpio
> + - const: pwm
> + minItems: 1
> +
> + interrupts:
> + description: |
> + The list of interrupts that are used for all the pins managed by this
> + GPIO bank. There can be more than one interrupt (example: 1 interrupt
> + per 8 pins on Armada XP, which means 4 interrupts per bank of 32
> + GPIOs).
> + minItems: 1
> + maxItems: 4
> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 2
> +
> + gpio-controller: true
> +
> + ngpios:
> + minimum: 1
> + maximum: 32
> +
> + "#gpio-cells":
> + const: 2
> +
> + "#pwm-cells":
> + description:
> + The first cell is the GPIO line number. The second cell is the period
> + in nanoseconds.
> + const: 2
> +
> + clocks:
> + description:
> + Clock(s) used for PWM function.
> + items:
> + - description: Core clock
> + - description: AXI bus clock
> + minItems: 1
> +
> + clock-names:
> + items:
> + - const: core
> + - const: axi
> + minItems: 1
> +
> +required:
> + - compatible
> + - gpio-controller
> + - ngpios
> + - "#gpio-cells"
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: marvell,armada-8k-gpio
> + then:
> + required:
> + - offset
> + else:
> + required:
> + - reg

one blank line please

> + - if:
> + properties:
> + compatible:
> + contains:
> + const: marvell,armadaxp-gpio

Original bindings are saying that second reg is optional for
marvell,armada-370-gpio. What about other cases, e.g. mv78200-gpio? Is
it also allowed (and optional) there?

> + then:
> + properties:
> + reg:
> + minItems: 2

Then you also should require two reg-names.


Best regards,
Krzysztof

\
 
 \ /
  Last update: 2022-05-13 10:41    [W:0.057 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site