Add support for Allwinner PWM on D1/T113s/R329 SoCs

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +52 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <[email protected]>
+  - Brandon Cheo Fusi <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:
+    items:
+      - description: Bus clock
+      - description: 24 MHz oscillator
+      - description: APB0 clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: hosc
+      - const: apb0
+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +26 -0

+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#pwm-cells"
+  - clocks
+  - clock-names
+  - resets
+
+examples:
+  - |
+    #include <dt-bindings/clock/sun20i-d1-ccu.h>
+    #include <dt-bindings/reset/sun20i-d1-ccu.h>
+
+    pwm: pwm@2000c00 {
+      compatible = "allwinner,sun20i-d1-pwm";
+      reg = <0x02000c00 0x400>;
+      clocks = <&ccu CLK_BUS_PWM>, <&dcxo>, <&ccu CLK_APB0>;
+      clock-names = "bus", "hosc", "apb0";
+      resets = <&ccu RST_BUS_PWM>;
+      #pwm-cells = <0x3>;
+    };
+
+...

On Wed, 31 Jan 2024 15:59:14 +0300
Aleksandr Shubin <[email protected]> wrote:

Hi,

Quoted 88 lines

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +58 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <[email protected]>
+  - Brandon Cheo Fusi <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:
+    items:
+      - description: Bus clock
+      - description: 24 MHz oscillator
+      - description: APB0 clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: hosc
+      - const: apb0
+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun20i-r329-pwm
+
+    then:
+      required:
+        - allwinner,pwm-channels

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml line 62

Quoted 4 lines

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +4 -0

+
+    else:
+      properties:
+        allwinner,pwm-channels: false

Do we really need to be that strict?
If something compatible to D1 pops up in the future, just with a different number of channels, we would need a new compatible string.

If we would leave this else branch out, we could just specify some number differing from the default, and be good.

The number of channels really looks like a parameter to the IP, it's modelled like this in the manual (PCR: 0x0100 + 0x0000 + N * 0x0020).

Quoted 93 lines

On Wed, 31 Jan 2024 15:59:14 +0300
Aleksandr Shubin <[email protected]> wrote:

Hi,

Quoted 88 lines

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +58 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <[email protected]>
+  - Brandon Cheo Fusi <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:
+    items:
+      - description: Bus clock
+      - description: 24 MHz oscillator
+      - description: APB0 clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: hosc
+      - const: apb0
+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun20i-r329-pwm
+
+    then:
+      required:
+        - allwinner,pwm-channels

Quoted 8 lines

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml line 62

Quoted 4 lines

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +4 -0

+
+    else:
+      properties:
+        allwinner,pwm-channels: false  

Do we really need to be that strict?
If something compatible to D1 pops up in the future, just with a different number of channels, we would need a new compatible string.

Well, you would want to have a soc specific compatible anyway then, right?

Quoted 2 lines

If we would leave this else branch out, we could just specify some number differing from the default, and be good.

If it were compatible with the d1, then the "then:" branch would apply, provided you used the fallback correctly.

Although if the number of
channels were different, we'd likely end up with modifications here to limit it to the correct values for each soc.

Quoted 2 lines

The number of channels really looks like a parameter to the IP, it's modelled like this in the manual (PCR: 0x0100 + 0x0000 + N * 0x0020).

On Wed, 31 Jan 2024 21:22:06 +0000
Conor Dooley <[email protected]> wrote:

Hi,

Quoted 103 lines · Andre Przywara

Quoted 93 lines

On Wed, 31 Jan 2024 15:59:14 +0300
Aleksandr Shubin <[email protected]> wrote:

Hi,

Quoted 88 lines

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +58 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <[email protected]>
+  - Brandon Cheo Fusi <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:
+    items:
+      - description: Bus clock
+      - description: 24 MHz oscillator
+      - description: APB0 clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: hosc
+      - const: apb0
+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun20i-r329-pwm
+
+    then:
+      required:
+        - allwinner,pwm-channels

Quoted 8 lines

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml line 62

Quoted 4 lines

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +4 -0

+
+    else:
+      properties:
+        allwinner,pwm-channels: false  

Do we really need to be that strict?
If something compatible to D1 pops up in the future, just with a different number of channels, we would need a new compatible string.

Quoted 2 lines

Well, you would want to have a soc specific compatible anyway then, right?

So the idea would be to add any new (specific) compatible string to that list then, when we add them?
I guess this would work, but strictly speaking any current driver would then only need to check this property for the R329 type? The Linux driver proposed in the next patch *always* honours the allwinner,pwm-channels property, which is IMHO the right way to implement this. And that's why I think the binding should reflect that, and not explicitly *forbid* the property for every one other than R329 (atm).

With the current Linux driver, a potential new SoC using:

	"allwinner,sun20i-d2-pwm", "allwinner,sun20i-d1-pwm";
	allwinner,pwm-channels = <6>;

would work without driver changes. A driver strictly written to this binding here might not, though, as it would be free to ignore the pwm-channels property.

Does that make sense? So to encourage future compatibility, can we drop the "else" branch?

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
.../bindings/pwm/allwinner,sun20i-pwm.yaml | 88 +++++++++++++++++++

1 file changed, 88 insertions(+)

create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +8 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +26 -0

+ - const: allwinner,sun20i-d1-pwm
+
+ reg:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 3
+
+ clocks:
+ items:
+ - description: Bus clock
+ - description: 24 MHz oscillator
+ - description: APB0 clock
+
+ clock-names:
+ items:
+ - const: bus
+ - const: hosc
+ - const: apb0
+
+ resets:
+ maxItems: 1
+
+ allwinner,pwm-channels:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: The number of PWM channels configured for this instance

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +33 -1

+ then:
+ required:
+ - allwinner,pwm-channels
+
+ else:
+ properties:
+ allwinner,pwm-channels: false
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#pwm-cells"
+ - clocks
+ - clock-names
+ - resets
+
+examples:
+ - |
+ #include <dt-bindings/clock/sun20i-d1-ccu.h>
+ #include <dt-bindings/reset/sun20i-d1-ccu.h>
+
+ pwm: pwm@2000c00 {
+ compatible = "allwinner,sun20i-d1-pwm";
+ reg = <0x02000c00 0x400>;
+ clocks = <&ccu CLK_BUS_PWM>, <&dcxo>, <&ccu CLK_APB0>;
+ clock-names = "bus", "hosc", "apb0";
+ resets = <&ccu RST_BUS_PWM>;
+ #pwm-cells = <0x3>;
+ };
+
+...
--

2.25.1

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by Aleksandr Shubin <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +27 -0

new file mode 100644
index 000000000000..716f75776006
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <[email protected]>
+  - Brandon Cheo Fusi <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:

Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +51 -0

+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun20i-r329-pwm
+
+    then:
+      required:
+        - allwinner,pwm-channels
+
+    else:
+      properties:
+        allwinner,pwm-channels: false
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#pwm-cells"
+  - clocks
+  - clock-names
+  - resets
+
+examples:
+  - |
+    #include <dt-bindings/clock/sun20i-d1-ccu.h>
+    #include <dt-bindings/reset/sun20i-d1-ccu.h>
+
+    pwm: pwm@2000c00 {
+      compatible = "allwinner,sun20i-d1-pwm";
+      reg = <0x02000c00 0x400>;
+      clocks = <&ccu CLK_BUS_PWM>, <&dcxo>, <&ccu CLK_APB0>;
+      clock-names = "bus", "hosc", "apb0";
+      resets = <&ccu RST_BUS_PWM>;
+      #pwm-cells = <0x3>;
+    };
+
+...

Signature · 1 line

2.25.1

Allwinner's D1, T113-S3 and R329 SoCs have a quite different PWM controllers with ones supported by pwm-sun4i driver.

This patch adds a PWM controller driver for Allwinner's D1, T113-S3 and R329 SoCs. The main difference between these SoCs is the number of channels defined by the DT property.

Co-developed-by Brandon Cheo Fusi <[email protected]>

Signed-off-by Brandon Cheo Fusi <[email protected]>

Signed-off-by Aleksandr Shubin <[email protected]>

---
 drivers/pwm/Kconfig      |  10 ++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-sun20i.c | 380 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 391 insertions(+)
 create mode 100644 drivers/pwm/pwm-sun20i.c

drivers/pwm/pwm-sun20i.c new file +0 -0

new file mode 100644
index 000000000000..19bf3f495155
@@ -0,0 +1,380 @@

drivers/pwm/pwm-sun20i.c new file +2 -0

+static int sun20i_pwm_probe(struct platform_device *pdev)
+{

drivers/pwm/pwm-sun20i.c new file +7 -0

+	sun20i_chip->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(sun20i_chip->rst))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->rst),
+				     "failed to get bus reset\n");
+
+	ret = of_property_read_u32(pdev->dev.of_node, "allwinner,pwm-channels",
+				   &sun20i_chip->chip.npwm);

drivers/pwm/pwm-sun20i.c new file +10 -0

+#define SUN20I_PWM_PERIOD(chan)			(0x104 + (chan) * 0x20)
+#define SUN20I_PWM_PERIOD_ENTIRE_CYCLE		GENMASK(31, 16)
+#define SUN20I_PWM_PERIOD_ACT_CYCLE		GENMASK(15, 0)
+
+#define SUN20I_PWM_PCNTR_SIZE			BIT(16)
+
+/**
+ * SUN20I_PWM_MAGIC is used to quickly compute the values of the clock dividers
+ * div_m (SUN20I_PWM_CLK_CFG_DIV_M) & prescale_k (SUN20I_PWM_CTL_PRESCAL_K)
+ * without using a loop. These dividers limit the # of cycles in a period

+ * period:
+ *
+ * for a given value of div_m we want the smallest prescale_k such that
+ *
+ * (val >> div_m) // (prescale_k + 1) ≤ 65536 (SUN20I_PWM_PCNTR_SIZE)
+ *
+ * This is equivalent to:
+ *
+ * (val >> div_m) ≤ 65536 * (prescale_k + 1) + prescale_k
+ * ⟺ (val >> div_m) ≤ 65537 * prescale_k + 65536
+ * ⟺ (val >> div_m) - 65536 ≤ 65537 * prescale_k
+ * ⟺ ((val >> div_m) - 65536) / 65537 ≤ prescale_k
+ *
+ * As prescale_k is integer, this becomes
+ *
+ * ((val >> div_m) - 65536) // 65537 ≤ prescale_k
+ *
+ * And is minimized at
+ *
+ * ((val >> div_m) - 65536) // 65537
+ *
+ * Now we pick the smallest div_m that satifies prescale_k ≤ 255
+ * (i.e SUN20I_PWM_CTL_PRESCAL_K_MAX),

+ *
+ * ((val >> div_m) - 65536) // 65537 ≤ 255
+ * ⟺ (val >> div_m) - 65536 ≤ 255 * 65537 + 65536
+ * ⟺ val >> div_m ≤ 255 * 65537 + 2 * 65536
+ * ⟺ val >> div_m < (255 * 65537 + 2 * 65536 + 1)
+ * ⟺ div_m = fls((val) / (255 * 65537 + 2 * 65536 + 1))
+ *
+ * Suggested by Uwe Kleine-König

+ */

drivers/pwm/pwm-sun20i.c new file +67 -0

+#define SUN20I_PWM_MAGIC			(255 * 65537 + 2 * 65536 + 1)
+
+struct sun20i_pwm_chip {
+	struct clk *clk_bus, *clk_hosc, *clk_apb0;
+	struct reset_control *rst;
+	struct pwm_chip chip;
+	void __iomem *base;
+	/* Mutex to protect pwm apply state */
+	struct mutex mutex;
+};
+
+static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct sun20i_pwm_chip, chip);
+}
+
+static inline u32 sun20i_pwm_readl(struct sun20i_pwm_chip *chip,
+				   unsigned long offset)
+{
+	return readl(chip->base + offset);
+}
+
+static inline void sun20i_pwm_writel(struct sun20i_pwm_chip *chip,
+				     u32 val, unsigned long offset)
+{
+	writel(val, chip->base + offset);
+}
+
+static int sun20i_pwm_get_state(struct pwm_chip *chip,
+				struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
+	u16 ent_cycle, act_cycle, prescale_k;
+	u64 clk_rate, tmp;
+	u8 div_m;
+	u32 val;
+
+	mutex_lock(&sun20i_chip->mutex);
+
+	val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CLK_CFG(pwm->hwpwm));
+	div_m = FIELD_GET(SUN20I_PWM_CLK_CFG_DIV_M, val);
+	if (div_m > SUN20I_PWM_CLK_DIV_M_MAX)
+		div_m = SUN20I_PWM_CLK_DIV_M_MAX;
+
+	if (FIELD_GET(SUN20I_PWM_CLK_CFG_SRC, val) == 0)
+		clk_rate = clk_get_rate(sun20i_chip->clk_hosc);
+	else
+		clk_rate = clk_get_rate(sun20i_chip->clk_apb0);
+
+	val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CTL(pwm->hwpwm));
+	state->polarity = (SUN20I_PWM_CTL_ACT_STA & val) ?
+			   PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED;
+
+	prescale_k = FIELD_GET(SUN20I_PWM_CTL_PRESCAL_K, val) + 1;
+
+	val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_ENABLE);
+	state->enabled = (SUN20I_PWM_ENABLE_EN(pwm->hwpwm) & val) ? true : false;
+
+	val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_PERIOD(pwm->hwpwm));
+
+	mutex_unlock(&sun20i_chip->mutex);
+
+	act_cycle = FIELD_GET(SUN20I_PWM_PERIOD_ACT_CYCLE, val);
+	ent_cycle = FIELD_GET(SUN20I_PWM_PERIOD_ENTIRE_CYCLE, val);
+
+	/*

drivers/pwm/pwm-sun20i.c new file +41 -0

+static const struct pwm_ops sun20i_pwm_ops = {
+	.apply = sun20i_pwm_apply,
+	.get_state = sun20i_pwm_get_state,
+};
+
+static const struct of_device_id sun20i_pwm_dt_ids[] = {
+	{ .compatible = "allwinner,sun20i-d1-pwm" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun20i_pwm_dt_ids);
+
+static int sun20i_pwm_probe(struct platform_device *pdev)
+{
+	struct sun20i_pwm_chip *sun20i_chip;
+	int ret;
+
+	sun20i_chip = devm_kzalloc(&pdev->dev, sizeof(*sun20i_chip), GFP_KERNEL);
+	if (!sun20i_chip)
+		return -ENOMEM;
+
+	sun20i_chip->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(sun20i_chip->base))
+		return PTR_ERR(sun20i_chip->base);
+
+	sun20i_chip->clk_bus = devm_clk_get_enabled(&pdev->dev, "bus");
+	if (IS_ERR(sun20i_chip->clk_bus))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_bus),
+				     "failed to get bus clock\n");
+
+	sun20i_chip->clk_hosc = devm_clk_get_enabled(&pdev->dev, "hosc");
+	if (IS_ERR(sun20i_chip->clk_hosc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_hosc),
+				     "failed to get hosc clock\n");
+
+	sun20i_chip->clk_apb0 = devm_clk_get_enabled(&pdev->dev, "apb0");
+	if (IS_ERR(sun20i_chip->clk_apb0))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_apb0),
+				     "failed to get apb0 clock\n");
+
+	sun20i_chip->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(sun20i_chip->rst))

hello Aleksandr,

drivers/pwm/pwm-sun20i.c line 18

Quoted 6 lines · Aleksandr Shubin

drivers/pwm/pwm-sun20i.c new file +6 -0

+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>

Some time ago there was some effort by Rob Herring to detangle the headers platform_device.h, of_device.h and of.h. See for example commit 87e51b76c9db8c29cde573af0faf5a3e13e23960. I think you should use linux/of.h instead of linux/of_device.h.

drivers/pwm/pwm-sun20i.c line 26

Quoted 8 lines

drivers/pwm/pwm-sun20i.c new file +8 -0

+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+
+#define SUN20I_PWM_CLK_CFG(chan)             (0x20 + (((chan) >> 1) * 0x4))
+#define SUN20I_PWM_CLK_CFG_SRC                       GENMASK(8, 7)
+#define SUN20I_PWM_CLK_CFG_DIV_M             GENMASK(3, 0)
+#define SUN20I_PWM_CLK_DIV_M_MAX             8

SUN20I_PWM_CLK_CFG_DIV_M_MAX?

drivers/pwm/pwm-sun20i.c line 38

Quoted 11 lines

drivers/pwm/pwm-sun20i.c new file +11 -0

+#define SUN20I_PWM_CLK_GATE                  0x40
+#define SUN20I_PWM_CLK_GATE_BYPASS(chan)     BIT((chan) + 16)
+#define SUN20I_PWM_CLK_GATE_GATING(chan)     BIT(chan)
+
+#define SUN20I_PWM_ENABLE                    0x80
+#define SUN20I_PWM_ENABLE_EN(chan)           BIT(chan)
+
+#define SUN20I_PWM_CTL(chan)                 (0x100 + (chan) * 0x20)
+#define SUN20I_PWM_CTL_ACT_STA                       BIT(8)
+#define UN20I_PWM_CTL_PRESCAL_K              GENMASK(7, 0)
+#define SUN20I_PWM_CTL_PRESCAL_K_MAX         0xff

This matches the theoretical maximum for GENMASK(7,0), so you could make use of field_max(SUN20I_PWM_CTL_PRESCAL_K) here.

Quoted 14 lines

drivers/pwm/pwm-sun20i.c new file +14 -0

+#define SUN20I_PWM_PERIOD(chan)                      (0x104 + (chan) * 0x20)
+#define SUN20I_PWM_PERIOD_ENTIRE_CYCLE               GENMASK(31, 16)
+#define SUN20I_PWM_PERIOD_ACT_CYCLE          GENMASK(15, 0)
+
+#define SUN20I_PWM_PCNTR_SIZE                        BIT(16)
+
+/**
+ * SUN20I_PWM_MAGIC is used to quickly compute the values of the clock dividers
+ * div_m (SUN20I_PWM_CLK_CFG_DIV_M) & prescale_k (SUN20I_PWM_CTL_PRESCAL_K)
+ * without using a loop. These dividers limit the # of cycles in a period
+ * to SUN20I_PWM_PCNTR_SIZE by applying a scaling factor of
+ * 1/(div_m * (prescale_k + 1)) to the clock source.
+ *
+ * SUN20I_PWM_MAGIC is derived by solving for div_m and prescale_k

Quoted 6 lines

+ * such that for a given requested period,
+ *
+ * i) div_m is minimized for any prescale_k ≤ SUN20I_PWM_CTL_PRESCAL_K_MAX,
+ * ii) prescale_k is minimized.
+ *
+ * The derivation proceeds as follows, with val = # of cycles for reqested

s/reqested/requested/

Quoted 23 lines

+ * period:
+ *
+ * for a given value of div_m we want the smallest prescale_k such that
+ *
+ * (val >> div_m) // (prescale_k + 1) ≤ 65536 (SUN20I_PWM_PCNTR_SIZE)
+ *
+ * This is equivalent to:
+ *
+ * (val >> div_m) ≤ 65536 * (prescale_k + 1) + prescale_k
+ * ⟺ (val >> div_m) ≤ 65537 * prescale_k + 65536
+ * ⟺ (val >> div_m) - 65536 ≤ 65537 * prescale_k
+ * ⟺ ((val >> div_m) - 65536) / 65537 ≤ prescale_k
+ *
+ * As prescale_k is integer, this becomes
+ *
+ * ((val >> div_m) - 65536) // 65537 ≤ prescale_k
+ *
+ * And is minimized at
+ *
+ * ((val >> div_m) - 65536) // 65537
+ *
+ * Now we pick the smallest div_m that satifies prescale_k ≤ 255
+ * (i.e SUN20I_PWM_CTL_PRESCAL_K_MAX),

Quoted 8 lines

+ *
+ * ((val >> div_m) - 65536) // 65537 ≤ 255
+ * ⟺ (val >> div_m) - 65536 ≤ 255 * 65537 + 65536
+ * ⟺ val >> div_m ≤ 255 * 65537 + 2 * 65536
+ * ⟺ val >> div_m < (255 * 65537 + 2 * 65536 + 1)
+ * ⟺ div_m = fls((val) / (255 * 65537 + 2 * 65536 + 1))
+ *
+ * Suggested by Uwe Kleine-König

Good man, I assume this is all sane then :-)

Quoted 68 lines

+ */

drivers/pwm/pwm-sun20i.c new file +67 -0

+#define SUN20I_PWM_MAGIC                     (255 * 65537 + 2 * 65536 + 1)
+
+struct sun20i_pwm_chip {
+     struct clk *clk_bus, *clk_hosc, *clk_apb0;
+     struct reset_control *rst;
+     struct pwm_chip chip;
+     void __iomem *base;
+     /* Mutex to protect pwm apply state */
+     struct mutex mutex;
+};
+
+static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip)
+{
+     return container_of(chip, struct sun20i_pwm_chip, chip);
+}
+
+static inline u32 sun20i_pwm_readl(struct sun20i_pwm_chip *chip,
+                                unsigned long offset)
+{
+     return readl(chip->base + offset);
+}
+
+static inline void sun20i_pwm_writel(struct sun20i_pwm_chip *chip,
+                                  u32 val, unsigned long offset)
+{
+     writel(val, chip->base + offset);
+}
+
+static int sun20i_pwm_get_state(struct pwm_chip *chip,
+                             struct pwm_device *pwm,
+                             struct pwm_state *state)
+{
+     struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
+     u16 ent_cycle, act_cycle, prescale_k;
+     u64 clk_rate, tmp;
+     u8 div_m;
+     u32 val;
+
+     mutex_lock(&sun20i_chip->mutex);
+
+     val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CLK_CFG(pwm->hwpwm));
+     div_m = FIELD_GET(SUN20I_PWM_CLK_CFG_DIV_M, val);
+     if (div_m > SUN20I_PWM_CLK_DIV_M_MAX)
+             div_m = SUN20I_PWM_CLK_DIV_M_MAX;
+
+     if (FIELD_GET(SUN20I_PWM_CLK_CFG_SRC, val) == 0)
+             clk_rate = clk_get_rate(sun20i_chip->clk_hosc);
+     else
+             clk_rate = clk_get_rate(sun20i_chip->clk_apb0);
+
+     val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CTL(pwm->hwpwm));
+     state->polarity = (SUN20I_PWM_CTL_ACT_STA & val) ?
+                        PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED;
+
+     prescale_k = FIELD_GET(SUN20I_PWM_CTL_PRESCAL_K, val) + 1;
+
+     val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_ENABLE);
+     state->enabled = (SUN20I_PWM_ENABLE_EN(pwm->hwpwm) & val) ? true : false;
+
+     val = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_PERIOD(pwm->hwpwm));
+
+     mutex_unlock(&sun20i_chip->mutex);
+
+     act_cycle = FIELD_GET(SUN20I_PWM_PERIOD_ACT_CYCLE, val);
+     ent_cycle = FIELD_GET(SUN20I_PWM_PERIOD_ENTIRE_CYCLE, val);
+
+     /*

drivers/pwm/pwm-sun20i.c line 168

Quoted 10 lines

drivers/pwm/pwm-sun20i.c new file +10 -0

+      * The duration of the active phase should not be longer
+      * than the duration of the period
+      */
+     if (act_cycle > ent_cycle)
+             act_cycle = ent_cycle;
+
+     tmp = ((u64)(act_cycle) * prescale_k << div_m) * NSEC_PER_SEC;
+     state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate);
+     tmp = ((u64)(ent_cycle) * prescale_k << div_m) * NSEC_PER_SEC;
+     state->period = DIV_ROUND_UP_ULL(tmp, clk_rate);

Please add a comment above this block that justifies assuming that the multiplication doesn't overflow. Something like:

        We have act_cycle <= ent_cycle <= 0xffff, prescale_k <= 0x100, div_m <= 8. So the multiplication fits into an u64 without overflow.

drivers/pwm/pwm-sun20i.c line 195

Quoted 9 lines

drivers/pwm/pwm-sun20i.c new file +9 -0

+
+     return 0;
+}
+
+static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+                         const struct pwm_state *state)
+{
+...
+}

I didn't recheck all the logic in .apply in detail and will assume it is sane for this round.

Quoted 41 lines

drivers/pwm/pwm-sun20i.c new file +41 -0

+static const struct pwm_ops sun20i_pwm_ops = {
+     .apply = sun20i_pwm_apply,
+     .get_state = sun20i_pwm_get_state,
+};
+
+static const struct of_device_id sun20i_pwm_dt_ids[] = {
+     { .compatible = "allwinner,sun20i-d1-pwm" },
+     { },
+};
+MODULE_DEVICE_TABLE(of, sun20i_pwm_dt_ids);
+
+static int sun20i_pwm_probe(struct platform_device *pdev)
+{
+     struct sun20i_pwm_chip *sun20i_chip;
+     int ret;
+
+     sun20i_chip = devm_kzalloc(&pdev->dev, sizeof(*sun20i_chip), GFP_KERNEL);
+     if (!sun20i_chip)
+             return -ENOMEM;
+
+     sun20i_chip->base = devm_platform_ioremap_resource(pdev, 0);
+     if (IS_ERR(sun20i_chip->base))
+             return PTR_ERR(sun20i_chip->base);
+
+     sun20i_chip->clk_bus = devm_clk_get_enabled(&pdev->dev, "bus");
+     if (IS_ERR(sun20i_chip->clk_bus))
+             return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_bus),
+                                  "failed to get bus clock\n");
+
+     sun20i_chip->clk_hosc = devm_clk_get_enabled(&pdev->dev, "hosc");
+     if (IS_ERR(sun20i_chip->clk_hosc))
+             return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_hosc),
+                                  "failed to get hosc clock\n");
+
+     sun20i_chip->clk_apb0 = devm_clk_get_enabled(&pdev->dev, "apb0");
+     if (IS_ERR(sun20i_chip->clk_apb0))
+             return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_apb0),
+                                  "failed to get apb0 clock\n");
+
+     sun20i_chip->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+     if (IS_ERR(sun20i_chip->rst))

drivers/pwm/pwm-sun20i.c line 336

Quoted 10 lines

drivers/pwm/pwm-sun20i.c new file +10 -0

+             return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->rst),
+                                  "failed to get bus reset\n");
+
+     ret = of_property_read_u32(pdev->dev.of_node, "allwinner,pwm-channels",
+                                &sun20i_chip->chip.npwm);
+     if (ret)
+             sun20i_chip->chip.npwm = 8;
+
+     if (sun20i_chip->chip.npwm > 16)
+             sun20i_chip->chip.npwm = 16;

Is it worth to emit an error message here? Something like:

        Limiting number of PWM lines from %u to 16

Best regards
Uwe

--
Pengutronix e.K.                           | Uwe Kleine-König            | Industrial Linux Solutions                 | https://www.pengutronix.de/ |

drivers/pwm/pwm-sun20i.c new file +29 -0

+	if (state->polarity != pwm->state.polarity ||
+	    state->duty_cycle != pwm->state.duty_cycle ||
+	    state->period != pwm->state.period) {
+		ctl = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CTL(pwm->hwpwm));
+		clk_cfg = sun20i_pwm_readl(sun20i_chip, SUN20I_PWM_CLK_CFG(pwm->hwpwm));
+		hosc_rate = clk_get_rate(sun20i_chip->clk_hosc);
+		bus_rate = clk_get_rate(sun20i_chip->clk_apb0);
+		if (pwm_en & SUN20I_PWM_ENABLE_EN(pwm->hwpwm ^ 1)) {
+			/* if the neighbor channel is enable, check period only */
+			use_bus_clk = FIELD_GET(SUN20I_PWM_CLK_CFG_SRC, clk_cfg) != 0;
+			val = mul_u64_u64_div_u64(state->period,
+						  (use_bus_clk ? bus_rate : hosc_rate),
+						  NSEC_PER_SEC);
+
+			div_m = FIELD_GET(SUN20I_PWM_CLK_CFG_DIV_M, clk_cfg);
+		} else {
+			/* check period and select clock source */
+			use_bus_clk = false;
+			val = mul_u64_u64_div_u64(state->period, hosc_rate, NSEC_PER_SEC);
+			if (val <= 1) {
+				use_bus_clk = true;
+				val = mul_u64_u64_div_u64(state->period, bus_rate, NSEC_PER_SEC);
+				if (val <= 1) {
+					ret = -EINVAL;
+					goto unlock_mutex;
+				}
+			}
+			div_m = fls(DIV_ROUND_DOWN_ULL(val, SUN20I_PWM_MAGIC));
+			if (div_m > SUN20I_PWM_CLK_DIV_M_MAX) {

Signature · 7 lines

2.25.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

D1 and T113s contain a pwm controller with 8 channels. This controller is supported by the sun20i-pwm driver.

Add a device tree node for it.

Signed-off-by Aleksandr Shubin <[email protected]>

---
 arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi +12 -0

index 5a9d7f5a75b4..435a1e66aa6a 100644
@@ -145,6 +145,18 @@ uart3_pb_pins: uart3-pb-pins {
 			};
 		};
 
+		pwm: pwm@2000c00 {
+			compatible = "allwinner,sun20i-d1-pwm";
+			reg = <0x02000c00 0x400>;
+			clocks = <&ccu CLK_BUS_PWM>,
+				 <&dcxo>,
+				 <&ccu CLK_APB0>;
+			clock-names = "bus", "hosc", "apb0";
+			resets = <&ccu RST_BUS_PWM>;
+			status = "disabled";
+			#pwm-cells = <0x3>;
+		};
+
 		ccu: clock-controller@2001000 {
 			compatible = "allwinner,sun20i-d1-ccu";
 			reg = <0x2001000 0x1000>;