| 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: <20170703203640.697b8286@bbrezillon>
Date: Mon, 3 Jul 2017 20:36:40 +0200
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
To: David Wu <david.wu@...k-chips.com>
Cc: thierry.reding@...il.com, heiko@...ech.de, robh+dt@...nel.org,
catalin.marinas@....com, briannorris@...omium.org,
dianders@...omium.org, mark.rutland@....com,
huangtao@...k-chips.com, linux-pwm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] pwm: rockchip: Move the configuration of polarity
from rockchip_pwm_set_enable() to rockchip_pwm_config()
On Thu, 29 Jun 2017 20:27:49 +0800
David Wu <david.wu@...k-chips.com> wrote:
> It is usually possible to configure the polarity, cycle and duty all at once,
> so that the polarity and cycle and duty should be binding together. Move it
> into rockchip_pwm_config(), as well as prepared for the next atomic update
> commit.
>
> As the rk2928-pwm is different from rk3288-pwm, the rk2928-pwm doesn't support
> the polarity, use the pwm_config_v1 and pwm_config_v2 to distinguish them.
>
> Signed-off-by: David Wu <david.wu@...k-chips.com>
> ---
> drivers/pwm/pwm-rockchip.c | 77 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 57 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index cd45f17..eb630ff 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -27,6 +27,7 @@
> #define PWM_DUTY_NEGATIVE (0 << 3)
> #define PWM_INACTIVE_NEGATIVE (0 << 4)
> #define PWM_INACTIVE_POSITIVE (1 << 4)
> +#define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
> #define PWM_OUTPUT_LEFT (0 << 5)
> #define PWM_LP_DISABLE (0 << 8)
>
> @@ -52,10 +53,12 @@ struct rockchip_pwm_data {
> const struct pwm_ops *ops;
>
> void (*set_enable)(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity);
> + struct pwm_device *pwm, bool enable);
> void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
> struct pwm_state *state);
> + void (*pwm_config)(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity);
> };
Hm, maybe it's time to drop these custom hooks and implement
pwm_apply_v1 and pwm_apply_v2 instead.
>
> static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
> @@ -64,8 +67,7 @@ static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
> }
>
> static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity)
> + struct pwm_device *pwm, bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
> @@ -95,19 +97,13 @@ static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
> }
>
> static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity)
> + struct pwm_device *pwm, bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
> PWM_CONTINUOUS;
> u32 val;
>
> - if (polarity == PWM_POLARITY_INVERSED)
> - enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
> - else
> - enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
> -
> val = readl_relaxed(pc->base + pc->data->regs.ctrl);
>
> if (enable)
> @@ -165,12 +161,42 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
> clk_disable(pc->pclk);
> }
>
> -static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> - int duty_ns, int period_ns)
> +static void rockchip_pwm_config_v1(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity)
> +{
> + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> + unsigned long period, duty;
> + u64 clk_rate, div;
> +
> + clk_rate = clk_get_rate(pc->clk);
> +
> + /*
> + * Since period and duty cycle registers have a width of 32
> + * bits, every possible input period can be obtained using the
> + * default prescaler value for all practical clock rate values.
> + */
> + div = clk_rate * period_ns;
> + period = DIV_ROUND_CLOSEST_ULL(div,
> + pc->data->prescaler * NSEC_PER_SEC);
> +
> + div = clk_rate * duty_ns;
> + duty = DIV_ROUND_CLOSEST_ULL(div, pc->data->prescaler * NSEC_PER_SEC);
> +
> + writel(period, pc->base + pc->data->regs.period);
> + writel(duty, pc->base + pc->data->regs.duty);
> +}
> +
> +static void rockchip_pwm_config_v2(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> unsigned long period, duty;
> u64 clk_rate, div;
> + u32 ctrl;
>
> clk_rate = clk_get_rate(pc->clk);
>
> @@ -188,12 +214,20 @@ static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> writel(period, pc->base + pc->data->regs.period);
> writel(duty, pc->base + pc->data->regs.duty);
> +
> + ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
> + ctrl &= ~PWM_POLARITY_MASK;
> + if (polarity == PWM_POLARITY_INVERSED)
> + ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
> + else
> + ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
> +
> + writel(ctrl, pc->base + pc->data->regs.ctrl);
> }
>
> static int rockchip_pwm_enable(struct pwm_chip *chip,
> struct pwm_device *pwm,
> - bool enable,
> - enum pwm_polarity polarity)
> + bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> int ret;
> @@ -204,7 +238,7 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
> return ret;
> }
>
> - pc->data->set_enable(chip, pwm, enable, polarity);
> + pc->data->set_enable(chip, pwm, enable);
>
> if (!enable)
> clk_disable(pc->clk);
> @@ -228,17 +262,17 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> return ret;
>
> if (state->polarity != curstate.polarity && enabled) {
> - ret = rockchip_pwm_enable(chip, pwm, false, state->polarity);
> + ret = rockchip_pwm_enable(chip, pwm, false);
> if (ret)
> goto out;
> enabled = false;
> }
>
> - rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
> + pc->data->pwm_config(chip, pwm, state->duty_cycle,
> + state->period, state->polarity);
>
> if (state->enabled != enabled) {
> - ret = rockchip_pwm_enable(chip, pwm, state->enabled,
> - state->polarity);
> + ret = rockchip_pwm_enable(chip, pwm, state->enabled);
> if (ret)
> goto out;
> }
> @@ -278,6 +312,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v1,
> .set_enable = rockchip_pwm_set_enable_v1,
> .get_state = rockchip_pwm_get_state_v1,
> + .pwm_config = rockchip_pwm_config_v1,
> };
>
> static const struct rockchip_pwm_data pwm_data_v2 = {
> @@ -292,6 +327,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v2,
> .set_enable = rockchip_pwm_set_enable_v2,
> .get_state = rockchip_pwm_get_state_v2,
> + .pwm_config = rockchip_pwm_config_v2,
> };
>
> static const struct rockchip_pwm_data pwm_data_vop = {
> @@ -306,6 +342,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v2,
> .set_enable = rockchip_pwm_set_enable_v2,
> .get_state = rockchip_pwm_get_state_v2,
> + .pwm_config = rockchip_pwm_config_v2,
> };
>
> static const struct of_device_id rockchip_pwm_dt_ids[] = {
Powered by blists - more mailing lists