All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] drivers: pwm: sun4i: Improve support for A64 and H6 SoCs
@ 2018-03-07  2:07 ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/5] drivers: pwm: sun4i: Improve support for A64 and H6 SoCs
@ 2018-03-07  2:07 ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member
  2018-03-07  2:07 ` Andre Przywara
@ 2018-03-07  2:07     ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member
@ 2018-03-07  2:07     ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/5] pwm: sun4i: simplify controller mapping
  2018-03-07  2:07 ` Andre Przywara
@ 2018-03-07  2:07     ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/5] pwm: sun4i: simplify controller mapping
@ 2018-03-07  2:07     ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
  2018-03-07  2:07 ` Andre Przywara
@ 2018-03-07  2:07     ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
@ 2018-03-07  2:07     ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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@arm.com>
---
 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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
  2018-03-07  2:07 ` Andre Przywara
@ 2018-03-07  2:07     ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
@ 2018-03-07  2:07     ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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@arm.com>
---
 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 at 1c20e00 {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/5] dts: sunxi: A64: Add PWM controllers
  2018-03-07  2:07 ` Andre Przywara
@ 2018-03-07  2:07     ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: Thierry Reding, Maxime Ripard, Chen-Yu Tsai
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/5] dts: sunxi: A64: Add PWM controllers
@ 2018-03-07  2:07     ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-07  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

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@arm.com>
---
 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 at 1C21400 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01c21400 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		rtc: rtc at 1f00000 {
 			compatible = "allwinner,sun6i-a31-rtc";
 			reg = <0x01f00000 0x54>;
@@ -563,6 +577,15 @@
 			#reset-cells = <1>;
 		};
 
+		r_pwm: pwm at 1f03800 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01f03800 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		r_pio: pinctrl at 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 at 1f03400 {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member
  2018-03-07  2:07     ` Andre Przywara
@ 2018-03-07  7:40       ` Maxime Ripard
  -1 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:40 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-pwm, devicetree, linux-sunxi, Chen-Yu Tsai, Thierry Reding,
	linux-arm-kernel


[-- 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member
@ 2018-03-07  7:40       ` Maxime Ripard
  0 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:40 UTC (permalink / raw)
  To: linux-arm-kernel

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180307/a9bf98a7/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/5] pwm: sun4i: simplify controller mapping
  2018-03-07  2:07     ` Andre Przywara
@ 2018-03-07  7:44       ` Maxime Ripard
  -1 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:44 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-pwm, devicetree, linux-sunxi, Chen-Yu Tsai, Thierry Reding,
	linux-arm-kernel


[-- 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/5] pwm: sun4i: simplify controller mapping
@ 2018-03-07  7:44       ` Maxime Ripard
  0 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:44 UTC (permalink / raw)
  To: linux-arm-kernel

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180307/f97cfeaf/attachment.sig>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
  2018-03-07  2:07     ` Andre Przywara
@ 2018-03-07  7:45       ` Maxime Ripard
  -1 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:45 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-pwm, devicetree, linux-sunxi, Chen-Yu Tsai, Thierry Reding,
	linux-arm-kernel


[-- 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
@ 2018-03-07  7:45       ` Maxime Ripard
  0 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-07  7:45 UTC (permalink / raw)
  To: linux-arm-kernel

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180307/eef51ad9/attachment.sig>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
  2018-03-07  2:07     ` Andre Przywara
@ 2018-03-08  2:08         ` Rob Herring
  -1 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2018-03-08  2:08 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Thierry Reding, Maxime Ripard, Chen-Yu Tsai,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland

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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
@ 2018-03-08  2:08         ` Rob Herring
  0 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2018-03-08  2:08 UTC (permalink / raw)
  To: linux-arm-kernel

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@arm.com>
> ---
>  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 at 1c20e00 {
> -- 
> 2.14.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
  2018-03-08  2:08         ` Rob Herring
@ 2018-03-08  9:09           ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-08  9:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Thierry Reding, Maxime Ripard, Chen-Yu Tsai,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland

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
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
@ 2018-03-08  9:09           ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-08  9:09 UTC (permalink / raw)
  To: linux-arm-kernel

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@arm.com>
>> ---
>>  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 at 1c20e00 {
>> -- 
>> 2.14.1
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
  2018-03-08  9:09           ` Andre Przywara
@ 2018-03-08 14:37               ` Rob Herring
  -1 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2018-03-08 14:37 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Thierry Reding, Maxime Ripard, Chen-Yu Tsai, Linux PWM List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-sunxi, devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
@ 2018-03-08 14:37               ` Rob Herring
  0 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2018-03-08 14:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara <andre.przywara@arm.com> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
  2018-03-08 14:37               ` Rob Herring
@ 2018-03-08 15:27                   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-08 15:27 UTC (permalink / raw)
  To: robh-DgEjT+Ai2ygdnm+yROfE0A
  Cc: Thierry Reding, Maxime Ripard, Chen-Yu Tsai, Linux PWM List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-sunxi, devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland

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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [linux-sunxi] Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings
@ 2018-03-08 15:27                   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-08 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 08/03/18 14:37, Rob Herring wrote:
> On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara <andre.przywara@arm.com> 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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
  2018-03-07  7:45       ` Maxime Ripard
@ 2018-03-13 14:05           ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-13 14:05 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Thierry Reding, Chen-Yu Tsai, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
@ 2018-03-13 14:05           ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2018-03-13 14:05 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
  2018-03-13 14:05           ` Andre Przywara
@ 2018-03-13 15:32             ` Maxime Ripard
  -1 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-13 15:32 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-pwm, devicetree, linux-sunxi, Chen-Yu Tsai, Thierry Reding,
	linux-arm-kernel


[-- 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support
@ 2018-03-13 15:32             ` Maxime Ripard
  0 siblings, 0 replies; 30+ messages in thread
From: Maxime Ripard @ 2018-03-13 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180313/deccedfc/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-03-13 15:32 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07  2:07 [PATCH 0/5] drivers: pwm: sun4i: Improve support for A64 and H6 SoCs Andre Przywara
2018-03-07  2:07 ` Andre Przywara
     [not found] ` <20180307020719.6675-1-andre.przywara-5wv7dgnIgG8@public.gmane.org>
2018-03-07  2:07   ` [PATCH 1/5] pwm: sun4i: drop unused .has_rdy member Andre Przywara
2018-03-07  2:07     ` Andre Przywara
2018-03-07  7:40     ` Maxime Ripard
2018-03-07  7:40       ` Maxime Ripard
2018-03-07  2:07   ` [PATCH 2/5] pwm: sun4i: simplify controller mapping Andre Przywara
2018-03-07  2:07     ` Andre Przywara
2018-03-07  7:44     ` Maxime Ripard
2018-03-07  7:44       ` Maxime Ripard
2018-03-07  2:07   ` [PATCH 3/5] pwm: sun4i: Introduce (optional) reset support Andre Przywara
2018-03-07  2:07     ` Andre Przywara
2018-03-07  7:45     ` Maxime Ripard
2018-03-07  7:45       ` Maxime Ripard
     [not found]       ` <20180307074516.dbak7ztkua4p7mr5-ZC1Zs529Oq4@public.gmane.org>
2018-03-13 14:05         ` Andre Przywara
2018-03-13 14:05           ` Andre Przywara
2018-03-13 15:32           ` Maxime Ripard
2018-03-13 15:32             ` Maxime Ripard
2018-03-07  2:07   ` [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings Andre Przywara
2018-03-07  2:07     ` Andre Przywara
     [not found]     ` <20180307020719.6675-5-andre.przywara-5wv7dgnIgG8@public.gmane.org>
2018-03-08  2:08       ` Rob Herring
2018-03-08  2:08         ` Rob Herring
2018-03-08  9:09         ` Andre Przywara
2018-03-08  9:09           ` Andre Przywara
     [not found]           ` <9431a141-8b9c-9743-0907-8f0720df34a2-5wv7dgnIgG8@public.gmane.org>
2018-03-08 14:37             ` Rob Herring
2018-03-08 14:37               ` Rob Herring
     [not found]               ` <CAL_Jsq+e8uRm0gi1dn3Ln-b37SC9WGw5aWPaBKoRA2vi-==j6A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-03-08 15:27                 ` Andre Przywara
2018-03-08 15:27                   ` [linux-sunxi] " Andre Przywara
2018-03-07  2:07   ` [PATCH 5/5] dts: sunxi: A64: Add PWM controllers Andre Przywara
2018-03-07  2:07     ` Andre Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.