On 17.01.2018 23:47, Ladislav Michl wrote: > Prescaler setting is currently not taken into account. > Fix that by introducing freq member variable and initialize > it at device probe time. This also avoids frequency > recomputing at each pwm configure time. > > Signed-off-by: Ladislav Michl <[email protected]> > --- > drivers/pwm/pwm-omap-dmtimer.c | 92 +++++++++++++++++++++++---------------- > 1 file changed, 53 insertions(+), 39 deletions(-) > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c > index cc485d9946f3..81c79e41a167 100644 > --- a/drivers/pwm/pwm-omap-dmtimer.c > +++ b/drivers/pwm/pwm-omap-dmtimer.c > @@ -40,6 +40,7 @@ struct pwm_omap_dmtimer_chip { > pwm_omap_dmtimer *dm_timer; > struct omap_dm_timer_ops *pdata; > struct platform_device *dm_timer_pdev; > + unsigned long freq; > }; > > static inline struct pwm_omap_dmtimer_chip * > @@ -48,9 +49,10 @@ to_pwm_omap_dmtimer_chip(struct pwm_chip *chip) > return container_of(chip, struct pwm_omap_dmtimer_chip, chip); > } > > -static u32 pwm_omap_dmtimer_get_clock_cycles(unsigned long clk_rate, int ns) > +static inline u32 > +pwm_omap_dmtimer_get_clock_cycles(struct pwm_omap_dmtimer_chip *omap, int ns) > { > - return DIV_ROUND_CLOSEST_ULL((u64)clk_rate * ns, NSEC_PER_SEC); > + return DIV_ROUND_CLOSEST_ULL((u64)omap->freq * ns, NSEC_PER_SEC); > } > > static void pwm_omap_dmtimer_start(struct pwm_omap_dmtimer_chip *omap) > @@ -99,8 +101,6 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, > struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); > u32 period_cycles, duty_cycles; > u32 load_value, match_value; > - struct clk *fclk; > - unsigned long clk_rate; > bool timer_active; > > dev_dbg(chip->dev, "requested duty cycle: %d ns, period: %d ns\n", > @@ -114,19 +114,6 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, > return 0; > } > > - fclk = omap->pdata->get_fclk(omap->dm_timer); > - if (!fclk) { > - dev_err(chip->dev, "invalid pmtimer fclk\n"); > - goto err_einval; > - } > - > - clk_rate = clk_get_rate(fclk); > - if (!clk_rate) { > - dev_err(chip->dev, "invalid pmtimer fclk rate\n"); > - goto err_einval; > - } > - > - dev_dbg(chip->dev, "clk rate: %luHz\n", clk_rate); > > /* > * Calculate the appropriate load and match values based on the > @@ -144,35 +131,35 @@ static int pwm_omap_dmtimer_config(struct pwm_chip > *chip, > * OMAP4430/60/70 TRM sections 22.2.4.10 and 22.2.4.11 > * AM335x Sitara TRM sections 20.1.3.5 and 20.1.3.6 > */ > - period_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, period_ns); > - duty_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, duty_ns); > + period_cycles = pwm_omap_dmtimer_get_clock_cycles(omap, period_ns); > + duty_cycles = pwm_omap_dmtimer_get_clock_cycles(omap, duty_ns); > > if (period_cycles < 2) { > dev_info(chip->dev, > "period %d ns too short for clock rate %lu Hz\n", > - period_ns, clk_rate); > + period_ns, omap->freq); > goto err_einval; > } > > if (duty_cycles < 1) { > dev_dbg(chip->dev, > "duty cycle %d ns is too short for clock rate %lu Hz\n", > - duty_ns, clk_rate); > + duty_ns, omap->freq); > dev_dbg(chip->dev, "using minimum of 1 clock cycle\n"); > duty_cycles = 1; > } else if (duty_cycles >= period_cycles) { > dev_dbg(chip->dev, > "duty cycle %d ns is too long for period %d ns at clock > rate %lu Hz\n", > - duty_ns, period_ns, clk_rate); > + duty_ns, period_ns, omap->freq); > dev_dbg(chip->dev, "using maximum of 1 clock cycle less than > period\n"); > duty_cycles = period_cycles - 1; > } > > dev_dbg(chip->dev, "effective duty cycle: %lld ns, period: %lld ns\n", > DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * duty_cycles, > - clk_rate), > + omap->freq), > DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * period_cycles, > - clk_rate)); > + omap->freq)); > > load_value = (DM_TIMER_MAX - period_cycles) + 1; > match_value = load_value + duty_cycles - 1; > @@ -248,8 +235,9 @@ static int pwm_omap_dmtimer_probe(struct platform_device > *pdev) > struct dmtimer_platform_data *timer_pdata; > struct omap_dm_timer_ops *pdata; > pwm_omap_dmtimer *dm_timer; > + struct clk *fclk; > u32 v; > - int status, ret; > + int ret; > > timer = of_parse_phandle(np, "ti,timers", 0); > if (!timer) > @@ -302,9 +290,8 @@ static int pwm_omap_dmtimer_probe(struct platform_device > *pdev) > > omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL); > if (!omap) { > - pdata->free(dm_timer); > ret = -ENOMEM; > - goto put; > + goto free; > } > > omap->pdata = pdata; > @@ -315,15 +302,42 @@ static int pwm_omap_dmtimer_probe(struct > platform_device *pdev) > * Ensure that the timer is stopped before we allow PWM core to call > * pwm_enable. > */ > - if (pm_runtime_active(&omap->dm_timer_pdev->dev)) > - omap->pdata->stop(omap->dm_timer); > - > - if (!of_property_read_u32(pdev->dev.of_node, "ti,prescaler", &v)) > - omap->pdata->set_prescaler(omap->dm_timer, v); > + if (pm_runtime_active(&timer_pdev->dev)) > + pdata->stop(dm_timer); > > /* setup dmtimer clock source */ > - if (!of_property_read_u32(pdev->dev.of_node, "ti,clock-source", &v)) > - omap->pdata->set_source(omap->dm_timer, v); > + if (!of_property_read_u32(pdev->dev.of_node, "ti,clock-source", &v)) { > + ret = pdata->set_source(dm_timer, v); > + if (ret) { > + dev_err(&pdev->dev, "invalid clock-source\n"); > + goto free; > + } > + } > + > + fclk = pdata->get_fclk(dm_timer); > + if (!fclk) { > + dev_err(&pdev->dev, "invalid fclk\n"); > + ret = -EINVAL; > + goto free; > + } > + > + omap->freq = clk_get_rate(fclk); > + if (!omap->freq) { > + dev_err(&pdev->dev, "invalid fclk rate\n"); > + ret = -EINVAL; > + goto free; > + } > + > + if (!of_property_read_u32(pdev->dev.of_node, "ti,prescaler", &v)) { > + ret = pdata->set_prescaler(dm_timer, v); > + if (ret) { > + dev_err(&pdev->dev, "invalid prescaler\n"); > + goto free; > + } > + omap->freq >>= v + 1; > + } > + > + dev_dbg(&pdev->dev, "clk rate: %luHz\n", omap->freq); > > omap->chip.dev = &pdev->dev; > omap->chip.ops = &pwm_omap_dmtimer_ops; > @@ -334,18 +348,18 @@ static int pwm_omap_dmtimer_probe(struct > platform_device *pdev) > > mutex_init(&omap->mutex); > > - status = pwmchip_add(&omap->chip); > - if (status < 0) { > + ret = pwmchip_add(&omap->chip); > + if (ret < 0) { > dev_err(&pdev->dev, "failed to register PWM\n"); > - omap->pdata->free(omap->dm_timer); > - ret = status; > - goto put; > + goto free; > } > > platform_set_drvdata(pdev, omap); >From documentation: "of_parse_phandle(): Returns the device_node pointer with >refcount incremented. Use of_node_put() on it when done." In case of success the of_node_put() should also be called as I see.
> > return 0; > > +free: > + pdata->free(dm_timer); > put: > of_node_put(timer); > >
