| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3e00e721-984c-e8db-49f7-046d7b335404@microchip.com>
Date: Mon, 22 Jan 2018 11:17:08 +0200
From: Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To: Ladislav Michl <ladis@...ux-mips.org>,
<linux-omap@...r.kernel.org>, <linux-pwm@...r.kernel.org>
CC: Keerthy <j-keerthy@...com>, <daniel.lezcano@...aro.org>,
<thierry.reding@...il.com>, <tony@...mide.com>,
<aaro.koskinen@....fi>, <narmstrong@...libre.com>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<sebastian.reichel@...labora.co.uk>, <robh+dt@...nel.org>,
<t-kristo@...com>, <grygorii.strashko@...com>
Subject: Re: [PATCH 2/3] pwm: pwm-omap-dmtimer: Fix frequency when using
prescaler
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 <ladis@...ux-mips.org>
> ---
> 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);
>
>
Powered by blists - more mailing lists