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/[email protected]
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
Add compatible string to properly handle the PWMs found in the AO domain
of the gxbb (and gxl) family
Acked-by: Neil Armstrong <[email protected]>
Signed-off-by: Jerome Brunet <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Jerome Brunet <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> Documentation/devicetree/bindings/pwm/pwm-meson.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Rob Herring <[email protected]>
Jerome Brunet <[email protected]> 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 <[email protected]>
> Signed-off-by: Jerome Brunet <[email protected]>
Reviewed-by: Kevin Hilman <[email protected]>
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/[email protected]
>
> 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 <[email protected]>
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
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/[email protected]
> >
> > 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 <[email protected]>
>
> 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
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/[email protected]
> > >
> > > 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 <[email protected]>
> >
> > 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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
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
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