* [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
@ 2022-05-10 9:44 Chris Packham
2022-05-10 13:40 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Chris Packham @ 2022-05-10 9:44 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt,
thierry.reding, u.kleine-koenig, lee.jones
Cc: linux-gpio, devicetree, linux-kernel, linux-pwm, Chris Packham
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.
The offset and marvell,pwm-offset properties weren't in the old binding
and are added to the YAML binding. The offset property is required when
the marvell,armada-8k-gpio compatible is used.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
.../devicetree/bindings/gpio/gpio-mvebu.txt | 93 -----------
.../devicetree/bindings/gpio/gpio-mvebu.yaml | 147 ++++++++++++++++++
MAINTAINERS | 2 +-
3 files changed, 148 insertions(+), 94 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
deleted file mode 100644
index 0fc6700ed800..000000000000
--- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+++ /dev/null
@@ -1,93 +0,0 @@
-* Marvell EBU GPIO controller
-
-Required properties:
-
-- compatible : Should be "marvell,orion-gpio", "marvell,mv78200-gpio",
- "marvell,armadaxp-gpio" or "marvell,armada-8k-gpio".
-
- "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove,
- Discovery (except MV78200) and Armada 370. "marvell,mv78200-gpio"
- should be used for the Discovery MV78200.
-
- "marvel,armadaxp-gpio" should be used for all Armada XP SoCs
- (MV78230, MV78260, MV78460).
-
- "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K
- SoCs (either from AP or CP), see
- Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
- for specific details about the offset property.
-
-- reg: Address and length of the register set for the device. Only one
- entry is expected, except for the "marvell,armadaxp-gpio" variant
- for which two entries are expected: one for the general registers,
- one for the per-cpu registers. Not used for marvell,armada-8k-gpio.
-
-- interrupts: 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).
-
-- interrupt-controller: identifies the node as an interrupt controller
-
-- #interrupt-cells: specifies the number of cells needed to encode an
- interrupt source. Should be two.
- The first cell is the GPIO number.
- The second cell is used to specify flags:
- bits[3:0] trigger type and level flags:
- 1 = low-to-high edge triggered.
- 2 = high-to-low edge triggered.
- 4 = active high level-sensitive.
- 8 = active low level-sensitive.
-
-- gpio-controller: marks the device node as a gpio controller
-
-- ngpios: number of GPIOs this controller has
-
-- #gpio-cells: Should be two. The first cell is the pin number. The
- second cell is reserved for flags, unused at the moment.
-
-Optional properties:
-
-In order to use the GPIO lines in PWM mode, some additional optional
-properties are required.
-
-- compatible: Must contain "marvell,armada-370-gpio"
-
-- reg: an additional register set is needed, for the GPIO Blink
- Counter on/off registers.
-
-- reg-names: Must contain an entry "pwm" corresponding to the
- additional register range needed for PWM operation.
-
-- #pwm-cells: Should be two. The first cell is the GPIO line number. The
- second cell is the period in nanoseconds.
-
-- clocks: Must be a phandle to the clock for the GPIO controller.
-
-Example:
-
- gpio0: gpio@d0018100 {
- compatible = "marvell,armadaxp-gpio";
- reg = <0xd0018100 0x40>,
- <0xd0018800 0x30>;
- ngpios = <32>;
- gpio-controller;
- #gpio-cells = <2>;
- interrupt-controller;
- #interrupt-cells = <2>;
- interrupts = <16>, <17>, <18>, <19>;
- };
-
- gpio1: gpio@18140 {
- compatible = "marvell,armada-370-gpio";
- reg = <0x18140 0x40>, <0x181c8 0x08>;
- reg-names = "gpio", "pwm";
- ngpios = <17>;
- gpio-controller;
- #gpio-cells = <2>;
- #pwm-cells = <2>;
- interrupt-controller;
- #interrupt-cells = <2>;
- interrupts = <87>, <88>, <89>;
- clocks = <&coreclk 0>;
- };
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
new file mode 100644
index 000000000000..84b72e506526
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
@@ -0,0 +1,147 @@
+# 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>
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - marvell,orion-gpio
+ - marvell,mv78200-gpio
+ - marvell,armada-370-gpio
+ - marvell,armadaxp-gpio
+ - marvell,armada-8k-gpio
+ - items:
+ - const: marvell,armada-370-gpio
+ - const: marvell,orion-gpio
+
+ description: |
+ "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove, Discovery
+ (except MV78200) and Armada 370. "marvell,mv78200-gpio" should be used
+ for the Discovery MV78200.
+
+ "marvel,armadaxp-gpio" should be used for all Armada XP SoCs (MV78230,
+ MV78260, MV78460).
+
+ "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K SoCs
+ (either from AP or CP), see
+ Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
+ for specific details about the offset property.
+
+ reg:
+ description: |
+ Address and length of the register set for the device. Only one entry
+ is expected, except for the "marvell,armadaxp-gpio" variant for which
+ two entries are expected: one for the general registers, one for the
+ per-cpu registers. Not used for marvell,armada-8k-gpio.
+
+ An additional register set is needed, for the GPIO Blink
+ Counter on/off registers.
+ minItems: 1
+ maxItems: 2
+
+ reg-names:
+ description:
+ Must contain an entry "pwm" corresponding to the
+ additional register range needed for PWM operation.
+ minItems: 1
+ maxItems: 2
+
+ offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Offset in the register map for the gpio registers (in bytes)
+
+ 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:
+ description:
+ number of GPIOs this controller has
+ minimum: 1
+ maximum: 32
+
+ "#gpio-cells":
+ const: 2
+
+ marvell,pwm-offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Offset in the register map for the pwm registers (in bytes)
+
+ "#pwm-cells":
+ description:
+ The first cell is the GPIO line number. The second cell is the period
+ in nanoseconds.
+ const: 2
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+required:
+ - compatible
+ - gpio-controller
+ - ngpios
+ - "#gpio-cells"
+
+if:
+ properties:
+ compatible:
+ contains:
+ const: marvell,armada-8k-gpio
+then:
+ required:
+ - offset
+else:
+ required:
+ - reg
+
+unevaluatedProperties: true
+
+examples:
+ - |
+ gpio@d0018100 {
+ compatible = "marvell,armadaxp-gpio";
+ reg = <0xd0018100 0x40>, <0xd0018800 0x30>;
+ ngpios = <32>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <16>, <17>, <18>, <19>;
+ };
+
+ - |
+ gpio@18140 {
+ compatible = "marvell,armada-370-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
+ ngpios = <17>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #pwm-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index e8c52d0192a6..6b1c80fd7611 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16019,7 +16019,7 @@ L: linux-pwm@vger.kernel.org
S: Maintained
Q: https://patchwork.ozlabs.org/project/linux-pwm/list/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git
-F: Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+F: Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
F: Documentation/devicetree/bindings/pwm/
F: Documentation/driver-api/pwm.rst
F: drivers/gpio/gpio-mvebu.c
--
2.36.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
2022-05-10 9:44 [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML Chris Packham
@ 2022-05-10 13:40 ` Krzysztof Kozlowski
2022-05-10 20:55 ` Chris Packham
2022-05-10 23:34 ` Chris Packham
0 siblings, 2 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-10 13:40 UTC (permalink / raw)
To: Chris Packham, linus.walleij, brgl, robh+dt,
krzysztof.kozlowski+dt, thierry.reding, u.kleine-koenig,
lee.jones
Cc: linux-gpio, devicetree, linux-kernel, linux-pwm
On 10/05/2022 11:44, 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.
>
> The offset and marvell,pwm-offset properties weren't in the old binding
> and are added to the YAML binding. The offset property is required when
> the marvell,armada-8k-gpio compatible is used.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> .../devicetree/bindings/gpio/gpio-mvebu.txt | 93 -----------
> .../devicetree/bindings/gpio/gpio-mvebu.yaml | 147 ++++++++++++++++++
> MAINTAINERS | 2 +-
> 3 files changed, 148 insertions(+), 94 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> deleted file mode 100644
> index 0fc6700ed800..000000000000
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ /dev/null
> @@ -1,93 +0,0 @@
> -* Marvell EBU GPIO controller
> -
> -Required properties:
> -
> -- compatible : Should be "marvell,orion-gpio", "marvell,mv78200-gpio",
> - "marvell,armadaxp-gpio" or "marvell,armada-8k-gpio".
> -
> - "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove,
> - Discovery (except MV78200) and Armada 370. "marvell,mv78200-gpio"
> - should be used for the Discovery MV78200.
> -
> - "marvel,armadaxp-gpio" should be used for all Armada XP SoCs
> - (MV78230, MV78260, MV78460).
> -
> - "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K
> - SoCs (either from AP or CP), see
> - Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
> - for specific details about the offset property.
> -
> -- reg: Address and length of the register set for the device. Only one
> - entry is expected, except for the "marvell,armadaxp-gpio" variant
> - for which two entries are expected: one for the general registers,
> - one for the per-cpu registers. Not used for marvell,armada-8k-gpio.
> -
> -- interrupts: 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).
> -
> -- interrupt-controller: identifies the node as an interrupt controller
> -
> -- #interrupt-cells: specifies the number of cells needed to encode an
> - interrupt source. Should be two.
> - The first cell is the GPIO number.
> - The second cell is used to specify flags:
> - bits[3:0] trigger type and level flags:
> - 1 = low-to-high edge triggered.
> - 2 = high-to-low edge triggered.
> - 4 = active high level-sensitive.
> - 8 = active low level-sensitive.
> -
> -- gpio-controller: marks the device node as a gpio controller
> -
> -- ngpios: number of GPIOs this controller has
> -
> -- #gpio-cells: Should be two. The first cell is the pin number. The
> - second cell is reserved for flags, unused at the moment.
> -
> -Optional properties:
> -
> -In order to use the GPIO lines in PWM mode, some additional optional
> -properties are required.
> -
> -- compatible: Must contain "marvell,armada-370-gpio"
> -
> -- reg: an additional register set is needed, for the GPIO Blink
> - Counter on/off registers.
> -
> -- reg-names: Must contain an entry "pwm" corresponding to the
> - additional register range needed for PWM operation.
> -
> -- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> - second cell is the period in nanoseconds.
> -
> -- clocks: Must be a phandle to the clock for the GPIO controller.
> -
> -Example:
> -
> - gpio0: gpio@d0018100 {
> - compatible = "marvell,armadaxp-gpio";
> - reg = <0xd0018100 0x40>,
> - <0xd0018800 0x30>;
> - ngpios = <32>;
> - gpio-controller;
> - #gpio-cells = <2>;
> - interrupt-controller;
> - #interrupt-cells = <2>;
> - interrupts = <16>, <17>, <18>, <19>;
> - };
> -
> - gpio1: gpio@18140 {
> - compatible = "marvell,armada-370-gpio";
> - reg = <0x18140 0x40>, <0x181c8 0x08>;
> - reg-names = "gpio", "pwm";
> - ngpios = <17>;
> - gpio-controller;
> - #gpio-cells = <2>;
> - #pwm-cells = <2>;
> - interrupt-controller;
> - #interrupt-cells = <2>;
> - interrupts = <87>, <88>, <89>;
> - clocks = <&coreclk 0>;
> - };
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
> new file mode 100644
> index 000000000000..84b72e506526
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
> @@ -0,0 +1,147 @@
> +# 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>
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - marvell,orion-gpio
> + - marvell,mv78200-gpio
> + - marvell,armada-370-gpio
It's expected to have orion fallback, so this does not look correct.
> + - marvell,armadaxp-gpio
> + - marvell,armada-8k-gpio
> + - items:
> + - const: marvell,armada-370-gpio
> + - const: marvell,orion-gpio
> +
> + description: |
> + "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove, Discovery
> + (except MV78200) and Armada 370. "marvell,mv78200-gpio" should be used
> + for the Discovery MV78200.
> +
> + "marvel,armadaxp-gpio" should be used for all Armada XP SoCs (MV78230,
> + MV78260, MV78460).
> +
> + "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K SoCs
> + (either from AP or CP), see
> + Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
> + for specific details about the offset property.
Why having the description? The usage should be obvious from the schema,
so what is so special here?
> +
> + reg:
> + description: |
> + Address and length of the register set for the device. Only one entry
> + is expected, except for the "marvell,armadaxp-gpio" variant for which
> + two entries are expected: one for the general registers, one for the
> + per-cpu registers.
This needs also entry in allOf with per-variant constraints.
> Not used for marvell,armada-8k-gpio.
> +
> + An additional register set is needed, for the GPIO Blink
> + Counter on/off registers.
> + minItems: 1
> + maxItems: 2
PWM? the "reg" above was saying about per-cpu registers, so this is
confusing. I understand old bindings wrote it like that, so maybe it
should be fixed now.
Anyway, you need to describe the items and then apply constraints in allOf.
> +
> + reg-names:
> + description:
> + Must contain an entry "pwm" corresponding to the
> + additional register range needed for PWM operation.
> + minItems: 1
> + maxItems: 2
> +
> + offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Offset in the register map for the gpio registers (in bytes)
> +
> + 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:
> + description:
> + number of GPIOs this controller has
Skip description, it's obvious from generic bindings.
> + minimum: 1
> + maximum: 32
> +
> + "#gpio-cells":
> + const: 2
> +
> + marvell,pwm-offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Offset in the register map for the pwm registers (in bytes)
It's the same as offset. Why allowing both? Isn't one deprecated?
> +
> + "#pwm-cells":
> + description:
> + The first cell is the GPIO line number. The second cell is the period
> + in nanoseconds.
> + const: 2
> +
> + clocks:
> + minItems: 1
> + maxItems: 2
This should be strictly defined, either here or per variant.
> +
> +required:
> + - compatible
> + - gpio-controller
> + - ngpios
> + - "#gpio-cells"
> +
> +if:
Within allOf please.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
2022-05-10 13:40 ` Krzysztof Kozlowski
@ 2022-05-10 20:55 ` Chris Packham
2022-05-11 7:23 ` Krzysztof Kozlowski
2022-05-10 23:34 ` Chris Packham
1 sibling, 1 reply; 5+ messages in thread
From: Chris Packham @ 2022-05-10 20:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, linus.walleij, brgl, robh+dt,
krzysztof.kozlowski+dt, thierry.reding, u.kleine-koenig,
lee.jones
Cc: linux-gpio, devicetree, linux-kernel, linux-pwm
(sigh resend, Thunderbird decided that this needed html)
On 11/05/22 01:40, Krzysztof Kozlowski wrote:
>> + marvell,pwm-offset:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Offset in the register map for the pwm registers (in bytes)
> It's the same as offset. Why allowing both? Isn't one deprecated?
>
This one is in addition to offset. The "offset" is for the gpio
registers "marvell,pwm-offset" is for a separate pwm related register
that is not in the same contiguous block.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
2022-05-10 13:40 ` Krzysztof Kozlowski
2022-05-10 20:55 ` Chris Packham
@ 2022-05-10 23:34 ` Chris Packham
1 sibling, 0 replies; 5+ messages in thread
From: Chris Packham @ 2022-05-10 23:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, linus.walleij, brgl, robh+dt,
krzysztof.kozlowski+dt, thierry.reding, u.kleine-koenig,
lee.jones
Cc: linux-gpio, devicetree, linux-kernel, linux-pwm
On 11/05/22 01:40, Krzysztof Kozlowski wrote:
> On 10/05/2022 11:44, 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.
>>
>> The offset and marvell,pwm-offset properties weren't in the old binding
>> and are added to the YAML binding. The offset property is required when
>> the marvell,armada-8k-gpio compatible is used.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>> .../devicetree/bindings/gpio/gpio-mvebu.txt | 93 -----------
>> .../devicetree/bindings/gpio/gpio-mvebu.yaml | 147 ++++++++++++++++++
>> MAINTAINERS | 2 +-
>> 3 files changed, 148 insertions(+), 94 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
>> deleted file mode 100644
>> index 0fc6700ed800..000000000000
>> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
>> +++ /dev/null
>> @@ -1,93 +0,0 @@
>> -* Marvell EBU GPIO controller
>> -
>> -Required properties:
>> -
>> -- compatible : Should be "marvell,orion-gpio", "marvell,mv78200-gpio",
>> - "marvell,armadaxp-gpio" or "marvell,armada-8k-gpio".
>> -
>> - "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove,
>> - Discovery (except MV78200) and Armada 370. "marvell,mv78200-gpio"
>> - should be used for the Discovery MV78200.
>> -
>> - "marvel,armadaxp-gpio" should be used for all Armada XP SoCs
>> - (MV78230, MV78260, MV78460).
>> -
>> - "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K
>> - SoCs (either from AP or CP), see
>> - Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
>> - for specific details about the offset property.
>> -
>> -- reg: Address and length of the register set for the device. Only one
>> - entry is expected, except for the "marvell,armadaxp-gpio" variant
>> - for which two entries are expected: one for the general registers,
>> - one for the per-cpu registers. Not used for marvell,armada-8k-gpio.
>> -
>> -- interrupts: 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).
>> -
>> -- interrupt-controller: identifies the node as an interrupt controller
>> -
>> -- #interrupt-cells: specifies the number of cells needed to encode an
>> - interrupt source. Should be two.
>> - The first cell is the GPIO number.
>> - The second cell is used to specify flags:
>> - bits[3:0] trigger type and level flags:
>> - 1 = low-to-high edge triggered.
>> - 2 = high-to-low edge triggered.
>> - 4 = active high level-sensitive.
>> - 8 = active low level-sensitive.
>> -
>> -- gpio-controller: marks the device node as a gpio controller
>> -
>> -- ngpios: number of GPIOs this controller has
>> -
>> -- #gpio-cells: Should be two. The first cell is the pin number. The
>> - second cell is reserved for flags, unused at the moment.
>> -
>> -Optional properties:
>> -
>> -In order to use the GPIO lines in PWM mode, some additional optional
>> -properties are required.
>> -
>> -- compatible: Must contain "marvell,armada-370-gpio"
>> -
>> -- reg: an additional register set is needed, for the GPIO Blink
>> - Counter on/off registers.
>> -
>> -- reg-names: Must contain an entry "pwm" corresponding to the
>> - additional register range needed for PWM operation.
>> -
>> -- #pwm-cells: Should be two. The first cell is the GPIO line number. The
>> - second cell is the period in nanoseconds.
>> -
>> -- clocks: Must be a phandle to the clock for the GPIO controller.
>> -
>> -Example:
>> -
>> - gpio0: gpio@d0018100 {
>> - compatible = "marvell,armadaxp-gpio";
>> - reg = <0xd0018100 0x40>,
>> - <0xd0018800 0x30>;
>> - ngpios = <32>;
>> - gpio-controller;
>> - #gpio-cells = <2>;
>> - interrupt-controller;
>> - #interrupt-cells = <2>;
>> - interrupts = <16>, <17>, <18>, <19>;
>> - };
>> -
>> - gpio1: gpio@18140 {
>> - compatible = "marvell,armada-370-gpio";
>> - reg = <0x18140 0x40>, <0x181c8 0x08>;
>> - reg-names = "gpio", "pwm";
>> - ngpios = <17>;
>> - gpio-controller;
>> - #gpio-cells = <2>;
>> - #pwm-cells = <2>;
>> - interrupt-controller;
>> - #interrupt-cells = <2>;
>> - interrupts = <87>, <88>, <89>;
>> - clocks = <&coreclk 0>;
>> - };
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
>> new file mode 100644
>> index 000000000000..84b72e506526
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.yaml
>> @@ -0,0 +1,147 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=1ev64paEKF7o3yTVoF_fgiVBkeUi0_Dg3YQi4JPtpA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fgpio%2fgpio-mvebu%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=1ev64paEKF7o3yTVoF_fgiVBkeUi0_Dg3YBxtMDuqw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Marvell EBU GPIO controller
>> +
>> +maintainers:
>> + - Thierry Reding <thierry.reding@gmail.com>
>> + - Lee Jones <lee.jones@linaro.org>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - marvell,orion-gpio
>> + - marvell,mv78200-gpio
>> + - marvell,armada-370-gpio
> It's expected to have orion fallback, so this does not look correct.
Except for marvell,armada-8k-gpio. I'll look in to how to capture that.
>
>> + - marvell,armadaxp-gpio
>> + - marvell,armada-8k-gpio
>> + - items:
>> + - const: marvell,armada-370-gpio
>> + - const: marvell,orion-gpio
>> +
>> + description: |
>> + "marvell,orion-gpio" should be used for Orion, Kirkwood, Dove, Discovery
>> + (except MV78200) and Armada 370. "marvell,mv78200-gpio" should be used
>> + for the Discovery MV78200.
>> +
>> + "marvel,armadaxp-gpio" should be used for all Armada XP SoCs (MV78230,
>> + MV78260, MV78460).
>> +
>> + "marvell,armada-8k-gpio" should be used for the Armada 7K and 8K SoCs
>> + (either from AP or CP), see
>> + Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
>> + for specific details about the offset property.
> Why having the description? The usage should be obvious from the schema,
> so what is so special here?
I found the mapping of SoCs to compatible a bit non-obvious so I kept
it. The in-tree dts files don't actually follow what's documented
(armada-xp-*.dtsi don't use "marvel,armadaxp-gpio").
>> +
>> + reg:
>> + description: |
>> + Address and length of the register set for the device. Only one entry
>> + is expected, except for the "marvell,armadaxp-gpio" variant for which
>> + two entries are expected: one for the general registers, one for the
>> + per-cpu registers.
> This needs also entry in allOf with per-variant constraints.
>
>> Not used for marvell,armada-8k-gpio.
>> +
>> + An additional register set is needed, for the GPIO Blink
>> + Counter on/off registers.
>> + minItems: 1
>> + maxItems: 2
> PWM? the "reg" above was saying about per-cpu registers, so this is
> confusing. I understand old bindings wrote it like that, so maybe it
> should be fixed now.
I merged two sections from the old bindings. On the ArmadaXP the 2nd
entry is documented as for the per-CPU registers and appears to be
mandatory. For the others the 2nd entry is optional and is for the pwm
function. But as noted above the in-tree dts files don't actually use
the armadaxp compatible so this might be wrong in the old binding.
>
> Anyway, you need to describe the items and then apply constraints in allOf.
>
>> +
>> + reg-names:
>> + description:
>> + Must contain an entry "pwm" corresponding to the
>> + additional register range needed for PWM operation.
>> + minItems: 1
>> + maxItems: 2
>> +
>> + offset:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Offset in the register map for the gpio registers (in bytes)
>> +
>> + 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:
>> + description:
>> + number of GPIOs this controller has
> Skip description, it's obvious from generic bindings.
>
>> + minimum: 1
>> + maximum: 32
>> +
>> + "#gpio-cells":
>> + const: 2
>> +
>> + marvell,pwm-offset:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Offset in the register map for the pwm registers (in bytes)
> It's the same as offset. Why allowing both? Isn't one deprecated?
As mentioned separately. This is for the pwm function which is different
to the other offset.
>> +
>> + "#pwm-cells":
>> + description:
>> + The first cell is the GPIO line number. The second cell is the period
>> + in nanoseconds.
>> + const: 2
>> +
>> + clocks:
>> + minItems: 1
>> + maxItems: 2
> This should be strictly defined, either here or per variant.
Will update
>> +
>> +required:
>> + - compatible
>> + - gpio-controller
>> + - ngpios
>> + - "#gpio-cells"
>> +
>> +if:
> Within allOf please.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML
2022-05-10 20:55 ` Chris Packham
@ 2022-05-11 7:23 ` Krzysztof Kozlowski
0 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-11 7:23 UTC (permalink / raw)
To: Chris Packham, linus.walleij, brgl, robh+dt,
krzysztof.kozlowski+dt, thierry.reding, u.kleine-koenig,
lee.jones
Cc: linux-gpio, devicetree, linux-kernel, linux-pwm
On 10/05/2022 22:55, Chris Packham wrote:
> (sigh resend, Thunderbird decided that this needed html)
>
> On 11/05/22 01:40, Krzysztof Kozlowski wrote:
>>> + marvell,pwm-offset:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Offset in the register map for the pwm registers (in bytes)
>> It's the same as offset. Why allowing both? Isn't one deprecated?
>>
> This one is in addition to offset. The "offset" is for the gpio
> registers "marvell,pwm-offset" is for a separate pwm related register
> that is not in the same contiguous block.
I see now different in the description. Egh, confusing. But the
confusion was done earlier, so let it be.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-11 7:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10 9:44 [PATCH] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML Chris Packham
2022-05-10 13:40 ` Krzysztof Kozlowski
2022-05-10 20:55 ` Chris Packham
2022-05-11 7:23 ` Krzysztof Kozlowski
2022-05-10 23:34 ` Chris Packham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).