[v7,1/4] dt-bindings: pwm: allwinner: add h616 pwm compatible
Commit Message
Allwinner H616 PWM block is quite different from the A10 or H6, but at
the end, it uses the same clocks as the H6; so the sun4i pwm binding can
be used.
It has 6 channels than can generate PWM waveforms.
If the bypass is enabled (one bypass per channel) the output is no more
a PWM waveform, but a clock that can (and is) used as input for other
devices, like the AC300 PHY.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
.../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
Comments
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `#clock-cells` property is missing from the `required` list for the `allwinner,sun50i-h616-pwm` compatible.
--
commit d7a701d864bf3d5f7e7d179d4498b5006b8bf3f7
Author: Richard Genoud <richard.genoud@bootlin.com>
dt-bindings: pwm: allwinner: add h616 pwm compatible
This commit introduces the Allwinner H616 PWM block to the sun4i pwm
binding. It adds the new compatible string and handles the new clock
provider capabilities introduced by the H616 bypass mode.
> diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> index 1197858e431f2..4f58110ec98f7 100644
> --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
[ ... ]
> @@ -83,6 +89,17 @@ allOf:
> clocks:
> maxItems: 1
>
> + - if:
> + not:
> + properties:
> + compatible:
> + contains:
> + const: allwinner,sun50i-h616-pwm
> +
> + then:
> + properties:
> + "#clock-cells": false
> +
[Severity: Medium]
Should #clock-cells be added to the required properties for the H616 variant?
The commit message mentions that the H616 PWM block can output a clock
used as an input for other devices. While the conditional block above
correctly prevents older hardware variants from using #clock-cells, it
appears to leave the property optional for the H616.
If a device tree node for allwinner,sun50i-h616-pwm omits #clock-cells,
it might silently pass schema validation but fail at runtime when dependent
subsystems try to use it as a clock provider.
Would it make sense to add an else clause here (or a separate conditional)
to explicitly require #clock-cells when the H616 compatible is present?
> required:
> - compatible
> - reg
@@ -14,6 +14,9 @@ properties:
"#pwm-cells":
const: 3
+ "#clock-cells":
+ const: 1
+
compatible:
oneOf:
- const: allwinner,sun4i-a10-pwm
@@ -36,6 +39,7 @@ properties:
- const: allwinner,sun50i-h5-pwm
- const: allwinner,sun5i-a13-pwm
- const: allwinner,sun50i-h6-pwm
+ - const: allwinner,sun50i-h616-pwm
reg:
maxItems: 1
@@ -62,7 +66,9 @@ allOf:
properties:
compatible:
contains:
- const: allwinner,sun50i-h6-pwm
+ enum:
+ - allwinner,sun50i-h6-pwm
+ - allwinner,sun50i-h616-pwm
then:
properties:
@@ -83,6 +89,17 @@ allOf:
clocks:
maxItems: 1
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: allwinner,sun50i-h616-pwm
+
+ then:
+ properties:
+ "#clock-cells": false
+
required:
- compatible
- reg