| Message ID | 1416407589-8157-1-git-send-email-boris.brezillon@free-electrons.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Nov 19, 2014 at 03:33:09PM +0100, Boris Brezillon wrote: > at91sam9x5 has an errata forbidding the use of slow clk as a clk source and > sama5d3 SoCs has another errata forbidding the use of div1 prescaler. > > Take both of these erratas into account. > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > Hi Thierry, > > I've addressed the "erratas stored in of_device_id data" part, but still > haven't modified the compatible strings of the HLCDC subdevices. > > Please let me know if you really want to handle erratas through pwm > compatibles instead of parent device compatibles. > > Regards, > > Boris > > Changes since v1: > - use data field in of_device_id to attach erratas to an IP revision > > drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 45 insertions(+), 5 deletions(-) I applied this, but this was really much more difficult than I would've wanted. Since the MFD driver hasn't been merged into Linus' tree yet I wasn't able to actually build test this driver at all without manually pulling in the patches that add the MFD support. I went through this trouble this time because it's what we had agreed upon, but for the record, next time I'll request a stable branch that I can pull into the PWM tree to resolve this kind of dependency. Or patches will have to wait until the next merge window. I don't want to have to jump through hoops just to make sure the code in my tree actually compiles. Oh, and it's good that I do compile tests because... > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c [...] > +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = { > + .slow_clk_errata = true, > +}; > + > +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = { > + .div1_clk_errata = true, > +}; ... these actually should be static. Also I took the liberty of substituting "erratum" for "errata" and "errata" for "erratas". Thierry
Hi Thierry On Thu, 4 Dec 2014 14:12:31 +0100 Thierry Reding <thierry.reding@gmail.com> wrote: > On Wed, Nov 19, 2014 at 03:33:09PM +0100, Boris Brezillon wrote: > > at91sam9x5 has an errata forbidding the use of slow clk as a clk source and > > sama5d3 SoCs has another errata forbidding the use of div1 prescaler. > > > > Take both of these erratas into account. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > > --- > > Hi Thierry, > > > > I've addressed the "erratas stored in of_device_id data" part, but still > > haven't modified the compatible strings of the HLCDC subdevices. > > > > Please let me know if you really want to handle erratas through pwm > > compatibles instead of parent device compatibles. > > > > Regards, > > > > Boris > > > > Changes since v1: > > - use data field in of_device_id to attach erratas to an IP revision > > > > drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 45 insertions(+), 5 deletions(-) > > I applied this, but this was really much more difficult than I would've > wanted. Since the MFD driver hasn't been merged into Linus' tree yet I > wasn't able to actually build test this driver at all without manually > pulling in the patches that add the MFD support. I went through this > trouble this time because it's what we had agreed upon, but for the > record, next time I'll request a stable branch that I can pull into the > PWM tree to resolve this kind of dependency. Or patches will have to > wait until the next merge window. I don't want to have to jump through > hoops just to make sure the code in my tree actually compiles. Thanks for doing that. Just for my understanding, am I the one responsible for providing this stable branch, or were you expecting Lee to provide it ? > > Oh, and it's good that I do compile tests because... > > > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c > [...] > > +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = { > > + .slow_clk_errata = true, > > +}; > > + > > +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = { > > + .div1_clk_errata = true, > > +}; > > ... these actually should be static. > > Also I took the liberty of substituting "erratum" for "errata" and > "errata" for "erratas". Indeed, thanks for fixing those problems. Best Regards, Boris
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c index eaf8b12..e7c4400 100644 --- a/drivers/pwm/pwm-atmel-hlcdc.c +++ b/drivers/pwm/pwm-atmel-hlcdc.c @@ -32,10 +32,16 @@ #define ATMEL_HLCDC_PWMPS_MAX 0x6 #define ATMEL_HLCDC_PWMPS(x) ((x) & ATMEL_HLCDC_PWMPS_MASK) +struct atmel_hlcdc_pwm_erratas { + bool slow_clk_errata; + bool div1_clk_errata; +}; + struct atmel_hlcdc_pwm { struct pwm_chip chip; struct atmel_hlcdc *hlcdc; struct clk *cur_clk; + const struct atmel_hlcdc_pwm_erratas *erratas; }; static inline struct atmel_hlcdc_pwm *to_atmel_hlcdc_pwm(struct pwm_chip *chip) @@ -56,20 +62,29 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c, u32 pwmcfg; int pres; - clk_freq = clk_get_rate(new_clk); - clk_period_ns = (u64)NSEC_PER_SEC * 256; - do_div(clk_period_ns, clk_freq); + if (!chip->erratas || !chip->erratas->slow_clk_errata) { + clk_freq = clk_get_rate(new_clk); + clk_period_ns = (u64)NSEC_PER_SEC * 256; + do_div(clk_period_ns, clk_freq); + } - if (clk_period_ns > period_ns) { + /* Errata: cannot use slow clk on some IP revisions */ + if ((chip->erratas && chip->erratas->slow_clk_errata) || + clk_period_ns > period_ns) { new_clk = hlcdc->sys_clk; clk_freq = clk_get_rate(new_clk); clk_period_ns = (u64)NSEC_PER_SEC * 256; do_div(clk_period_ns, clk_freq); } - for (pres = 0; pres <= ATMEL_HLCDC_PWMPS_MAX; pres++) + for (pres = 0; pres <= ATMEL_HLCDC_PWMPS_MAX; pres++) { + /* Errata: cannot divide by 1 on some IP revisions */ + if (!pres && chip->erratas && chip->erratas->div1_clk_errata) + continue; + if ((clk_period_ns << pres) >= period_ns) break; + } if (pres > ATMEL_HLCDC_PWMPS_MAX) return -EINVAL; @@ -187,8 +202,29 @@ static const struct pwm_ops atmel_hlcdc_pwm_ops = { .owner = THIS_MODULE, }; +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = { + .slow_clk_errata = true, +}; + +const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = { + .div1_clk_errata = true, +}; + +static const struct of_device_id atmel_hlcdc_dt_ids[] = { + { + .compatible = "atmel,at91sam9x5-hlcdc", + .data = &atmel_hlcdc_pwm_at91sam9x5_erratas, + }, + { + .compatible = "atmel,sama5d3-hlcdc", + .data = &atmel_hlcdc_pwm_sama5d3_erratas, + }, + { /* sentinel */ }, +}; + static int atmel_hlcdc_pwm_probe(struct platform_device *pdev) { + const struct of_device_id *match; struct device *dev = &pdev->dev; struct atmel_hlcdc_pwm *chip; struct atmel_hlcdc *hlcdc; @@ -204,6 +240,10 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev) if (ret) return ret; + match = of_match_node(atmel_hlcdc_dt_ids, dev->parent->of_node); + if (match) + chip->erratas = match->data; + chip->hlcdc = hlcdc; chip->chip.ops = &atmel_hlcdc_pwm_ops; chip->chip.dev = dev;
at91sam9x5 has an errata forbidding the use of slow clk as a clk source and sama5d3 SoCs has another errata forbidding the use of div1 prescaler. Take both of these erratas into account. Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- Hi Thierry, I've addressed the "erratas stored in of_device_id data" part, but still haven't modified the compatible strings of the HLCDC subdevices. Please let me know if you really want to handle erratas through pwm compatibles instead of parent device compatibles. Regards, Boris Changes since v1: - use data field in of_device_id to attach erratas to an IP revision drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-)