| 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
| ||
|
Date: Mon, 27 Mar 2017 14:57:05 +0200
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
To: Claudiu Beznea <claudiu.beznea@...rochip.com>
Cc: <thierry.reding@...il.com>, <robh+dt@...nel.org>,
<pawel.moll@....com>, <mark.rutland@....com>,
<ijc+devicetree@...lion.org.uk>, <galak@...eaurora.org>,
<alexandre.belloni@...e-electrons.com>,
<linux-pwm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<nicolas.ferre@...rochip.com>, <cristian.birsan@...rochip.com>,
<andrei.pistirica@...rochip.com>, <tudor.ambarus@...rochip.com>,
<eugen.hristev@...rochip.com>
Subject: Re: [PATCH v3 1/2] drivers: pwm: pwm-atmel: switch to atomic PWM
On Wed, 22 Mar 2017 15:29:34 +0200
Claudiu Beznea <claudiu.beznea@...rochip.com> wrote:
> The currently Atmel PWM controllers supported by this driver
> could change period or duty factor without channel disable,
> for regular channels (sama5d3 support this by using period
> or duty factor update registers, sam9rl support this by
> writing channel update register and select the corresponding
> update: period or duty factor). The chip doesn't support run
> time changings of signal polarity. To take advantage of
> atomic PWM framework and let controller works without glitches,
> in this patch only the duty factor could be changed without
> disabling PWM channel. For period and signal polarity the
> atomic PWM is simulated by disabling + enabling the right PWM channel.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
>
> ---
> drivers/pwm/pwm-atmel.c | 273 +++++++++++++++++++++++-------------------------
> 1 file changed, 129 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 67a7023..f147154 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -58,17 +58,22 @@
> #define PWM_MAX_PRD 0xFFFF
> #define PRD_MAX_PRES 10
>
> +struct atmel_pwm_registers {
> + u8 period;
> + u8 period_upd;
> + u8 duty;
> + u8 duty_upd;
> +};
> +
> struct atmel_pwm_chip {
> struct pwm_chip chip;
> struct clk *clk;
> void __iomem *base;
> + const struct atmel_pwm_registers *regs;
>
> unsigned int updated_pwms;
> /* ISR is cleared when read, ensure only one thread does that */
> struct mutex isr_lock;
> -
> - void (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
> - unsigned long dty, unsigned long prd);
> };
>
> static inline struct atmel_pwm_chip *to_atmel_pwm_chip(struct pwm_chip *chip)
> @@ -105,153 +110,71 @@ static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip,
> writel_relaxed(val, chip->base + base + offset);
> }
>
> -static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> - int duty_ns, int period_ns)
> +static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
> + const struct pwm_state *state,
> + unsigned long *cprd, u32 *pres)
> {
> struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> - unsigned long prd, dty;
> - unsigned long long div;
> - unsigned int pres = 0;
> - u32 val;
> - int ret;
> -
> - if (pwm_is_enabled(pwm) && (period_ns != pwm_get_period(pwm))) {
> - dev_err(chip->dev, "cannot change PWM period while enabled\n");
> - return -EBUSY;
> - }
> + unsigned long long cycles = state->period;
>
> /* Calculate the period cycles and prescale value */
> - div = (unsigned long long)clk_get_rate(atmel_pwm->clk) * period_ns;
> - do_div(div, NSEC_PER_SEC);
> + cycles *= clk_get_rate(atmel_pwm->clk);
> + do_div(cycles, NSEC_PER_SEC);
>
> - while (div > PWM_MAX_PRD) {
> - div >>= 1;
> - pres++;
> - }
> + for (*pres = 0; cycles > PWM_MAX_PRD; cycles >>= 1)
> + (*pres)++;
>
> - if (pres > PRD_MAX_PRES) {
> + if (*pres > PRD_MAX_PRES) {
> dev_err(chip->dev, "pres exceeds the maximum value\n");
> return -EINVAL;
> }
>
> - /* Calculate the duty cycles */
> - prd = div;
> - div *= duty_ns;
> - do_div(div, period_ns);
> - dty = prd - div;
> -
> - ret = clk_enable(atmel_pwm->clk);
> - if (ret) {
> - dev_err(chip->dev, "failed to enable PWM clock\n");
> - return ret;
> - }
> -
> - /* It is necessary to preserve CPOL, inside CMR */
> - val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> - val = (val & ~PWM_CMR_CPRE_MSK) | (pres & PWM_CMR_CPRE_MSK);
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
> - atmel_pwm->config(chip, pwm, dty, prd);
> - mutex_lock(&atmel_pwm->isr_lock);
> - atmel_pwm->updated_pwms |= atmel_pwm_readl(atmel_pwm, PWM_ISR);
> - atmel_pwm->updated_pwms &= ~(1 << pwm->hwpwm);
> - mutex_unlock(&atmel_pwm->isr_lock);
> + *cprd = cycles;
>
> - clk_disable(atmel_pwm->clk);
> - return ret;
> + return 0;
> }
>
> -static void atmel_pwm_config_v1(struct pwm_chip *chip, struct pwm_device *pwm,
> - unsigned long dty, unsigned long prd)
> +static void atmel_pwm_calculate_cdty(const struct pwm_state *state,
> + unsigned long cprd, unsigned long *cdty)
> {
> - struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> - unsigned int val;
> + unsigned long long cycles = state->duty_cycle;
>
> -
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV1_CUPD, dty);
> -
> - val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> - val &= ~PWM_CMR_UPD_CDTY;
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
> -
> - /*
> - * If the PWM channel is enabled, only update CDTY by using the update
> - * register, it needs to set bit 10 of CMR to 0
> - */
> - if (pwm_is_enabled(pwm))
> - return;
> - /*
> - * If the PWM channel is disabled, write value to duty and period
> - * registers directly.
> - */
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV1_CDTY, dty);
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV1_CPRD, prd);
> + cycles *= cprd;
> + do_div(cycles, state->period);
> + *cdty = cprd - cycles;
> }
>
> -static void atmel_pwm_config_v2(struct pwm_chip *chip, struct pwm_device *pwm,
> - unsigned long dty, unsigned long prd)
> -{
> - struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> -
> - if (pwm_is_enabled(pwm)) {
> - /*
> - * If the PWM channel is enabled, using the duty update register
> - * to update the value.
> - */
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV2_CDTYUPD, dty);
> - } else {
> - /*
> - * If the PWM channel is disabled, write value to duty and
> - * period registers directly.
> - */
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV2_CDTY, dty);
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWMV2_CPRD, prd);
> - }
> -}
> -
> -static int atmel_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> - enum pwm_polarity polarity)
> +static void atmel_pwm_update_cdty(struct pwm_chip *chip, struct pwm_device *pwm,
> + unsigned long cdty)
> {
> struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> u32 val;
> - int ret;
> -
> - val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
>
> - if (polarity == PWM_POLARITY_NORMAL)
> - val &= ~PWM_CMR_CPOL;
> - else
> - val |= PWM_CMR_CPOL;
> -
> - ret = clk_enable(atmel_pwm->clk);
> - if (ret) {
> - dev_err(chip->dev, "failed to enable PWM clock\n");
> - return ret;
> + if (atmel_pwm->regs->duty_upd ==
> + atmel_pwm->regs->period_upd) {
> + val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> + val &= ~PWM_CMR_UPD_CDTY;
> + atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
> }
>
> - atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
> -
> - clk_disable(atmel_pwm->clk);
> -
> - return 0;
> + atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
> + atmel_pwm->regs->duty_upd, cdty);
> }
>
> -static int atmel_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + unsigned long cprd, unsigned long cdty)
> {
> struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> - int ret;
> -
> - ret = clk_enable(atmel_pwm->clk);
> - if (ret) {
> - dev_err(chip->dev, "failed to enable PWM clock\n");
> - return ret;
> - }
>
> - atmel_pwm_writel(atmel_pwm, PWM_ENA, 1 << pwm->hwpwm);
> -
> - return 0;
> + atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
> + atmel_pwm->regs->duty, cdty);
> + atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
> + atmel_pwm->regs->period, cprd);
> }
>
> -static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm,
> + bool disable_clk)
> {
> struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> unsigned long timeout = jiffies + 2 * HZ;
> @@ -282,37 +205,99 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> time_before(jiffies, timeout))
> usleep_range(10, 100);
>
> - clk_disable(atmel_pwm->clk);
> + if (disable_clk)
> + clk_disable(atmel_pwm->clk);
> +}
> +
> +static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + struct pwm_state *state)
> +{
> + struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> + struct pwm_state cstate;
> + unsigned long cprd, cdty;
> + u32 pres, val;
> + int ret;
> +
> + pwm_get_state(pwm, &cstate);
> +
> + if (state->enabled) {
> + if (cstate.enabled &&
> + cstate.polarity == state->polarity &&
> + cstate.period == state->period) {
> + cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
> + atmel_pwm->regs->period);
> + atmel_pwm_calculate_cdty(state, cprd, &cdty);
> + atmel_pwm_update_cdty(chip, pwm, cdty);
> + return 0;
> + }
> +
> + ret = atmel_pwm_calculate_cprd_and_pres(chip, state, &cprd,
> + &pres);
> + if (ret) {
> + dev_err(chip->dev,
> + "failed to calculate cprd and prescaler\n");
> + return ret;
> + }
> +
> + atmel_pwm_calculate_cdty(state, cprd, &cdty);
> +
> + if (cstate.enabled) {
> + atmel_pwm_disable(chip, pwm, false);
> + } else {
> + ret = clk_enable(atmel_pwm->clk);
> + if (ret) {
> + dev_err(chip->dev, "failed to enable clock\n");
> + return ret;
> + }
> + }
> +
> + /* It is necessary to preserve CPOL, inside CMR */
> + val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
> + val = (val & ~PWM_CMR_CPRE_MSK) | (pres & PWM_CMR_CPRE_MSK);
> + if (state->polarity == PWM_POLARITY_NORMAL)
> + val &= ~PWM_CMR_CPOL;
> + else
> + val |= PWM_CMR_CPOL;
> + atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
> + atmel_pwm_set_cprd_cdty(chip, pwm, cprd, cdty);
> + mutex_lock(&atmel_pwm->isr_lock);
> + atmel_pwm->updated_pwms |= atmel_pwm_readl(atmel_pwm, PWM_ISR);
> + atmel_pwm->updated_pwms &= ~(1 << pwm->hwpwm);
> + mutex_unlock(&atmel_pwm->isr_lock);
> + atmel_pwm_writel(atmel_pwm, PWM_ENA, 1 << pwm->hwpwm);
> + } else if (cstate.enabled) {
> + atmel_pwm_disable(chip, pwm, true);
> + }
> +
> + return 0;
> }
>
> static const struct pwm_ops atmel_pwm_ops = {
> - .config = atmel_pwm_config,
> - .set_polarity = atmel_pwm_set_polarity,
> - .enable = atmel_pwm_enable,
> - .disable = atmel_pwm_disable,
> + .apply = atmel_pwm_apply,
> .owner = THIS_MODULE,
> };
>
> -struct atmel_pwm_data {
> - void (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
> - unsigned long dty, unsigned long prd);
> -};
> -
> -static const struct atmel_pwm_data atmel_pwm_data_v1 = {
> - .config = atmel_pwm_config_v1,
> +static const struct atmel_pwm_registers atmel_pwm_regs_v1 = {
> + .period = PWMV1_CPRD,
> + .period_upd = PWMV1_CUPD,
> + .duty = PWMV1_CDTY,
> + .duty_upd = PWMV1_CUPD,
> };
>
> -static const struct atmel_pwm_data atmel_pwm_data_v2 = {
> - .config = atmel_pwm_config_v2,
> +static const struct atmel_pwm_registers atmel_pwm_regs_v2 = {
> + .period = PWMV2_CPRD,
> + .period_upd = PWMV2_CPRDUPD,
> + .duty = PWMV2_CDTY,
> + .duty_upd = PWMV2_CDTYUPD,
> };
>
> static const struct platform_device_id atmel_pwm_devtypes[] = {
> {
> .name = "at91sam9rl-pwm",
> - .driver_data = (kernel_ulong_t)&atmel_pwm_data_v1,
> + .driver_data = (kernel_ulong_t)&atmel_pwm_regs_v1,
> }, {
> .name = "sama5d3-pwm",
> - .driver_data = (kernel_ulong_t)&atmel_pwm_data_v2,
> + .driver_data = (kernel_ulong_t)&atmel_pwm_regs_v2,
> }, {
> /* sentinel */
> },
> @@ -322,17 +307,17 @@ MODULE_DEVICE_TABLE(platform, atmel_pwm_devtypes);
> static const struct of_device_id atmel_pwm_dt_ids[] = {
> {
> .compatible = "atmel,at91sam9rl-pwm",
> - .data = &atmel_pwm_data_v1,
> + .data = &atmel_pwm_regs_v1,
> }, {
> .compatible = "atmel,sama5d3-pwm",
> - .data = &atmel_pwm_data_v2,
> + .data = &atmel_pwm_regs_v2,
> }, {
> /* sentinel */
> },
> };
> MODULE_DEVICE_TABLE(of, atmel_pwm_dt_ids);
>
> -static inline const struct atmel_pwm_data *
> +static inline const struct atmel_pwm_registers *
> atmel_pwm_get_driver_data(struct platform_device *pdev)
> {
> const struct platform_device_id *id;
> @@ -342,18 +327,18 @@ atmel_pwm_get_driver_data(struct platform_device *pdev)
>
> id = platform_get_device_id(pdev);
>
> - return (struct atmel_pwm_data *)id->driver_data;
> + return (struct atmel_pwm_registers *)id->driver_data;
> }
>
> static int atmel_pwm_probe(struct platform_device *pdev)
> {
> - const struct atmel_pwm_data *data;
> + const struct atmel_pwm_registers *regs;
> struct atmel_pwm_chip *atmel_pwm;
> struct resource *res;
> int ret;
>
> - data = atmel_pwm_get_driver_data(pdev);
> - if (!data)
> + regs = atmel_pwm_get_driver_data(pdev);
> + if (!regs)
> return -ENODEV;
>
> atmel_pwm = devm_kzalloc(&pdev->dev, sizeof(*atmel_pwm), GFP_KERNEL);
> @@ -385,7 +370,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
>
> atmel_pwm->chip.base = -1;
> atmel_pwm->chip.npwm = 4;
> - atmel_pwm->config = data->config;
> + atmel_pwm->regs = regs;
> atmel_pwm->updated_pwms = 0;
> mutex_init(&atmel_pwm->isr_lock);
>
Powered by blists - more mailing lists