Re: [PATCH v4 1/6] dt-bindings: pwm: amlogic: fix s4 bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/1/17 18:30, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]

On Wed 17 Jan 2024 at 11:03, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:

[[PGP Signed Part:Undecided]]
On Fri, Dec 22, 2023 at 12:16:49PM +0100, Jerome Brunet wrote:
s4 has been added to the compatible list while converting the Amlogic PWM
binding documentation from txt to yaml.

However, on the s4, the clock bindings have different meaning compared to
the previous SoCs.

On the previous SoCs the clock bindings used to describe which input the
PWM channel multiplexer should pick among its possible parents.

This is very much tied to the driver implementation, instead of describing
the HW for what it is. When support for the Amlogic PWM was first added,
how to deal with clocks through DT was not as clear as it nowadays.
The Linux driver now ignores this DT setting, but still relies on the
hard-coded list of clock sources.

On the s4, the input multiplexer is gone. The clock bindings actually
describe the clock as it exists, not a setting. The property has a
different meaning, even if it is still 2 clocks and it would pass the check
when support is actually added.

Also the s4 cannot work if the clocks are not provided, so the property no
longer optional.

s/no/is no/

Finally, for once it makes sense to see the input as being numbered
somehow. No need to bother with clock-names on the s4 type of PWM.

Fixes: 43a1c4ff3977 ("dt-bindings: pwm: Convert Amlogic Meson PWM binding")
Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
---
  .../devicetree/bindings/pwm/pwm-amlogic.yaml  | 67 ++++++++++++++++---
  1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
index 527864a4d855..a1d382aacb82 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
@@ -9,9 +9,6 @@ title: Amlogic PWM
  maintainers:
    - Heiner Kallweit <hkallweit1@xxxxxxxxx>

-allOf:
-  - $ref: pwm.yaml#
-
  properties:
    compatible:
      oneOf:
@@ -43,12 +40,8 @@ properties:
      maxItems: 2

    clock-names:
-    oneOf:
-      - items:
-          - enum: [clkin0, clkin1]
-      - items:
-          - const: clkin0
-          - const: clkin1
+    minItems: 1
+    maxItems: 2

    "#pwm-cells":
      const: 3
@@ -57,6 +50,55 @@ required:
    - compatible
    - reg

+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson8-pwm
+              - amlogic,meson8b-pwm
+              - amlogic,meson-gxbb-pwm
+              - amlogic,meson-gxbb-ao-pwm
+              - amlogic,meson-axg-ee-pwm
+              - amlogic,meson-axg-ao-pwm
+              - amlogic,meson-g12a-ee-pwm
+              - amlogic,meson-g12a-ao-pwm-ab
+              - amlogic,meson-g12a-ao-pwm-cd
+    then:
+      # Historic bindings tied to the driver implementation
+      # The clocks provided here are meant to be matched with the input
+      # known (hard-coded) in the driver and used to select pwm clock
+      # source. Currently, the linux driver ignores this.

I admit I didn't understand the relevant difference between the old and
the new binding yet.

Let's try to explain differently then:
* So far each AML PWM IP has 2 pwm.
* Up to G12, 4 input PLL/clocks are wired to the HW IP and there
   mux/div/gate to select which input to take.
   - The historic bindings just described how to setup each of the 2
     internal muxes - 2 optionnal clocks.
     The actual 4 inputs names from CCF are hard coded in
     the driver. This is a pretty bad description. The driver has been
     updated since then to use CCF to figure out the best parent
     according to pwm rate so this setting is ignored now.
   - The 'new' bindings (introduced in patch #2) fixes the problem above
     but the meaning of the clock binding is different. It describes the
     actual HW parents - 4 clocks, optionnal since some are not wired on
     some PWM blocks. To avoid breaking the ABI, a new compatible for
     these SoC is introduced.

(The driver currently doesn't support the s4, right?)

Indeed. I know Amlogic is preparing the support. I could do it in this
series but I prefer to encourage them to contribute. It should come
shortly after this series is merged.
Yes. Amlogic is waiting these patches of Jerome. after these merge, we will start to rebase and submit s4 code.
-
Best Regards Junyi




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux