linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] pwm: add pwm AO on meson gx
@ 2017-06-08 12:24 Jerome Brunet
  2017-06-08 12:24 ` [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms Jerome Brunet
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Jerome Brunet @ 2017-06-08 12:24 UTC (permalink / raw)
  To: Thierry Reding, Kevin Hilman, Neil Armstrong
  Cc: Jerome Brunet, Carlo Caione, linux-pwm, devicetree,
	linux-amlogic, linux-kernel

The pwms found in the AO domain of the meson gx family have different
clock sources than the ones found in the EE domain. The AO pwms only
have 2 connected clock input which are clk81 and the crytal.

This patchset adds the necessary compatible and the clock data for it.

Changes since v1: [0]
* Correct clock source order for gxbb pwm ao. Documenation shows xtal as
  source #1 while it is actually source #0
* Add patch 3 to fix pwm calculation. Issue while testing pwm with clk81
  as clock source.

[0]:  https://lkml.kernel.org/r/20170607133013.4766-1-jbrunet@baylibre.com

Jerome Brunet (3):
  dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
  pwm: meson: add compatible for the gxbb ao pwms
  pwm: meson: improve pwm calculation precision.

 .../devicetree/bindings/pwm/pwm-meson.txt          |  4 +-
 drivers/pwm/pwm-meson.c                            | 47 +++++++++++++++++-----
 2 files changed, 41 insertions(+), 10 deletions(-)

-- 
2.9.4

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

* [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
  2017-06-08 12:24 [PATCH v2 0/3] pwm: add pwm AO on meson gx Jerome Brunet
@ 2017-06-08 12:24 ` Jerome Brunet
  2017-06-13 22:18   ` Rob Herring
  2017-06-08 12:24 ` [PATCH v2 2/3] pwm: meson: add compatible for the " Jerome Brunet
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Jerome Brunet @ 2017-06-08 12:24 UTC (permalink / raw)
  To: Thierry Reding, Kevin Hilman, Neil Armstrong
  Cc: Jerome Brunet, Carlo Caione, linux-pwm, devicetree,
	linux-amlogic, linux-kernel

Add compatible string to properly handle the PWMs found in the AO domain
of the gxbb (and gxl) family

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Documentation/devicetree/bindings/pwm/pwm-meson.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-meson.txt b/Documentation/devicetree/bindings/pwm/pwm-meson.txt
index 5376a4468cb6..5b07bebbf6f7 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-meson.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-meson.txt
@@ -2,7 +2,9 @@ Amlogic Meson PWM Controller
 ============================
 
 Required properties:
-- compatible: Shall contain "amlogic,meson8b-pwm" or "amlogic,meson-gxbb-pwm".
+- compatible: Shall contain "amlogic,meson8b-pwm"
+                         or "amlogic,meson-gxbb-pwm"
+                         or "amlogic,meson-gxbb-ao-pwm"
 - #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
   the cells format.
 
-- 
2.9.4

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

* [PATCH v2 2/3] pwm: meson: add compatible for the gxbb ao pwms
  2017-06-08 12:24 [PATCH v2 0/3] pwm: add pwm AO on meson gx Jerome Brunet
  2017-06-08 12:24 ` [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms Jerome Brunet
@ 2017-06-08 12:24 ` Jerome Brunet
  2017-06-14 17:38   ` Kevin Hilman
  2017-06-08 12:24 ` [PATCH v2 3/3] pwm: meson: improve pwm calculation precision Jerome Brunet
  2017-07-06  6:59 ` [PATCH v2 0/3] pwm: add pwm AO on meson gx Thierry Reding
  3 siblings, 1 reply; 14+ messages in thread
From: Jerome Brunet @ 2017-06-08 12:24 UTC (permalink / raw)
  To: Thierry Reding, Kevin Hilman, Neil Armstrong
  Cc: Jerome Brunet, Carlo Caione, linux-pwm, linux-amlogic, linux-kernel

On the gxbb (and gxl) family, the PWMs of the AO domain require a
specific compatible because the possible input clocks are different
from the EE PWMs input clocks.

Since the number of possible input clocks is also different, the
'num_parents' field is added to all the meson pwm data.

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/pwm/pwm-meson.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 045ef9fa6fe3..b911a944744a 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -103,6 +103,7 @@ struct meson_pwm_channel {
 
 struct meson_pwm_data {
 	const char * const *parent_names;
+	int num_parents;
 };
 
 struct meson_pwm {
@@ -381,6 +382,7 @@ static const char * const pwm_meson8b_parent_names[] = {
 
 static const struct meson_pwm_data pwm_meson8b_data = {
 	.parent_names = pwm_meson8b_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_meson8b_parent_names),
 };
 
 static const char * const pwm_gxbb_parent_names[] = {
@@ -389,11 +391,35 @@ static const char * const pwm_gxbb_parent_names[] = {
 
 static const struct meson_pwm_data pwm_gxbb_data = {
 	.parent_names = pwm_gxbb_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_gxbb_parent_names),
+};
+
+/*
+ * Only the 2 first inputs of the GXBB AO PWMs are valid
+ * The last 2 are grounded
+ */
+static const char * const pwm_gxbb_ao_parent_names[] = {
+	"xtal", "clk81"
+};
+
+static const struct meson_pwm_data pwm_gxbb_ao_data = {
+	.parent_names = pwm_gxbb_ao_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_gxbb_ao_parent_names),
 };
 
 static const struct of_device_id meson_pwm_matches[] = {
-	{ .compatible = "amlogic,meson8b-pwm", .data = &pwm_meson8b_data },
-	{ .compatible = "amlogic,meson-gxbb-pwm", .data = &pwm_gxbb_data },
+	{
+		.compatible = "amlogic,meson8b-pwm",
+		.data = &pwm_meson8b_data
+	},
+	{
+		.compatible = "amlogic,meson-gxbb-pwm",
+		.data = &pwm_gxbb_data
+	},
+	{
+		.compatible = "amlogic,meson-gxbb-ao-pwm",
+		.data = &pwm_gxbb_ao_data
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, meson_pwm_matches);
@@ -417,7 +443,7 @@ static int meson_pwm_init_channels(struct meson_pwm *meson,
 		init.ops = &clk_mux_ops;
 		init.flags = CLK_IS_BASIC;
 		init.parent_names = meson->data->parent_names;
-		init.num_parents = 1 << MISC_CLK_SEL_WIDTH;
+		init.num_parents = meson->data->num_parents;
 
 		channel->mux.reg = meson->base + REG_MISC_AB;
 		channel->mux.shift = mux_reg_shifts[i];
-- 
2.9.4

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

* [PATCH v2 3/3] pwm: meson: improve pwm calculation precision.
  2017-06-08 12:24 [PATCH v2 0/3] pwm: add pwm AO on meson gx Jerome Brunet
  2017-06-08 12:24 ` [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms Jerome Brunet
  2017-06-08 12:24 ` [PATCH v2 2/3] pwm: meson: add compatible for the " Jerome Brunet
@ 2017-06-08 12:24 ` Jerome Brunet
  2017-06-08 12:35   ` Neil Armstrong
  2017-07-06 21:21   ` Thierry Reding
  2017-07-06  6:59 ` [PATCH v2 0/3] pwm: add pwm AO on meson gx Thierry Reding
  3 siblings, 2 replies; 14+ messages in thread
From: Jerome Brunet @ 2017-06-08 12:24 UTC (permalink / raw)
  To: Thierry Reding, Kevin Hilman, Neil Armstrong
  Cc: Jerome Brunet, Carlo Caione, linux-pwm, linux-amlogic, linux-kernel

When using input clocks with high rates, such as clk81 (166MHz), the
fin_ns = NSEC_PER_SEC / fin_freq can introduce a significant error.

Ex: fin_freq = 166666667, NSEC_PER_SEC = 1000000000
    fin_ns = 5,9999999

which is, of course, rounded down to 5. This introduce an error of ~20%
on the period requested from the pwm.

This patch use ps instead of ns (and 64bits integer) to perform the
calculation. This should give a good enough precision.

Fixes: 211ed630753d ("pwm: Add support for Meson PWM Controller")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/pwm/pwm-meson.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index b911a944744a..4cdc66f7f718 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -163,7 +163,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
 			  unsigned int duty, unsigned int period)
 {
 	unsigned int pre_div, cnt, duty_cnt;
-	unsigned long fin_freq = -1, fin_ns;
+	unsigned long fin_freq = -1;
+	u64 fin_ps;
 
 	if (~(meson->inverter_mask >> id) & 0x1)
 		duty = period - duty;
@@ -179,13 +180,14 @@ static int meson_pwm_calc(struct meson_pwm *meson,
 	}
 
 	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
-	fin_ns = NSEC_PER_SEC / fin_freq;
+	fin_ps = ((u64)NSEC_PER_SEC * 1000) / fin_freq;
 
 	/* Calc pre_div with the period */
 	for (pre_div = 0; pre_div < MISC_CLK_DIV_MASK; pre_div++) {
-		cnt = DIV_ROUND_CLOSEST(period, fin_ns * (pre_div + 1));
-		dev_dbg(meson->chip.dev, "fin_ns=%lu pre_div=%u cnt=%u\n",
-			fin_ns, pre_div, cnt);
+		cnt = DIV_ROUND_CLOSEST_ULL((u64)period * 1000,
+					    fin_ps * (pre_div + 1));
+		dev_dbg(meson->chip.dev, "fin_ps=%llu pre_div=%u cnt=%u\n",
+			fin_ps, pre_div, cnt);
 		if (cnt <= 0xffff)
 			break;
 	}
@@ -208,7 +210,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
 		channel->lo = cnt;
 	} else {
 		/* Then check is we can have the duty with the same pre_div */
-		duty_cnt = DIV_ROUND_CLOSEST(duty, fin_ns * (pre_div + 1));
+		duty_cnt = DIV_ROUND_CLOSEST_ULL((u64)duty * 1000,
+						 fin_ps * (pre_div + 1));
 		if (duty_cnt > 0xffff) {
 			dev_err(meson->chip.dev, "unable to get duty cycle\n");
 			return -EINVAL;
-- 
2.9.4

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

* Re: [PATCH v2 3/3] pwm: meson: improve pwm calculation precision.
  2017-06-08 12:24 ` [PATCH v2 3/3] pwm: meson: improve pwm calculation precision Jerome Brunet
@ 2017-06-08 12:35   ` Neil Armstrong
  2017-07-06 21:21   ` Thierry Reding
  1 sibling, 0 replies; 14+ messages in thread
From: Neil Armstrong @ 2017-06-08 12:35 UTC (permalink / raw)
  To: Jerome Brunet, Thierry Reding, Kevin Hilman
  Cc: Carlo Caione, linux-pwm, linux-amlogic, linux-kernel

On 06/08/2017 02:24 PM, Jerome Brunet wrote:
> When using input clocks with high rates, such as clk81 (166MHz), the
> fin_ns = NSEC_PER_SEC / fin_freq can introduce a significant error.
> 
> Ex: fin_freq = 166666667, NSEC_PER_SEC = 1000000000
>     fin_ns = 5,9999999
> 
> which is, of course, rounded down to 5. This introduce an error of ~20%
> on the period requested from the pwm.
> 
> This patch use ps instead of ns (and 64bits integer) to perform the
> calculation. This should give a good enough precision.
> 
> Fixes: 211ed630753d ("pwm: Add support for Meson PWM Controller")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/pwm/pwm-meson.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index b911a944744a..4cdc66f7f718 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -163,7 +163,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
>  			  unsigned int duty, unsigned int period)
>  {
>  	unsigned int pre_div, cnt, duty_cnt;
> -	unsigned long fin_freq = -1, fin_ns;
> +	unsigned long fin_freq = -1;
> +	u64 fin_ps;
>  
>  	if (~(meson->inverter_mask >> id) & 0x1)
>  		duty = period - duty;
> @@ -179,13 +180,14 @@ static int meson_pwm_calc(struct meson_pwm *meson,
>  	}
>  
>  	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
> -	fin_ns = NSEC_PER_SEC / fin_freq;
> +	fin_ps = ((u64)NSEC_PER_SEC * 1000) / fin_freq;
>  
>  	/* Calc pre_div with the period */
>  	for (pre_div = 0; pre_div < MISC_CLK_DIV_MASK; pre_div++) {
> -		cnt = DIV_ROUND_CLOSEST(period, fin_ns * (pre_div + 1));
> -		dev_dbg(meson->chip.dev, "fin_ns=%lu pre_div=%u cnt=%u\n",
> -			fin_ns, pre_div, cnt);
> +		cnt = DIV_ROUND_CLOSEST_ULL((u64)period * 1000,
> +					    fin_ps * (pre_div + 1));
> +		dev_dbg(meson->chip.dev, "fin_ps=%llu pre_div=%u cnt=%u\n",
> +			fin_ps, pre_div, cnt);
>  		if (cnt <= 0xffff)
>  			break;
>  	}
> @@ -208,7 +210,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
>  		channel->lo = cnt;
>  	} else {
>  		/* Then check is we can have the duty with the same pre_div */
> -		duty_cnt = DIV_ROUND_CLOSEST(duty, fin_ns * (pre_div + 1));
> +		duty_cnt = DIV_ROUND_CLOSEST_ULL((u64)duty * 1000,
> +						 fin_ps * (pre_div + 1));
>  		if (duty_cnt > 0xffff) {
>  			dev_err(meson->chip.dev, "unable to get duty cycle\n");
>  			return -EINVAL;
> 

Great, I missed this !!

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

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

* Re: [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
  2017-06-08 12:24 ` [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms Jerome Brunet
@ 2017-06-13 22:18   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-06-13 22:18 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Thierry Reding, Kevin Hilman, Neil Armstrong, Carlo Caione,
	linux-pwm, devicetree, linux-amlogic, linux-kernel

On Thu, Jun 08, 2017 at 02:24:14PM +0200, Jerome Brunet wrote:
> Add compatible string to properly handle the PWMs found in the AO domain
> of the gxbb (and gxl) family
> 
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Documentation/devicetree/bindings/pwm/pwm-meson.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 2/3] pwm: meson: add compatible for the gxbb ao pwms
  2017-06-08 12:24 ` [PATCH v2 2/3] pwm: meson: add compatible for the " Jerome Brunet
@ 2017-06-14 17:38   ` Kevin Hilman
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2017-06-14 17:38 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Thierry Reding, Neil Armstrong, Carlo Caione, linux-pwm,
	linux-amlogic, linux-kernel

Jerome Brunet <jbrunet@baylibre.com> writes:

> On the gxbb (and gxl) family, the PWMs of the AO domain require a
> specific compatible because the possible input clocks are different
> from the EE PWMs input clocks.
>
> Since the number of possible input clocks is also different, the
> 'num_parents' field is added to all the meson pwm data.
>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

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

* Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
  2017-06-08 12:24 [PATCH v2 0/3] pwm: add pwm AO on meson gx Jerome Brunet
                   ` (2 preceding siblings ...)
  2017-06-08 12:24 ` [PATCH v2 3/3] pwm: meson: improve pwm calculation precision Jerome Brunet
@ 2017-07-06  6:59 ` Thierry Reding
  2017-07-06  9:02   ` Jerome Brunet
  3 siblings, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2017-07-06  6:59 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	devicetree, linux-amlogic, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

On Thu, Jun 08, 2017 at 02:24:13PM +0200, Jerome Brunet wrote:
> The pwms found in the AO domain of the meson gx family have different
> clock sources than the ones found in the EE domain. The AO pwms only
> have 2 connected clock input which are clk81 and the crytal.
> 
> This patchset adds the necessary compatible and the clock data for it.
> 
> Changes since v1: [0]
> * Correct clock source order for gxbb pwm ao. Documenation shows xtal as
>   source #1 while it is actually source #0
> * Add patch 3 to fix pwm calculation. Issue while testing pwm with clk81
>   as clock source.
> 
> [0]:  https://lkml.kernel.org/r/20170607133013.4766-1-jbrunet@baylibre.com
> 
> Jerome Brunet (3):
>   dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
>   pwm: meson: add compatible for the gxbb ao pwms
>   pwm: meson: improve pwm calculation precision.
> 
>  .../devicetree/bindings/pwm/pwm-meson.txt          |  4 +-
>  drivers/pwm/pwm-meson.c                            | 47 +++++++++++++++++-----
>  2 files changed, 41 insertions(+), 10 deletions(-)

For some reason the Author field keeps getting set to:

	jbrunet <jbrunet@baylibre.com>

It seems like the headers are correct, so maybe this is patchwork
messing things up?

Anyway, I've applied the series and manually fixed up the Author field.

Thanks,
Thierry

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

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

* Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
  2017-07-06  6:59 ` [PATCH v2 0/3] pwm: add pwm AO on meson gx Thierry Reding
@ 2017-07-06  9:02   ` Jerome Brunet
  2017-07-06  9:17     ` Thierry Reding
  0 siblings, 1 reply; 14+ messages in thread
From: Jerome Brunet @ 2017-07-06  9:02 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	devicetree, linux-amlogic, linux-kernel

On Thu, 2017-07-06 at 08:59 +0200, Thierry Reding wrote:
> On Thu, Jun 08, 2017 at 02:24:13PM +0200, Jerome Brunet wrote:
> > The pwms found in the AO domain of the meson gx family have different
> > clock sources than the ones found in the EE domain. The AO pwms only
> > have 2 connected clock input which are clk81 and the crytal.
> > 
> > This patchset adds the necessary compatible and the clock data for it.
> > 
> > Changes since v1: [0]
> > * Correct clock source order for gxbb pwm ao. Documenation shows xtal as
> >   source #1 while it is actually source #0
> > * Add patch 3 to fix pwm calculation. Issue while testing pwm with clk81
> >   as clock source.
> > 
> > [0]:  https://lkml.kernel.org/r/20170607133013.4766-1-jbrunet@baylibre.com
> > 
> > Jerome Brunet (3):
> >   dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
> >   pwm: meson: add compatible for the gxbb ao pwms
> >   pwm: meson: improve pwm calculation precision.
> > 
> >  .../devicetree/bindings/pwm/pwm-meson.txt          |  4 +-
> >  drivers/pwm/pwm-meson.c                            | 47 +++++++++++++++++
> > -----
> >  2 files changed, 41 insertions(+), 10 deletions(-)
> 
> For some reason the Author field keeps getting set to:
> 
> 	jbrunet <jbrunet@baylibre.com>
> 
> It seems like the headers are correct, so maybe this is patchwork
> messing things up?

And it strikes again ... :(
I already had the issue with https://patchwork.kernel.org a few month ago.
I have contacted the kernel.org helpdesk about it and here is the explanation:

"Patchwork treats users and patch submitters slightly differently, so once your
entry is created upon first patch submission, it sticks forever. I've adjusted
your entry manually per your request."

So my very first patch on the ML must have been malformed somehow.

> 
> Anyway, I've applied the series and manually fixed up the Author field.

Thanks a lot for this ! 
Any idea who I may contact to get this "manual adjustment" done on https://patch
work.ozlabs.org/ ?


> 
> Thanks,
> Thierry

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

* Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
  2017-07-06  9:02   ` Jerome Brunet
@ 2017-07-06  9:17     ` Thierry Reding
  2017-07-14  5:51       ` Jeremy Kerr
  0 siblings, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2017-07-06  9:17 UTC (permalink / raw)
  To: Jerome Brunet, Jeremy Kerr
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	devicetree, linux-amlogic, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

On Thu, Jul 06, 2017 at 11:02:20AM +0200, Jerome Brunet wrote:
> On Thu, 2017-07-06 at 08:59 +0200, Thierry Reding wrote:
> > On Thu, Jun 08, 2017 at 02:24:13PM +0200, Jerome Brunet wrote:
> > > The pwms found in the AO domain of the meson gx family have different
> > > clock sources than the ones found in the EE domain. The AO pwms only
> > > have 2 connected clock input which are clk81 and the crytal.
> > > 
> > > This patchset adds the necessary compatible and the clock data for it.
> > > 
> > > Changes since v1: [0]
> > > * Correct clock source order for gxbb pwm ao. Documenation shows xtal as
> > >   source #1 while it is actually source #0
> > > * Add patch 3 to fix pwm calculation. Issue while testing pwm with clk81
> > >   as clock source.
> > > 
> > > [0]:  https://lkml.kernel.org/r/20170607133013.4766-1-jbrunet@baylibre.com
> > > 
> > > Jerome Brunet (3):
> > >   dt-bindings: pwm: meson: add comptabible for gxbb ao pwms
> > >   pwm: meson: add compatible for the gxbb ao pwms
> > >   pwm: meson: improve pwm calculation precision.
> > > 
> > >  .../devicetree/bindings/pwm/pwm-meson.txt          |  4 +-
> > >  drivers/pwm/pwm-meson.c                            | 47 +++++++++++++++++
> > > -----
> > >  2 files changed, 41 insertions(+), 10 deletions(-)
> > 
> > For some reason the Author field keeps getting set to:
> > 
> > 	jbrunet <jbrunet@baylibre.com>
> > 
> > It seems like the headers are correct, so maybe this is patchwork
> > messing things up?
> 
> And it strikes again ... :(
> I already had the issue with https://patchwork.kernel.org a few month ago.
> I have contacted the kernel.org helpdesk about it and here is the explanation:
> 
> "Patchwork treats users and patch submitters slightly differently, so once your
> entry is created upon first patch submission, it sticks forever. I've adjusted
> your entry manually per your request."
> 
> So my very first patch on the ML must have been malformed somehow.
> 
> > 
> > Anyway, I've applied the series and manually fixed up the Author field.
> 
> Thanks a lot for this ! 
> Any idea who I may contact to get this "manual adjustment" done on https://patch
> work.ozlabs.org/ ?

I /think/ Jeremy Kerr (To'ed) would be a good person to contact about
this.

Jeremy, anything you can do about this?

Thanks,
Thierry

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

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

* Re: [PATCH v2 3/3] pwm: meson: improve pwm calculation precision.
  2017-06-08 12:24 ` [PATCH v2 3/3] pwm: meson: improve pwm calculation precision Jerome Brunet
  2017-06-08 12:35   ` Neil Armstrong
@ 2017-07-06 21:21   ` Thierry Reding
  2017-07-07  8:57     ` Jerome Brunet
  1 sibling, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2017-07-06 21:21 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	linux-amlogic, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]

On Thu, Jun 08, 2017 at 02:24:16PM +0200, Jerome Brunet wrote:
> When using input clocks with high rates, such as clk81 (166MHz), the
> fin_ns = NSEC_PER_SEC / fin_freq can introduce a significant error.
> 
> Ex: fin_freq = 166666667, NSEC_PER_SEC = 1000000000
>     fin_ns = 5,9999999
> 
> which is, of course, rounded down to 5. This introduce an error of ~20%
> on the period requested from the pwm.
> 
> This patch use ps instead of ns (and 64bits integer) to perform the
> calculation. This should give a good enough precision.
> 
> Fixes: 211ed630753d ("pwm: Add support for Meson PWM Controller")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/pwm/pwm-meson.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index b911a944744a..4cdc66f7f718 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -163,7 +163,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
>  			  unsigned int duty, unsigned int period)
>  {
>  	unsigned int pre_div, cnt, duty_cnt;
> -	unsigned long fin_freq = -1, fin_ns;
> +	unsigned long fin_freq = -1;
> +	u64 fin_ps;
>  
>  	if (~(meson->inverter_mask >> id) & 0x1)
>  		duty = period - duty;
> @@ -179,13 +180,14 @@ static int meson_pwm_calc(struct meson_pwm *meson,
>  	}
>  
>  	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
> -	fin_ns = NSEC_PER_SEC / fin_freq;
> +	fin_ps = ((u64)NSEC_PER_SEC * 1000) / fin_freq;

This failed to build, so I had to change this division to a do_div().

Thierry

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

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

* Re: [PATCH v2 3/3] pwm: meson: improve pwm calculation precision.
  2017-07-06 21:21   ` Thierry Reding
@ 2017-07-07  8:57     ` Jerome Brunet
  0 siblings, 0 replies; 14+ messages in thread
From: Jerome Brunet @ 2017-07-07  8:57 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	linux-amlogic, linux-kernel

On Thu, 2017-07-06 at 23:21 +0200, Thierry Reding wrote:
> On Thu, Jun 08, 2017 at 02:24:16PM +0200, Jerome Brunet wrote:
> > When using input clocks with high rates, such as clk81 (166MHz), the
> > fin_ns = NSEC_PER_SEC / fin_freq can introduce a significant error.
> > 
> > Ex: fin_freq = 166666667, NSEC_PER_SEC = 1000000000
> >     fin_ns = 5,9999999
> > 
> > which is, of course, rounded down to 5. This introduce an error of ~20%
> > on the period requested from the pwm.
> > 
> > This patch use ps instead of ns (and 64bits integer) to perform the
> > calculation. This should give a good enough precision.
> > 
> > Fixes: 211ed630753d ("pwm: Add support for Meson PWM Controller")
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  drivers/pwm/pwm-meson.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> > index b911a944744a..4cdc66f7f718 100644
> > --- a/drivers/pwm/pwm-meson.c
> > +++ b/drivers/pwm/pwm-meson.c
> > @@ -163,7 +163,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
> >  			  unsigned int duty, unsigned int period)
> >  {
> >  	unsigned int pre_div, cnt, duty_cnt;
> > -	unsigned long fin_freq = -1, fin_ns;
> > +	unsigned long fin_freq = -1;
> > +	u64 fin_ps;
> >  
> >  	if (~(meson->inverter_mask >> id) & 0x1)
> >  		duty = period - duty;
> > @@ -179,13 +180,14 @@ static int meson_pwm_calc(struct meson_pwm *meson,
> >  	}
> >  
> >  	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
> > -	fin_ns = NSEC_PER_SEC / fin_freq;
> > +	fin_ps = ((u64)NSEC_PER_SEC * 1000) / fin_freq;
> 
> This failed to build, so I had to change this division to a do_div().

Oh ! Indeed :(
I have mostly tested with arm64 which is fine.
I thought I had tested arm(32) build as well but I just noticed that MESON_PWM
option is disabled in multi_v7_defconfig. 

Thanks for catching and fixing this Thierry !


> 
> Thierry

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

* Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
  2017-07-06  9:17     ` Thierry Reding
@ 2017-07-14  5:51       ` Jeremy Kerr
  2017-07-18 14:52         ` Thierry Reding
  0 siblings, 1 reply; 14+ messages in thread
From: Jeremy Kerr @ 2017-07-14  5:51 UTC (permalink / raw)
  To: Thierry Reding, Jerome Brunet
  Cc: Kevin Hilman, Neil Armstrong, Carlo Caione, linux-pwm,
	devicetree, linux-amlogic, linux-kernel

Hi Thierry,

> I /think/ Jeremy Kerr (To'ed) would be a good person to contact about
> this.
> 
> Jeremy, anything you can do about this?

OK, all sorted. I've updated Jerome's entry in the database to suit.

Cheers,


Jeremy

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

* Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
  2017-07-14  5:51       ` Jeremy Kerr
@ 2017-07-18 14:52         ` Thierry Reding
  0 siblings, 0 replies; 14+ messages in thread
From: Thierry Reding @ 2017-07-18 14:52 UTC (permalink / raw)
  To: Jeremy Kerr
  Cc: Jerome Brunet, Kevin Hilman, Neil Armstrong, Carlo Caione,
	linux-pwm, devicetree, linux-amlogic, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Fri, Jul 14, 2017 at 01:51:12PM +0800, Jeremy Kerr wrote:
> Hi Thierry,
> 
> > I /think/ Jeremy Kerr (To'ed) would be a good person to contact about
> > this.
> > 
> > Jeremy, anything you can do about this?
> 
> OK, all sorted. I've updated Jerome's entry in the database to suit.

Headers do indeed look correct now.

Thanks a lot, Jeremy.

Thierry

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

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

end of thread, other threads:[~2017-07-18 14:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-08 12:24 [PATCH v2 0/3] pwm: add pwm AO on meson gx Jerome Brunet
2017-06-08 12:24 ` [PATCH v2 1/3] dt-bindings: pwm: meson: add comptabible for gxbb ao pwms Jerome Brunet
2017-06-13 22:18   ` Rob Herring
2017-06-08 12:24 ` [PATCH v2 2/3] pwm: meson: add compatible for the " Jerome Brunet
2017-06-14 17:38   ` Kevin Hilman
2017-06-08 12:24 ` [PATCH v2 3/3] pwm: meson: improve pwm calculation precision Jerome Brunet
2017-06-08 12:35   ` Neil Armstrong
2017-07-06 21:21   ` Thierry Reding
2017-07-07  8:57     ` Jerome Brunet
2017-07-06  6:59 ` [PATCH v2 0/3] pwm: add pwm AO on meson gx Thierry Reding
2017-07-06  9:02   ` Jerome Brunet
2017-07-06  9:17     ` Thierry Reding
2017-07-14  5:51       ` Jeremy Kerr
2017-07-18 14:52         ` 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).