[PATCH 0/5] drivers: pwm: sun4i: Improve support for A64 and H6 SoCs

[PATCH 0/5] drivers: pwm: sun4i: Improve support for A64 and H6 SoCs

[Andre Przywara]

This series adds PWM support for new Allwinner SoCs. Actually the A64 PWM 
is fully compatible with the A13 and H3 PWM IP, so the driver does not
need any additional code. But I use this opportunity to provide some
cleanup and to add optional reset controller support, which will be needed
for the H6.
Patch 1 removes a no longer used parameter from our per-SoC data structure,
to simplify patch 2, which groups SoCs with a compatible PWM controller.
Patch 3 adds optional reset controller support, the line will be deasserted
and asserted at the same time we enable and disable the clock. Patch 4 adds
the new compatible strings to the binding documentation (and just there,
we expect to use "allwinner,sun5i-a13-pwm" as a fallback compatible string).
The final patch 5 adds the respective PWM nodes to the A64 .dtsi.
This eventually does not enable the PWM on any new board at the moment, as
the PWM pins are either not usable (muxed with Ethernet) or exposed on
a header pin not dedicated to PWM. But the Pinebook (and Teres I) should be
able to use the PWM for the LCD backlights, plus users can enable the
R_PWM on their Pine64 boards, if they like.

Tested by manually enabling r_pwm on a Pine64-LTS.

Let me know how you like it and what needs to change.

Cheers,
Andre.

Andre Przywara (5):
  pwm: sun4i: drop unused .has_rdy member
  pwm: sun4i: simplify controller mapping
  pwm: sun4i: Introduce (optional) reset support
  dt-bindings: pwm: sunxi: add new compatible strings
  dts: sunxi: A64: Add PWM controllers

 .../devicetree/bindings/pwm/pwm-sun4i.txt          |  6 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      | 28 +++++++++
 drivers/pwm/pwm-sun4i.c                            | 66 ++++++++++++----------
 3 files changed, 69 insertions(+), 31 deletions(-)

-- 
2.14.1

[PATCH 1/5] pwm: sun4i: drop unused .has_rdy member

[Andre Przywara]

Commit a054c4d68408 ("pwm: sun4i: Drop legacy callbacks") dropped the
only user of the .has_rdy member in our sun4i_pwm_data struct.
Consequently we don't need to store this anymore for the various SoCs,
which paves the way for further simplifications.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/pwm/pwm-sun4i.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 334199c58f1d..b3e4a4b3774d 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -73,7 +73,6 @@ static const u32 prescaler_table[] = {
 
 struct sun4i_pwm_data {
 	bool has_prescaler_bypass;
-	bool has_rdy;
 	unsigned int npwm;
 };
 
@@ -313,31 +312,26 @@ static const struct pwm_ops sun4i_pwm_ops = {
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
 	.has_prescaler_bypass = false,
-	.has_rdy = false,
 	.npwm = 2,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
 	.has_prescaler_bypass = true,
-	.has_rdy = true,
 	.npwm = 2,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
 	.has_prescaler_bypass = true,
-	.has_rdy = true,
 	.npwm = 1,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
 	.has_prescaler_bypass = true,
-	.has_rdy = true,
 	.npwm = 2,
 };
 
 static const struct sun4i_pwm_data sun4i_pwm_data_h3 = {
 	.has_prescaler_bypass = true,
-	.has_rdy = true,
 	.npwm = 1,
 };
 
-- 
2.14.1

Re: [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 583 bytes --]

On Wed, Mar 07, 2018 at 02:07:15AM +0000, Andre Przywara wrote:
> Commit a054c4d68408 ("pwm: sun4i: Drop legacy callbacks") dropped the
> only user of the .has_rdy member in our sun4i_pwm_data struct.
> Consequently we don't need to store this anymore for the various SoCs,
> which paves the way for further simplifications.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] pwm: sun4i: simplify controller mapping

[Andre Przywara]

At the moment we assign our supported compatible strings to a respective
instance of our sun4i_pwm_data structure, even though some of them
are the same.
To avoid further clutter, split out the three different combinations of
features we have at the moment and name them accordingly.
This should make it more obvious which compatible string to use for new
SoCs.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/pwm/pwm-sun4i.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index b3e4a4b3774d..078172dee462 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -310,27 +310,17 @@ static const struct pwm_ops sun4i_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
-static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
+static const struct sun4i_pwm_data sun4i_pwm_dual_nobypass = {
 	.has_prescaler_bypass = false,
 	.npwm = 2,
 };
 
-static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
+static const struct sun4i_pwm_data sun4i_pwm_dual_bypass = {
 	.has_prescaler_bypass = true,
 	.npwm = 2,
 };
 
-static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
-	.has_prescaler_bypass = true,
-	.npwm = 1,
-};
-
-static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
-	.has_prescaler_bypass = true,
-	.npwm = 2,
-};
-
-static const struct sun4i_pwm_data sun4i_pwm_data_h3 = {
+static const struct sun4i_pwm_data sun4i_pwm_single_bypass = {
 	.has_prescaler_bypass = true,
 	.npwm = 1,
 };
@@ -338,19 +328,19 @@ static const struct sun4i_pwm_data sun4i_pwm_data_h3 = {
 static const struct of_device_id sun4i_pwm_dt_ids[] = {
 	{
 		.compatible = "allwinner,sun4i-a10-pwm",
-		.data = &sun4i_pwm_data_a10,
+		.data = &sun4i_pwm_dual_nobypass,
 	}, {
 		.compatible = "allwinner,sun5i-a10s-pwm",
-		.data = &sun4i_pwm_data_a10s,
+		.data = &sun4i_pwm_dual_bypass,
 	}, {
 		.compatible = "allwinner,sun5i-a13-pwm",
-		.data = &sun4i_pwm_data_a13,
+		.data = &sun4i_pwm_single_bypass,
 	}, {
 		.compatible = "allwinner,sun7i-a20-pwm",
-		.data = &sun4i_pwm_data_a20,
+		.data = &sun4i_pwm_dual_bypass,
 	}, {
 		.compatible = "allwinner,sun8i-h3-pwm",
-		.data = &sun4i_pwm_data_h3,
+		.data = &sun4i_pwm_single_bypass,
 	}, {
 		/* sentinel */
 	},
-- 
2.14.1

Re: [PATCH 2/5] pwm: sun4i: simplify controller mapping

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 690 bytes --]

On Wed, Mar 07, 2018 at 02:07:16AM +0000, Andre Przywara wrote:
> At the moment we assign our supported compatible strings to a respective
> instance of our sun4i_pwm_data structure, even though some of them
> are the same.
> To avoid further clutter, split out the three different combinations of
> features we have at the moment and name them accordingly.
> This should make it more obvious which compatible string to use for new
> SoCs.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] pwm: sun4i: Introduce (optional) reset support

[Andre Przywara]

While the PWM IP in the Allwinner H6 SoC is fully compatible to those
used in older SoCs (H3, A64), it features a dedicated reset line which
needs to be de-asserted.
Add support for an optional "resets" DT property in our pwm-sun4i probe
routine, and assert and de-assert the reset line, where needed.
This allows to enable PWM support on the H6.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/pwm/pwm-sun4i.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 078172dee462..4eefb27fe80b 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -17,6 +17,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/time.h>
@@ -79,6 +80,7 @@ struct sun4i_pwm_data {
 struct sun4i_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
+	struct reset_control *reset;
 	void __iomem *base;
 	spinlock_t ctrl_lock;
 	const struct sun4i_pwm_data *data;
@@ -206,7 +208,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
 	struct pwm_state cstate;
 	u32 ctrl;
-	int ret;
+	int ret = 0;
 	unsigned int delay_us;
 	unsigned long now;
 
@@ -218,6 +220,18 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			dev_err(chip->dev, "failed to enable PWM clock\n");
 			return ret;
 		}
+
+		/* Deassert reset if we have a reset control */
+		if (sun4i_pwm->reset) {
+			ret = reset_control_deassert(sun4i_pwm->reset);
+			if (ret) {
+				dev_err(chip->dev,
+					"Cannot deassert reset control\n");
+				clk_disable_unprepare(sun4i_pwm->clk);
+
+				return ret;
+			}
+		}
 	}
 
 	spin_lock(&sun4i_pwm->ctrl_lock);
@@ -234,7 +248,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			dev_err(chip->dev, "period exceeds the maximum value\n");
 			spin_unlock(&sun4i_pwm->ctrl_lock);
 			if (!cstate.enabled)
-				clk_disable_unprepare(sun4i_pwm->clk);
+				goto out_disable;
+
 			return ret;
 		}
 
@@ -274,10 +289,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (state->enabled)
 		return 0;
 
-	if (!sun4i_pwm->needs_delay[pwm->hwpwm]) {
-		clk_disable_unprepare(sun4i_pwm->clk);
-		return 0;
-	}
+	if (!sun4i_pwm->needs_delay[pwm->hwpwm])
+		goto out_disable;
 
 	/* We need a full period to elapse before disabling the channel. */
 	now = jiffies;
@@ -299,9 +312,12 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
 	spin_unlock(&sun4i_pwm->ctrl_lock);
 
+out_disable:
 	clk_disable_unprepare(sun4i_pwm->clk);
+	if (sun4i_pwm->reset)
+		reset_control_assert(sun4i_pwm->reset);
 
-	return 0;
+	return ret;
 }
 
 static const struct pwm_ops sun4i_pwm_ops = {
@@ -370,6 +386,10 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(pwm->clk))
 		return PTR_ERR(pwm->clk);
 
+	pwm->reset = devm_reset_control_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(pwm->reset))
+		return PTR_ERR(pwm->reset);
+
 	pwm->chip.dev = &pdev->dev;
 	pwm->chip.ops = &sun4i_pwm_ops;
 	pwm->chip.base = -1;
-- 
2.14.1

Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 675 bytes --]

On Wed, Mar 07, 2018 at 02:07:17AM +0000, Andre Przywara wrote:
> While the PWM IP in the Allwinner H6 SoC is fully compatible to those
> used in older SoCs (H3, A64), it features a dedicated reset line which
> needs to be de-asserted.
> Add support for an optional "resets" DT property in our pwm-sun4i probe
> routine, and assert and de-assert the reset line, where needed.
> This allows to enable PWM support on the H6.

This isn't optional then. It's mandatory on the H6, and unneeded on
everything else. This is what we should have.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support

[Andre Przywara]

Hi Maxime,

thanks for looking into this and for the Acks!

On 07/03/18 07:45, Maxime Ripard wrote:
> On Wed, Mar 07, 2018 at 02:07:17AM +0000, Andre Przywara wrote:
>> While the PWM IP in the Allwinner H6 SoC is fully compatible to those
>> used in older SoCs (H3, A64), it features a dedicated reset line which
>> needs to be de-asserted.
>> Add support for an optional "resets" DT property in our pwm-sun4i probe
>> routine, and assert and de-assert the reset line, where needed.
>> This allows to enable PWM support on the H6.
> 
> This isn't optional then. It's mandatory on the H6, and unneeded on
> everything else. This is what we should have.

So are you aiming at:
	if (of_device_is_compatible(np, "allwinner,sun50i-h6-pwm")) {
		... devm_reset_control_get() ...

I guess this is preferable over coding something based on a new member
in struct sun4i_pwm_data?

Cheers,
Andre.

Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

On Tue, Mar 13, 2018 at 02:05:34PM +0000, Andre Przywara wrote:
> Hi Maxime,
> 
> thanks for looking into this and for the Acks!
> 
> On 07/03/18 07:45, Maxime Ripard wrote:
> > On Wed, Mar 07, 2018 at 02:07:17AM +0000, Andre Przywara wrote:
> >> While the PWM IP in the Allwinner H6 SoC is fully compatible to those
> >> used in older SoCs (H3, A64), it features a dedicated reset line which
> >> needs to be de-asserted.
> >> Add support for an optional "resets" DT property in our pwm-sun4i probe
> >> routine, and assert and de-assert the reset line, where needed.
> >> This allows to enable PWM support on the H6.
> > 
> > This isn't optional then. It's mandatory on the H6, and unneeded on
> > everything else. This is what we should have.
> 
> So are you aiming at:
> 	if (of_device_is_compatible(np, "allwinner,sun50i-h6-pwm")) {
> 		... devm_reset_control_get() ...
> 
> I guess this is preferable over coding something based on a new member
> in struct sun4i_pwm_data?

It's basically a long term vs short term debate :)

If we're thinking short term, then yes, sure it would make
sense. However, if we start having more and more SoCs, we'll have a
longer and longer condition.

Since we already have a structure available, I'd still prefer to go
for the structure flag. This is faster (no string comparison), it'll
be easier to extend, and the patch size is pretty much the same.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

[Andre Przywara]

The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
compatible to the PWM controllers found in the A13 and H3.
Add new compatible strings for those SoCs to the binding document, so
that they can be safely used, together with a fallback string
(preferably "allwinner,sun5i-a13-pwm").
Add add the optionals "resets" property, needed by the H6.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
index 51ff54c8b8ef..b3a127a0e58c 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
@@ -7,11 +7,17 @@ Required properties:
     - "allwinner,sun5i-a13-pwm"
     - "allwinner,sun7i-a20-pwm"
     - "allwinner,sun8i-h3-pwm"
+    - "allwinner,sun50i-a64-pwm"
+    - "allwinner,sun50i-h5-pwm"
+    - "allwinner,sun50i-h6-pwm"
   - reg: physical base address and length of the controller's registers
   - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
     the cells format.
   - clocks: From common clock binding, handle to the parent clock.
 
+Optional properties:
+  - resets: shall be the reset control phandle for the PWM block, if required.
+
 Example:
 
 	pwm: pwm@1c20e00 {
-- 
2.14.1

Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

[Rob Herring]

On Wed, Mar 07, 2018 at 02:07:18AM +0000, Andre Przywara wrote:
> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
> compatible to the PWM controllers found in the A13 and H3.

If fully compatible, then shouldn't there be a fallback to one of the 
existing compatible strings?

> Add new compatible strings for those SoCs to the binding document, so
> that they can be safely used, together with a fallback string
> (preferably "allwinner,sun5i-a13-pwm").
> Add add the optionals "resets" property, needed by the H6.
> 
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
> index 51ff54c8b8ef..b3a127a0e58c 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
> @@ -7,11 +7,17 @@ Required properties:
>      - "allwinner,sun5i-a13-pwm"
>      - "allwinner,sun7i-a20-pwm"
>      - "allwinner,sun8i-h3-pwm"
> +    - "allwinner,sun50i-a64-pwm"
> +    - "allwinner,sun50i-h5-pwm"
> +    - "allwinner,sun50i-h6-pwm"
>    - reg: physical base address and length of the controller's registers
>    - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
>      the cells format.
>    - clocks: From common clock binding, handle to the parent clock.
>  
> +Optional properties:
> +  - resets: shall be the reset control phandle for the PWM block, if required.
> +
>  Example:
>  
>  	pwm: pwm@1c20e00 {
> -- 
> 2.14.1
> 

Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

[Andre Przywara]

Hi,

On 08/03/18 02:08, Rob Herring wrote:
> On Wed, Mar 07, 2018 at 02:07:18AM +0000, Andre Przywara wrote:
>> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
>> compatible to the PWM controllers found in the A13 and H3.
> 
> If fully compatible, then shouldn't there be a fallback to one of the 
> existing compatible strings?

Yes, that was the idea. I was already wondering how we would
differentiate that from "real" compatible strings, but couldn't find
convincing examples. So should that read:

- compatible: should at least contain be one of:
    - "allwinner,sun4i-a10-pwm"
    - "allwinner,sun5i-a10s-pwm"
    - "allwinner,sun5i-a13-pwm"
    - "allwinner,sun7i-a20-pwm"
    - "allwinner,sun8i-h3-pwm"
  May in addition contain one of:
    - "allwinner,sun50i-a64-pwm"
    - "allwinner,sun50i-h5-pwm"
    - "allwinner,sun50i-h6-pwm"

And this would possibly need to be amended once we need to actually use
one of those strings (to implement a quirk, for instance)?
Or is there some other way of reserving those compatible strings, which
are not expected to be matched in drivers directly?

Cheers,
Andre.

>> Add new compatible strings for those SoCs to the binding document, so
>> that they can be safely used, together with a fallback string
>> (preferably "allwinner,sun5i-a13-pwm").
>> Add add the optionals "resets" property, needed by the H6.
>>
>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> index 51ff54c8b8ef..b3a127a0e58c 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> @@ -7,11 +7,17 @@ Required properties:
>>      - "allwinner,sun5i-a13-pwm"
>>      - "allwinner,sun7i-a20-pwm"
>>      - "allwinner,sun8i-h3-pwm"
>> +    - "allwinner,sun50i-a64-pwm"
>> +    - "allwinner,sun50i-h5-pwm"
>> +    - "allwinner,sun50i-h6-pwm"
>>    - reg: physical base address and length of the controller's registers
>>    - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
>>      the cells format.
>>    - clocks: From common clock binding, handle to the parent clock.
>>  
>> +Optional properties:
>> +  - resets: shall be the reset control phandle for the PWM block, if required.
>> +
>>  Example:
>>  
>>  	pwm: pwm@1c20e00 {
>> -- 
>> 2.14.1
>>

Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

[Rob Herring]

On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi,
>
> On 08/03/18 02:08, Rob Herring wrote:
>> On Wed, Mar 07, 2018 at 02:07:18AM +0000, Andre Przywara wrote:
>>> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
>>> compatible to the PWM controllers found in the A13 and H3.
>>
>> If fully compatible, then shouldn't there be a fallback to one of the
>> existing compatible strings?
>
> Yes, that was the idea. I was already wondering how we would
> differentiate that from "real" compatible strings, but couldn't find
> convincing examples. So should that read:
>
> - compatible: should at least contain be one of:
>     - "allwinner,sun4i-a10-pwm"
>     - "allwinner,sun5i-a10s-pwm"
>     - "allwinner,sun5i-a13-pwm"
>     - "allwinner,sun7i-a20-pwm"
>     - "allwinner,sun8i-h3-pwm"
>   May in addition contain one of:
>     - "allwinner,sun50i-a64-pwm"
>     - "allwinner,sun50i-h5-pwm"
>     - "allwinner,sun50i-h6-pwm"

I can't validate a dts is correct with it written like this. Just list
each valid combination on each line:

"allwinner,sun8i-h3-pwm", "allwinner,sun50i-h6-pwm"

(Assuming "allwinner,sun50i-h6-pwm" was the first implementation and
h3 has the same block).

> And this would possibly need to be amended once we need to actually use
> one of those strings (to implement a quirk, for instance)?

The most specific compatible provides that. The driver matches on the
least specific one until you have a quirk to implement.

> Or is there some other way of reserving those compatible strings, which
> are not expected to be matched in drivers directly?

The binding should just list all that are appropriate from the start
and the driver is free to match on which ever string it wants.

Rob

Re: Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

[Andre Przywara]

Hi,

On 08/03/18 14:37, Rob Herring wrote:
> On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>> Hi,
>>
>> On 08/03/18 02:08, Rob Herring wrote:
>>> On Wed, Mar 07, 2018 at 02:07:18AM +0000, Andre Przywara wrote:
>>>> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
>>>> compatible to the PWM controllers found in the A13 and H3.
>>>
>>> If fully compatible, then shouldn't there be a fallback to one of the
>>> existing compatible strings?
>>
>> Yes, that was the idea. I was already wondering how we would
>> differentiate that from "real" compatible strings, but couldn't find
>> convincing examples. So should that read:
>>
>> - compatible: should at least contain be one of:
>>     - "allwinner,sun4i-a10-pwm"
>>     - "allwinner,sun5i-a10s-pwm"
>>     - "allwinner,sun5i-a13-pwm"
>>     - "allwinner,sun7i-a20-pwm"
>>     - "allwinner,sun8i-h3-pwm"
>>   May in addition contain one of:
>>     - "allwinner,sun50i-a64-pwm"
>>     - "allwinner,sun50i-h5-pwm"
>>     - "allwinner,sun50i-h6-pwm"
> 
> I can't validate a dts is correct with it written like this. Just list
> each valid combination on each line:
> 
> "allwinner,sun8i-h3-pwm", "allwinner,sun50i-h6-pwm"
> 
> (Assuming "allwinner,sun50i-h6-pwm" was the first implementation and
> h3 has the same block).

Ah, OK, that's makes some sense. I didn't find too many examples outside
of root compatible nodes in the binding docs, but I will use that.

>> And this would possibly need to be amended once we need to actually use
>> one of those strings (to implement a quirk, for instance)?
> 
> The most specific compatible provides that. The driver matches on the
> least specific one until you have a quirk to implement.

Yeah, I was just wondering how a (non-Linux) driver author would induce
what strings are actually needed just by reading the binding. But
enumerating the combinations explicitly should solve this.

Thanks!
Andre.

>> Or is there some other way of reserving those compatible strings, which
>> are not expected to be matched in drivers directly?
> 
> The binding should just list all that are appropriate from the start
> and the driver is free to match on which ever string it wants.
> 
> Rob
> 

[PATCH 5/5] dts: sunxi: A64: Add PWM controllers

[Andre Przywara]

The Allwinner A64 SoC features two PWM controllers, which are fully
compatible to the one used in the A13 and H3 chips.
Add the respective device nodes (one for the "normal" PWM, the other for
the one in the CPUS domain) and the pin their output is connected to.
On the A64 the "normal" PWM is muxed together with one of the MDIO pins
used to communicate with the Ethernet PHY, so it won't be usable on many
boards. But the Pinebook laptop uses this pin for controlling the LCD
backlight.
The CPUS PWM pin however is routed to the "RPi2" header, at the same
location as the PWM pin on the RaspberryPi.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index d783d164b9c3..5dd89d19f744 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -321,6 +321,11 @@
 				bias-pull-up;
 			};
 
+			pwm_pin: pwm_pin {
+				pins = "PD22";
+				function = "pwm";
+			};
+
 			rmii_pins: rmii_pins {
 				pins = "PD10", "PD11", "PD13", "PD14", "PD17",
 				       "PD18", "PD19", "PD20", "PD22", "PD23";
@@ -537,6 +542,15 @@
 			#interrupt-cells = <3>;
 		};
 
+		pwm: pwm@1C21400 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01c21400 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		rtc: rtc@1f00000 {
 			compatible = "allwinner,sun6i-a31-rtc";
 			reg = <0x01f00000 0x54>;
@@ -563,6 +577,15 @@
 			#reset-cells = <1>;
 		};
 
+		r_pwm: pwm@1f03800 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01f03800 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		r_pio: pinctrl@1f02c00 {
 			compatible = "allwinner,sun50i-a64-r-pinctrl";
 			reg = <0x01f02c00 0x400>;
@@ -578,6 +601,11 @@
 				pins = "PL0", "PL1";
 				function = "s_rsb";
 			};
+
+			r_pwm_pin: pwm {
+				pins = "PL10";
+				function = "s_pwm";
+			};
 		};
 
 		r_rsb: rsb@1f03400 {
-- 
2.14.1