linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Updates for the atmel PWM driver
@ 2019-08-24  0:10 Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation Uwe Kleine-König
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

Hello,

this is v2 of my series to update the atmel PWM driver. (Implicit) v1
was sent on Aug 15, starting with Message-Id:
20190815214133.11134-1-uwe@kleine-koenig.org.

I updated the patches from the feedback I got in v1, see the individual
patches for the details.

Best regards
Uwe

Uwe Kleine-König (6):
  pwm: atmel: Add a hint where to find hardware documentation
  pwm: atmel: use a constant for maximum prescale value
  pwm: atmel: replace loop in prescale calculation by ad-hoc calculation
  pwm: atmel: document known weaknesses of both hardware and software
  pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for
    readl
  pwm: atmel: implement .get_state()

 drivers/pwm/pwm-atmel.c | 86 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 73 insertions(+), 13 deletions(-)

-- 
2.20.1


_______________________________________________
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] 12+ messages in thread

* [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-26  8:19   ` Nicolas.Ferre
  2019-08-24  0:10 ` [PATCH v2 2/6] pwm: atmel: use a constant for maximum prescale value Uwe Kleine-König
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

Most Microchip (formerly Atmel) chips have publicly available manuals.
A comprehensive list is already contained in the documentation folder.
Reference this list in the header of the driver to allow reviewers to
find the relevant manuals.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Changes since (implicit) v1 sent with Message-Id:
20190815214133.11134-1-uwe@kleine-koenig.org:

 - Only reference Documentation/arm/microchip.rst instead of starting
   another list of links

 drivers/pwm/pwm-atmel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index e5e1eaf372fa..a61a30fa8b7e 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -4,6 +4,9 @@
  *
  * Copyright (C) 2013 Atmel Corporation
  *		 Bo Shen <voice.shen@atmel.com>
+ *
+ * Links to reference manuals for the supported PWM chips can be found in
+ * Documentation/arm/microchip.rst.
  */
 
 #include <linux/clk.h>
-- 
2.20.1


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

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

* [PATCH v2 2/6] pwm: atmel: use a constant for maximum prescale value
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 3/6] pwm: atmel: replace loop in prescale calculation by ad-hoc calculation Uwe Kleine-König
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

The maximal prescale value is 10 for all supported variants. So drop the
member in the variant description and introduce a global constant instead.

This reduces the size of the variant descriptions and the .apply() callback
can be compiled a bit more effectively.

Acked-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Changes since (implicit) v1, sent with Message-Id:
20190815214133.11134-2-uwe@kleine-koenig.org:

 - Added Claudiu's Ack

 drivers/pwm/pwm-atmel.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index a61a30fa8b7e..f497f84771f0 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -50,6 +50,8 @@
 #define PWMV2_CPRD		0x0C
 #define PWMV2_CPRDUPD		0x10
 
+#define PWM_MAX_PRES		10
+
 struct atmel_pwm_registers {
 	u8 period;
 	u8 period_upd;
@@ -59,7 +61,6 @@ struct atmel_pwm_registers {
 
 struct atmel_pwm_config {
 	u32 max_period;
-	u32 max_pres;
 };
 
 struct atmel_pwm_data {
@@ -126,7 +127,7 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
 	for (*pres = 0; cycles > atmel_pwm->data->cfg.max_period; cycles >>= 1)
 		(*pres)++;
 
-	if (*pres > atmel_pwm->data->cfg.max_pres) {
+	if (*pres > PWM_MAX_PRES) {
 		dev_err(chip->dev, "pres exceeds the maximum value\n");
 		return -EINVAL;
 	}
@@ -289,7 +290,6 @@ static const struct atmel_pwm_data atmel_sam9rl_pwm_data = {
 	.cfg = {
 		/* 16 bits to keep period and duty. */
 		.max_period	= 0xffff,
-		.max_pres	= 10,
 	},
 };
 
@@ -303,7 +303,6 @@ static const struct atmel_pwm_data atmel_sama5_pwm_data = {
 	.cfg = {
 		/* 16 bits to keep period and duty. */
 		.max_period	= 0xffff,
-		.max_pres	= 10,
 	},
 };
 
@@ -317,7 +316,6 @@ static const struct atmel_pwm_data mchp_sam9x60_pwm_data = {
 	.cfg = {
 		/* 32 bits to keep period and duty. */
 		.max_period	= 0xffffffff,
-		.max_pres	= 10,
 	},
 };
 
-- 
2.20.1


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

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

* [PATCH v2 3/6] pwm: atmel: replace loop in prescale calculation by ad-hoc calculation
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 2/6] pwm: atmel: use a constant for maximum prescale value Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 4/6] pwm: atmel: document known weaknesses of both hardware and software Uwe Kleine-König
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

The calculated values are the same with the modified algorithm. The only
difference is that the calculation is a bit more efficient.

Acked-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Changes since (implicit) v1 sent with Message-Id:
20190815214133.11134-3-uwe@kleine-koenig.org

 - Added Claudiu's Ack

 drivers/pwm/pwm-atmel.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index f497f84771f0..3786ab9db5cf 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -60,7 +60,7 @@ struct atmel_pwm_registers {
 };
 
 struct atmel_pwm_config {
-	u32 max_period;
+	u32 period_bits;
 };
 
 struct atmel_pwm_data {
@@ -119,17 +119,27 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
 {
 	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 	unsigned long long cycles = state->period;
+	int shift;
 
 	/* Calculate the period cycles and prescale value */
 	cycles *= clk_get_rate(atmel_pwm->clk);
 	do_div(cycles, NSEC_PER_SEC);
 
-	for (*pres = 0; cycles > atmel_pwm->data->cfg.max_period; cycles >>= 1)
-		(*pres)++;
+	/*
+	 * The register for the period length is cfg.period_bits bits wide.
+	 * So for each bit the number of clock cycles is wider divide the input
+	 * clock frequency by two using pres and shift cprd accordingly.
+	 */
+	shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
 
-	if (*pres > PWM_MAX_PRES) {
+	if (shift > PWM_MAX_PRES) {
 		dev_err(chip->dev, "pres exceeds the maximum value\n");
 		return -EINVAL;
+	} else if (shift > 0) {
+		*pres = shift;
+		cycles >>= *pres;
+	} else {
+		*pres = 0;
 	}
 
 	*cprd = cycles;
@@ -289,7 +299,7 @@ static const struct atmel_pwm_data atmel_sam9rl_pwm_data = {
 	},
 	.cfg = {
 		/* 16 bits to keep period and duty. */
-		.max_period	= 0xffff,
+		.period_bits	= 16,
 	},
 };
 
@@ -302,7 +312,7 @@ static const struct atmel_pwm_data atmel_sama5_pwm_data = {
 	},
 	.cfg = {
 		/* 16 bits to keep period and duty. */
-		.max_period	= 0xffff,
+		.period_bits	= 16,
 	},
 };
 
@@ -315,7 +325,7 @@ static const struct atmel_pwm_data mchp_sam9x60_pwm_data = {
 	},
 	.cfg = {
 		/* 32 bits to keep period and duty. */
-		.max_period	= 0xffffffff,
+		.period_bits	= 32,
 	},
 };
 
-- 
2.20.1


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

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

* [PATCH v2 4/6] pwm: atmel: document known weaknesses of both hardware and software
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2019-08-24  0:10 ` [PATCH v2 3/6] pwm: atmel: replace loop in prescale calculation by ad-hoc calculation Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 5/6] pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for readl Uwe Kleine-König
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

This documents the my findings while reading through the driver and the
reference manual.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Changes since (implicit) v1 sent with Message-Id:
20190816093748.11769-1-uwe@kleine-koenig.org:

 - Add some prosa to commit log

 drivers/pwm/pwm-atmel.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 3786ab9db5cf..89f3a62f7541 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -7,6 +7,16 @@
  *
  * Links to reference manuals for the supported PWM chips can be found in
  * Documentation/arm/microchip.rst.
+ *
+ * Limitations:
+ * - Periods start with the inactive level.
+ * - Hardware has to be stopped in general to update settings.
+ *
+ * Software bugs/possible improvements:
+ * - When atmel_pwm_apply() is called with state->enabled=false a change in
+ *   state->polarity isn't honored.
+ * - Instead of sleeping to wait for a completed period, the interrupt
+ *   functionality could be used.
  */
 
 #include <linux/clk.h>
-- 
2.20.1


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

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

* [PATCH v2 5/6] pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for readl
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2019-08-24  0:10 ` [PATCH v2 4/6] pwm: atmel: document known weaknesses of both hardware and software Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-24  0:10 ` [PATCH v2 6/6] pwm: atmel: implement .get_state() Uwe Kleine-König
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

This makes it a bit easier when instrumenting register access to only have
to add code in one place.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
New in v2

 drivers/pwm/pwm-atmel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 89f3a62f7541..152c2b7dd6df 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -111,7 +111,7 @@ static inline u32 atmel_pwm_ch_readl(struct atmel_pwm_chip *chip,
 {
 	unsigned long base = PWM_CH_REG_OFFSET + ch * PWM_CH_REG_SIZE;
 
-	return readl_relaxed(chip->base + base + offset);
+	return atmel_pwm_readl(chip, base + offset);
 }
 
 static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip,
@@ -120,7 +120,7 @@ static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip,
 {
 	unsigned long base = PWM_CH_REG_OFFSET + ch * PWM_CH_REG_SIZE;
 
-	writel_relaxed(val, chip->base + base + offset);
+	atmel_pwm_writel(chip, base + offset, val);
 }
 
 static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
-- 
2.20.1


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

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

* [PATCH v2 6/6] pwm: atmel: implement .get_state()
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
                   ` (4 preceding siblings ...)
  2019-08-24  0:10 ` [PATCH v2 5/6] pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for readl Uwe Kleine-König
@ 2019-08-24  0:10 ` Uwe Kleine-König
  2019-08-28 10:26   ` Claudiu.Beznea
  2019-10-24  7:30 ` [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
  2020-01-08 13:00 ` Thierry Reding
  7 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-08-24  0:10 UTC (permalink / raw)
  To: Claudiu Beznea, Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, linux-arm-kernel

This function reads back the configured parameters from the hardware. As
.apply rounds down (mostly) I'm rounding up in .get_state() to achieve
that applying a state just read from hardware is a no-op.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
New in v2

 drivers/pwm/pwm-atmel.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 152c2b7dd6df..f788501ab6ca 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -295,8 +295,47 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
+static void atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
+	u32 sr, cmr;
+
+	sr = atmel_pwm_readl(atmel_pwm, PWM_SR);
+	cmr = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
+
+	if (sr & (1 << pwm->hwpwm)) {
+		unsigned long rate = clk_get_rate(atmel_pwm->clk);
+		u32 cdty, cprd, pres; 
+		u64 tmp;
+
+		pres = cmr & PWM_CMR_CPRE_MSK;
+
+		cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.period);
+		tmp = (u64)cprd * NSEC_PER_SEC;
+		tmp <<= pres;
+		state->period = DIV64_U64_ROUND_UP(tmp, rate);
+
+		cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.duty);
+		tmp = (u64)cdty * NSEC_PER_SEC;
+		tmp <<= pres;
+		state->duty_cycle = DIV64_U64_ROUND_UP(tmp, rate);
+
+		state->enabled = true;
+	} else {
+		state->enabled = false;
+	}
+
+	if (cmr & PWM_CMR_CPOL)
+		state->polarity = PWM_POLARITY_INVERSED;
+	else
+		state->polarity = PWM_POLARITY_NORMAL;
+
+}
+
 static const struct pwm_ops atmel_pwm_ops = {
 	.apply = atmel_pwm_apply,
+	.get_state = atmel_pwm_get_state,
 	.owner = THIS_MODULE,
 };
 
-- 
2.20.1


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

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

* Re: [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation
  2019-08-24  0:10 ` [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation Uwe Kleine-König
@ 2019-08-26  8:19   ` Nicolas.Ferre
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas.Ferre @ 2019-08-26  8:19 UTC (permalink / raw)
  To: uwe, Claudiu.Beznea, thierry.reding
  Cc: linux-pwm, alexandre.belloni, Ludovic.Desroches, linux-arm-kernel

On 24/08/2019 at 02:10, Uwe Kleine-König wrote:
> External E-Mail
> 
> 
> Most Microchip (formerly Atmel) chips have publicly available manuals.
> A comprehensive list is already contained in the documentation folder.
> Reference this list in the header of the driver to allow reviewers to
> find the relevant manuals.
> 
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Thanks Uwe!

Best regards,
   Nicolas

> ---
> Changes since (implicit) v1 sent with Message-Id:
> 20190815214133.11134-1-uwe@kleine-koenig.org:
> 
>   - Only reference Documentation/arm/microchip.rst instead of starting
>     another list of links
> 
>   drivers/pwm/pwm-atmel.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index e5e1eaf372fa..a61a30fa8b7e 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -4,6 +4,9 @@
>    *
>    * Copyright (C) 2013 Atmel Corporation
>    *		 Bo Shen <voice.shen@atmel.com>
> + *
> + * Links to reference manuals for the supported PWM chips can be found in
> + * Documentation/arm/microchip.rst.
>    */
>   
>   #include <linux/clk.h>
> 


-- 
Nicolas Ferre
_______________________________________________
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] 12+ messages in thread

* Re: [PATCH v2 6/6] pwm: atmel: implement .get_state()
  2019-08-24  0:10 ` [PATCH v2 6/6] pwm: atmel: implement .get_state() Uwe Kleine-König
@ 2019-08-28 10:26   ` Claudiu.Beznea
  2019-09-02 20:06     ` Uwe Kleine-König
  0 siblings, 1 reply; 12+ messages in thread
From: Claudiu.Beznea @ 2019-08-28 10:26 UTC (permalink / raw)
  To: uwe, thierry.reding
  Cc: linux-pwm, alexandre.belloni, Ludovic.Desroches, linux-arm-kernel



On 24.08.2019 03:10, Uwe Kleine-König wrote:
> External E-Mail
> 
> 
> This function reads back the configured parameters from the hardware. As
> .apply rounds down (mostly) I'm rounding up in .get_state() to achieve
> that applying a state just read from hardware is a no-op.

Since this read is only at probing, at least for the moment, and, as far as
I remember, the idea w/ .get_state was to reflect in Linux the states of
PWMs that were setup before Linux takes control (e.g. PWMs setup in
bootloaders) I think it would no problem if it would be no-ops in this
scenario. In case of run-time state retrieval, pwm_get_state() should be
enough. If one would get the state previously saved w/ this .get_state API
he/she would change it, then it would apply the changes to the hardware. No
changes of PWM state would be anyway skipped from the beginning, in
pwm_apply_state() by this code:

        if (state->period == pwm->state.period &&
            state->duty_cycle == pwm->state.duty_cycle &&
	    state->polarity == pwm->state.polarity &&
            state->enabled == pwm->state.enabled)
		return 0;

But maybe I'm missing something.

> 
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
> New in v2
> 
>  drivers/pwm/pwm-atmel.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 152c2b7dd6df..f788501ab6ca 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -295,8 +295,47 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	return 0;
>  }
>  
> +static void atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> +	u32 sr, cmr;
> +
> +	sr = atmel_pwm_readl(atmel_pwm, PWM_SR);
> +	cmr = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> +
> +	if (sr & (1 << pwm->hwpwm)) {
> +		unsigned long rate = clk_get_rate(atmel_pwm->clk);
> +		u32 cdty, cprd, pres; 

There is a whitespace at the end of this line.

> +		u64 tmp;
> +
> +		pres = cmr & PWM_CMR_CPRE_MSK;
> +
> +		cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.period);

If this is possible, please try to keep it at 80 chars per line. In my
opinion this still looks good:
		cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
					  atmel_pwm->data->regs.period);

> +		tmp = (u64)cprd * NSEC_PER_SEC;
> +		tmp <<= pres;
> +		state->period = DIV64_U64_ROUND_UP(tmp, rate);
> +
> +		cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.duty);

Ditto.

> +		tmp = (u64)cdty * NSEC_PER_SEC;
> +		tmp <<= pres;
> +		state->duty_cycle = DIV64_U64_ROUND_UP(tmp, rate);
> +
> +		state->enabled = true;
> +	} else {
> +		state->enabled = false;
> +	}
> +
> +	if (cmr & PWM_CMR_CPOL)
> +		state->polarity = PWM_POLARITY_INVERSED;
> +	else
> +		state->polarity = PWM_POLARITY_NORMAL;
> +
> +}
> +
>  static const struct pwm_ops atmel_pwm_ops = {
>  	.apply = atmel_pwm_apply,
> +	.get_state = atmel_pwm_get_state,
>  	.owner = THIS_MODULE,
>  };

Other than the minor things above,
Acked-by: Claudiu Beznea <claudiu.beznea@microchip.com>

>  
> 
_______________________________________________
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] 12+ messages in thread

* Re: [PATCH v2 6/6] pwm: atmel: implement .get_state()
  2019-08-28 10:26   ` Claudiu.Beznea
@ 2019-09-02 20:06     ` Uwe Kleine-König
  0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-09-02 20:06 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: linux-pwm, alexandre.belloni, Ludovic.Desroches, thierry.reding,
	linux-arm-kernel

Hello Claudiu,

On Wed, Aug 28, 2019 at 10:26:18AM +0000, Claudiu.Beznea@microchip.com wrote:
> On 24.08.2019 03:10, Uwe Kleine-König wrote:
> > External E-Mail
> > This function reads back the configured parameters from the hardware. As
> > .apply rounds down (mostly) I'm rounding up in .get_state() to achieve
> > that applying a state just read from hardware is a no-op.
> 
> Since this read is only at probing, at least for the moment, and, as far as

Yes, up to now .get_state() is only called at probing time. There is a
patch series (by me) on the list that changes that. (Though I'm not
entirely sure this is a good idea. Will comment my doubts in that thread
later.)

> I remember, the idea w/ .get_state was to reflect in Linux the states of
> PWMs that were setup before Linux takes control (e.g. PWMs setup in
> bootloaders) I think it would no problem if it would be no-ops in this
> scenario.

IMHO it should be a no-op.

> In case of run-time state retrieval, pwm_get_state() should be
> enough. If one would get the state previously saved w/ this .get_state API
> he/she would change it, then it would apply the changes to the hardware. No
> changes of PWM state would be anyway skipped from the beginning, in
> pwm_apply_state() by this code:
> 
>         if (state->period == pwm->state.period &&
>             state->duty_cycle == pwm->state.duty_cycle &&
> 	    state->polarity == pwm->state.polarity &&
>             state->enabled == pwm->state.enabled)
> 		return 0;
> 
> But maybe I'm missing something.

There is a problem I want to solve generally, not only for the atmel driver.

For example I consider it "expected" that

	s1 = pwm_get_state(pwm)
	pwm_apply_state(pwm, s2)
	pwm_apply_state(pwm, s1)

ends in the same configuration as it started. For that it is necessary
(even for the atmel driver with the guard you pointed out above) to
round up in .get_state if .apply rounds down.

Best regards
Uwe

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

_______________________________________________
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] 12+ messages in thread

* Re: [PATCH v2 0/6] Updates for the atmel PWM driver
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
                   ` (5 preceding siblings ...)
  2019-08-24  0:10 ` [PATCH v2 6/6] pwm: atmel: implement .get_state() Uwe Kleine-König
@ 2019-10-24  7:30 ` Uwe Kleine-König
  2020-01-08 13:00 ` Thierry Reding
  7 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-10-24  7:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, Claudiu Beznea,
	linux-arm-kernel

Hello Thierry,

On Sat, Aug 24, 2019 at 02:10:35AM +0200, Uwe Kleine-König wrote:
> this is v2 of my series to update the atmel PWM driver. (Implicit) v1
> was sent on Aug 15, starting with Message-Id:
> 20190815214133.11134-1-uwe@kleine-koenig.org.
> 
> I updated the patches from the feedback I got in v1, see the individual
> patches for the details.

This patch series is marked "Superseeded" in patchwork[1] but AFAIC this
is the most recent version of this series.

Can you please reconsider applying this series?

Best regards
Uwe

[1] https://patchwork.ozlabs.org/project/linux-pwm/list/?series=127096&state=*

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

_______________________________________________
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] 12+ messages in thread

* Re: [PATCH v2 0/6] Updates for the atmel PWM driver
  2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
                   ` (6 preceding siblings ...)
  2019-10-24  7:30 ` [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
@ 2020-01-08 13:00 ` Thierry Reding
  7 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2020-01-08 13:00 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-pwm, Alexandre Belloni, Ludovic Desroches, Claudiu Beznea,
	linux-arm-kernel


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

On Sat, Aug 24, 2019 at 02:10:35AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> this is v2 of my series to update the atmel PWM driver. (Implicit) v1
> was sent on Aug 15, starting with Message-Id:
> 20190815214133.11134-1-uwe@kleine-koenig.org.
> 
> I updated the patches from the feedback I got in v1, see the individual
> patches for the details.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (6):
>   pwm: atmel: Add a hint where to find hardware documentation
>   pwm: atmel: use a constant for maximum prescale value
>   pwm: atmel: replace loop in prescale calculation by ad-hoc calculation
>   pwm: atmel: document known weaknesses of both hardware and software
>   pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for
>     readl
>   pwm: atmel: implement .get_state()
> 
>  drivers/pwm/pwm-atmel.c | 86 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 73 insertions(+), 13 deletions(-)

There were two patches in this that were not reviewed or acked, but they
seem trivial enough, so I've just applied the whole series.

Thanks,
Thierry

[-- 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] 12+ messages in thread

end of thread, other threads:[~2020-01-08 13:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-24  0:10 [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
2019-08-24  0:10 ` [PATCH v2 1/6] pwm: atmel: Add a hint where to find hardware documentation Uwe Kleine-König
2019-08-26  8:19   ` Nicolas.Ferre
2019-08-24  0:10 ` [PATCH v2 2/6] pwm: atmel: use a constant for maximum prescale value Uwe Kleine-König
2019-08-24  0:10 ` [PATCH v2 3/6] pwm: atmel: replace loop in prescale calculation by ad-hoc calculation Uwe Kleine-König
2019-08-24  0:10 ` [PATCH v2 4/6] pwm: atmel: document known weaknesses of both hardware and software Uwe Kleine-König
2019-08-24  0:10 ` [PATCH v2 5/6] pwm: atmel: use atmel_pwm_writel in atmel_pwm_ch_writel; ditto for readl Uwe Kleine-König
2019-08-24  0:10 ` [PATCH v2 6/6] pwm: atmel: implement .get_state() Uwe Kleine-König
2019-08-28 10:26   ` Claudiu.Beznea
2019-09-02 20:06     ` Uwe Kleine-König
2019-10-24  7:30 ` [PATCH v2 0/6] Updates for the atmel PWM driver Uwe Kleine-König
2020-01-08 13:00 ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).