[PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[Alexandre Belloni]

Switch to atomic PWM. The main goal is to properly wait for a period before
disabling a channel to ensure the correct level is set on the output.

Changes in v2:
 - fixed remaining checkpatch warnings
 - split the series to ease reviews
 - changed the delay handling to ensure the proper amount of time has elapsed
   before disabling a channel
 - dropped RDY bit handling as it doesn't have any real effect.

Alexandre Belloni (3):
  pwm: sun4i: improve hardware read out
  pwm: sun4i: switch to atomic PWM
  pwm: sun4i: drop legacy callbacks

 drivers/pwm/pwm-sun4i.c | 263 +++++++++++++++++++++++++++---------------------
 1 file changed, 148 insertions(+), 115 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] pwm: sun4i: improve hardware read out

[Alexandre Belloni]

Implement .get_state instead of only reading the polarity at probe time.
This allows to get the proper state, period and duty cycle.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/pwm/pwm-sun4i.c | 65 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 1284ffa05921..175a69245a8a 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -44,6 +44,10 @@
 
 #define PWM_DTY_MASK		GENMASK(15, 0)
 
+#define PWM_REG_PRD(reg)	((((reg) >> 16) & PWM_PRD_MASK) + 1)
+#define PWM_REG_DTY(reg)	((reg) & PWM_DTY_MASK)
+#define PWM_REG_PRESCAL(reg, chan)	(((reg) >> ((chan) * PWMCH_OFFSET)) & PWM_PRESCAL_MASK)
+
 #define BIT_CH(bit, chan)	((bit) << ((chan) * PWMCH_OFFSET))
 
 static const u32 prescaler_table[] = {
@@ -96,6 +100,46 @@ static inline void sun4i_pwm_writel(struct sun4i_pwm_chip *chip,
 	writel(val, chip->base + offset);
 }
 
+static void sun4i_pwm_get_state(struct pwm_chip *chip,
+				struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
+	u64 clk_rate, tmp;
+	u32 val;
+	unsigned int prescaler;
+
+	clk_rate = clk_get_rate(sun4i_pwm->clk);
+
+	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
+
+	if ((val == PWM_PRESCAL_MASK) && sun4i_pwm->data->has_prescaler_bypass)
+		prescaler = 1;
+	else
+		prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)];
+
+	if (prescaler == 0)
+		return;
+
+	if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm))
+		state->polarity = PWM_POLARITY_NORMAL;
+	else
+		state->polarity = PWM_POLARITY_INVERSED;
+
+	if (val & BIT_CH(PWM_CLK_GATING | PWM_EN, pwm->hwpwm))
+		state->enabled = true;
+	else
+		state->enabled = false;
+
+	val = sun4i_pwm_readl(sun4i_pwm, PWM_CH_PRD(pwm->hwpwm));
+
+	tmp = prescaler * NSEC_PER_SEC * PWM_REG_DTY(val);
+	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
+
+	tmp = prescaler * NSEC_PER_SEC * PWM_REG_PRD(val);
+	state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
+}
+
 static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			    int duty_ns, int period_ns)
 {
@@ -257,6 +301,7 @@ static const struct pwm_ops sun4i_pwm_ops = {
 	.set_polarity = sun4i_pwm_set_polarity,
 	.enable = sun4i_pwm_enable,
 	.disable = sun4i_pwm_disable,
+	.get_state = sun4i_pwm_get_state,
 	.owner = THIS_MODULE,
 };
 
@@ -316,8 +361,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 {
 	struct sun4i_pwm_chip *pwm;
 	struct resource *res;
-	u32 val;
-	int i, ret;
+	int ret;
 	const struct of_device_id *match;
 
 	match = of_match_device(sun4i_pwm_dt_ids, &pdev->dev);
@@ -353,24 +397,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pwm);
 
-	ret = clk_prepare_enable(pwm->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to enable PWM clock\n");
-		goto clk_error;
-	}
-
-	val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
-	for (i = 0; i < pwm->chip.npwm; i++)
-		if (!(val & BIT_CH(PWM_ACT_STATE, i)))
-			pwm_set_polarity(&pwm->chip.pwms[i],
-					 PWM_POLARITY_INVERSED);
-	clk_disable_unprepare(pwm->clk);
-
 	return 0;
-
-clk_error:
-	pwmchip_remove(&pwm->chip);
-	return ret;
 }
 
 static int sun4i_pwm_remove(struct platform_device *pdev)
-- 
2.11.0

Re: [PATCH v2 1/3] pwm: sun4i: improve hardware read out

[Chen-Yu Tsai]

On Wed, May 31, 2017 at 3:32 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Implement .get_state instead of only reading the polarity at probe time.
> This allows to get the proper state, period and duty cycle.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/pwm/pwm-sun4i.c | 65 ++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 1284ffa05921..175a69245a8a 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -44,6 +44,10 @@
>
>  #define PWM_DTY_MASK           GENMASK(15, 0)
>
> +#define PWM_REG_PRD(reg)       ((((reg) >> 16) & PWM_PRD_MASK) + 1)
> +#define PWM_REG_DTY(reg)       ((reg) & PWM_DTY_MASK)
> +#define PWM_REG_PRESCAL(reg, chan)     (((reg) >> ((chan) * PWMCH_OFFSET)) & PWM_PRESCAL_MASK)
> +
>  #define BIT_CH(bit, chan)      ((bit) << ((chan) * PWMCH_OFFSET))
>
>  static const u32 prescaler_table[] = {
> @@ -96,6 +100,46 @@ static inline void sun4i_pwm_writel(struct sun4i_pwm_chip *chip,
>         writel(val, chip->base + offset);
>  }
>
> +static void sun4i_pwm_get_state(struct pwm_chip *chip,
> +                               struct pwm_device *pwm,
> +                               struct pwm_state *state)
> +{
> +       struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> +       u64 clk_rate, tmp;
> +       u32 val;
> +       unsigned int prescaler;
> +
> +       clk_rate = clk_get_rate(sun4i_pwm->clk);
> +
> +       val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> +
> +       if ((val == PWM_PRESCAL_MASK) && sun4i_pwm->data->has_prescaler_bypass)
> +               prescaler = 1;
> +       else
> +               prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)];
> +
> +       if (prescaler == 0)
> +               return;

This looks like an invalid state. How does the PWM core handle it?
Or rather, how does the driver signal an invalid state?

Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> +
> +       if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm))
> +               state->polarity = PWM_POLARITY_NORMAL;
> +       else
> +               state->polarity = PWM_POLARITY_INVERSED;
> +
> +       if (val & BIT_CH(PWM_CLK_GATING | PWM_EN, pwm->hwpwm))
> +               state->enabled = true;
> +       else
> +               state->enabled = false;
> +
> +       val = sun4i_pwm_readl(sun4i_pwm, PWM_CH_PRD(pwm->hwpwm));
> +
> +       tmp = prescaler * NSEC_PER_SEC * PWM_REG_DTY(val);
> +       state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
> +
> +       tmp = prescaler * NSEC_PER_SEC * PWM_REG_PRD(val);
> +       state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
> +}
> +
>  static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>                             int duty_ns, int period_ns)
>  {
> @@ -257,6 +301,7 @@ static const struct pwm_ops sun4i_pwm_ops = {
>         .set_polarity = sun4i_pwm_set_polarity,
>         .enable = sun4i_pwm_enable,
>         .disable = sun4i_pwm_disable,
> +       .get_state = sun4i_pwm_get_state,
>         .owner = THIS_MODULE,
>  };
>
> @@ -316,8 +361,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>  {
>         struct sun4i_pwm_chip *pwm;
>         struct resource *res;
> -       u32 val;
> -       int i, ret;
> +       int ret;
>         const struct of_device_id *match;
>
>         match = of_match_device(sun4i_pwm_dt_ids, &pdev->dev);
> @@ -353,24 +397,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, pwm);
>
> -       ret = clk_prepare_enable(pwm->clk);
> -       if (ret) {
> -               dev_err(&pdev->dev, "failed to enable PWM clock\n");
> -               goto clk_error;
> -       }
> -
> -       val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
> -       for (i = 0; i < pwm->chip.npwm; i++)
> -               if (!(val & BIT_CH(PWM_ACT_STATE, i)))
> -                       pwm_set_polarity(&pwm->chip.pwms[i],
> -                                        PWM_POLARITY_INVERSED);
> -       clk_disable_unprepare(pwm->clk);
> -
>         return 0;
> -
> -clk_error:
> -       pwmchip_remove(&pwm->chip);
> -       return ret;
>  }
>
>  static int sun4i_pwm_remove(struct platform_device *pdev)
> --
> 2.11.0
>

Re: [PATCH v2 1/3] pwm: sun4i: improve hardware read out

[Alexandre Belloni]

On 31/05/2017 at 11:34:43 +0800, Chen-Yu Tsai wrote:
> On Wed, May 31, 2017 at 3:32 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > +static void sun4i_pwm_get_state(struct pwm_chip *chip,
> > +                               struct pwm_device *pwm,
> > +                               struct pwm_state *state)
> > +{
> > +       struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > +       u64 clk_rate, tmp;
> > +       u32 val;
> > +       unsigned int prescaler;
> > +
> > +       clk_rate = clk_get_rate(sun4i_pwm->clk);
> > +
> > +       val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > +
> > +       if ((val == PWM_PRESCAL_MASK) && sun4i_pwm->data->has_prescaler_bypass)
> > +               prescaler = 1;
> > +       else
> > +               prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)];
> > +
> > +       if (prescaler == 0)
> > +               return;
> 
> This looks like an invalid state. How does the PWM core handle it?
> Or rather, how does the driver signal an invalid state?
> 

It doesn't and it doesn't really matter, if the IP is in an invalid
state, this bails out without modifying the know state so the core will
behave as if there was no hardware read out and everything will be
synchronized on the first ->apply() call.

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 1/3] pwm: sun4i: improve hardware read out

[Chen-Yu Tsai]

On Wed, May 31, 2017 at 3:35 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 31/05/2017 at 11:34:43 +0800, Chen-Yu Tsai wrote:
>> On Wed, May 31, 2017 at 3:32 AM, Alexandre Belloni
>> <alexandre.belloni@free-electrons.com> wrote:
>> > +static void sun4i_pwm_get_state(struct pwm_chip *chip,
>> > +                               struct pwm_device *pwm,
>> > +                               struct pwm_state *state)
>> > +{
>> > +       struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
>> > +       u64 clk_rate, tmp;
>> > +       u32 val;
>> > +       unsigned int prescaler;
>> > +
>> > +       clk_rate = clk_get_rate(sun4i_pwm->clk);
>> > +
>> > +       val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
>> > +
>> > +       if ((val == PWM_PRESCAL_MASK) && sun4i_pwm->data->has_prescaler_bypass)
>> > +               prescaler = 1;
>> > +       else
>> > +               prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)];
>> > +
>> > +       if (prescaler == 0)
>> > +               return;
>>
>> This looks like an invalid state. How does the PWM core handle it?
>> Or rather, how does the driver signal an invalid state?
>>
>
> It doesn't and it doesn't really matter, if the IP is in an invalid
> state, this bails out without modifying the know state so the core will
> behave as if there was no hardware read out and everything will be
> synchronized on the first ->apply() call.

Cool.

Thanks
ChenYu

>
>> Otherwise,
>>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[PATCH v2 2/3] pwm: sun4i: switch to atomic PWM

[Alexandre Belloni]

Switch the driver to atomic PWM. This makes it easier to wait a proper
amount of time when changing the duty cycle before disabling the channel
(main use case is switching the duty cycle to 0 before disabling).

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/pwm/pwm-sun4i.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 175a69245a8a..cd8737d0804f 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -8,8 +8,10 @@
 
 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/jiffies.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -81,6 +83,8 @@ struct sun4i_pwm_chip {
 	void __iomem *base;
 	spinlock_t ctrl_lock;
 	const struct sun4i_pwm_data *data;
+	unsigned long next_period[2];
+	bool needs_delay[2];
 };
 
 static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip)
@@ -140,6 +144,167 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
 	state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
 }
 
+static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm,
+			       struct pwm_state *state,
+			       u32 *dty, u32 *prd, unsigned int *prsclr)
+{
+	u64 clk_rate, div = 0;
+	unsigned int pval, prescaler = 0;
+
+	clk_rate = clk_get_rate(sun4i_pwm->clk);
+
+	if (sun4i_pwm->data->has_prescaler_bypass) {
+		/* First, test without any prescaler when available */
+		prescaler = PWM_PRESCAL_MASK;
+		pval = 1;
+		/*
+		 * When not using any prescaler, the clock period in nanoseconds
+		 * is not an integer so round it half up instead of
+		 * truncating to get less surprising values.
+		 */
+		div = clk_rate * state->period + NSEC_PER_SEC / 2;
+		do_div(div, NSEC_PER_SEC);
+		if (div - 1 > PWM_PRD_MASK)
+			prescaler = 0;
+	}
+
+	if (prescaler == 0) {
+		/* Go up from the first divider */
+		for (prescaler = 0; prescaler < PWM_PRESCAL_MASK; prescaler++) {
+			if (!prescaler_table[prescaler])
+				continue;
+			pval = prescaler_table[prescaler];
+			div = clk_rate;
+			do_div(div, pval);
+			div = div * state->period;
+			do_div(div, NSEC_PER_SEC);
+			if (div - 1 <= PWM_PRD_MASK)
+				break;
+		}
+
+		if (div - 1 > PWM_PRD_MASK)
+			return -EINVAL;
+	}
+
+	*prd = div;
+	div *= state->duty_cycle;
+	do_div(div, state->period);
+	*dty = div;
+	*prsclr = prescaler;
+
+	div = (u64)pval * NSEC_PER_SEC * *prd;
+	state->period = DIV_ROUND_CLOSEST_ULL(div, clk_rate);
+
+	div = (u64)pval * NSEC_PER_SEC * *dty;
+	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(div, clk_rate);
+
+	return 0;
+}
+
+static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			   struct pwm_state *state)
+{
+	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
+	struct pwm_state cstate;
+	u32 ctrl;
+	int ret;
+	unsigned int delay_us;
+	unsigned long now;
+
+	pwm_get_state(pwm, &cstate);
+
+	if (!cstate.enabled) {
+		ret = clk_prepare_enable(sun4i_pwm->clk);
+		if (ret) {
+			dev_err(chip->dev, "failed to enable PWM clock\n");
+			return ret;
+		}
+	}
+
+	spin_lock(&sun4i_pwm->ctrl_lock);
+	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
+
+	if ((cstate.period != state->period) ||
+	    (cstate.duty_cycle != state->duty_cycle)) {
+		u32 period, duty, val;
+		unsigned int prescaler;
+
+		ret = sun4i_pwm_calculate(sun4i_pwm, state,
+					  &duty, &period, &prescaler);
+		if (ret) {
+			dev_err(chip->dev, "period exceeds the maximum value\n");
+			spin_unlock(&sun4i_pwm->ctrl_lock);
+			if (!cstate.enabled)
+				clk_disable_unprepare(sun4i_pwm->clk);
+			return ret;
+		}
+
+		if (PWM_REG_PRESCAL(ctrl, pwm->hwpwm) != prescaler) {
+			/* Prescaler changed, the clock has to be gated */
+			ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
+			sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
+
+			ctrl &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm);
+			ctrl |= BIT_CH(prescaler, pwm->hwpwm);
+		}
+
+		val = (duty & PWM_DTY_MASK) | PWM_PRD(period);
+		sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
+		sun4i_pwm->next_period[pwm->hwpwm] = jiffies +
+			usecs_to_jiffies(cstate.period / 1000 + 1);
+		sun4i_pwm->needs_delay[pwm->hwpwm] = true;
+	}
+
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		ctrl &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm);
+	else
+		ctrl |= BIT_CH(PWM_ACT_STATE, pwm->hwpwm);
+
+	ctrl |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
+	if (state->enabled) {
+		ctrl |= BIT_CH(PWM_EN, pwm->hwpwm);
+	} else if (!sun4i_pwm->needs_delay[pwm->hwpwm]) {
+		ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
+		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
+	}
+
+	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
+
+	spin_unlock(&sun4i_pwm->ctrl_lock);
+
+	if (state->enabled)
+		return 0;
+
+	if (!sun4i_pwm->needs_delay[pwm->hwpwm]) {
+		clk_disable_unprepare(sun4i_pwm->clk);
+		return 0;
+	}
+
+	/* We need a full period to elapse before disabling the channel. */
+	now = jiffies;
+	if (sun4i_pwm->needs_delay[pwm->hwpwm] &&
+	    time_before(now, sun4i_pwm->next_period[pwm->hwpwm])) {
+		delay_us = jiffies_to_usecs(sun4i_pwm->next_period[pwm->hwpwm] -
+					   now);
+		if ((delay_us / 500) > MAX_UDELAY_MS)
+			msleep(delay_us / 1000 + 1);
+		else
+			usleep_range(delay_us, delay_us * 2);
+	}
+	sun4i_pwm->needs_delay[pwm->hwpwm] = false;
+
+	spin_lock(&sun4i_pwm->ctrl_lock);
+	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
+	ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
+	ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
+	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
+	spin_unlock(&sun4i_pwm->ctrl_lock);
+
+	clk_disable_unprepare(sun4i_pwm->clk);
+
+	return 0;
+}
+
 static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			    int duty_ns, int period_ns)
 {
@@ -301,6 +466,7 @@ static const struct pwm_ops sun4i_pwm_ops = {
 	.set_polarity = sun4i_pwm_set_polarity,
 	.enable = sun4i_pwm_enable,
 	.disable = sun4i_pwm_disable,
+	.apply = sun4i_pwm_apply,
 	.get_state = sun4i_pwm_get_state,
 	.owner = THIS_MODULE,
 };
-- 
2.11.0

[PATCH v2 3/3] pwm: sun4i: drop legacy callbacks

[Alexandre Belloni]

Remove the legacy callbacks .enable(), .disable(), .set_polarity() and
.config().

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/pwm/pwm-sun4i.c | 160 ------------------------------------------------
 1 file changed, 160 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index cd8737d0804f..6d23f1d1c9b7 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -305,167 +305,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
-static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-			    int duty_ns, int period_ns)
-{
-	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 prd, dty, val, clk_gate;
-	u64 clk_rate, div = 0;
-	unsigned int prescaler = 0;
-	int err;
-
-	clk_rate = clk_get_rate(sun4i_pwm->clk);
-
-	if (sun4i_pwm->data->has_prescaler_bypass) {
-		/* First, test without any prescaler when available */
-		prescaler = PWM_PRESCAL_MASK;
-		/*
-		 * When not using any prescaler, the clock period in nanoseconds
-		 * is not an integer so round it half up instead of
-		 * truncating to get less surprising values.
-		 */
-		div = clk_rate * period_ns + NSEC_PER_SEC / 2;
-		do_div(div, NSEC_PER_SEC);
-		if (div - 1 > PWM_PRD_MASK)
-			prescaler = 0;
-	}
-
-	if (prescaler == 0) {
-		/* Go up from the first divider */
-		for (prescaler = 0; prescaler < PWM_PRESCAL_MASK; prescaler++) {
-			if (!prescaler_table[prescaler])
-				continue;
-			div = clk_rate;
-			do_div(div, prescaler_table[prescaler]);
-			div = div * period_ns;
-			do_div(div, NSEC_PER_SEC);
-			if (div - 1 <= PWM_PRD_MASK)
-				break;
-		}
-
-		if (div - 1 > PWM_PRD_MASK) {
-			dev_err(chip->dev, "period exceeds the maximum value\n");
-			return -EINVAL;
-		}
-	}
-
-	prd = div;
-	div *= duty_ns;
-	do_div(div, period_ns);
-	dty = div;
-
-	err = clk_prepare_enable(sun4i_pwm->clk);
-	if (err) {
-		dev_err(chip->dev, "failed to enable PWM clock\n");
-		return err;
-	}
-
-	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-
-	if (sun4i_pwm->data->has_rdy && (val & PWM_RDY(pwm->hwpwm))) {
-		spin_unlock(&sun4i_pwm->ctrl_lock);
-		clk_disable_unprepare(sun4i_pwm->clk);
-		return -EBUSY;
-	}
-
-	clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	if (clk_gate) {
-		val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-		sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-	}
-
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm);
-	val |= BIT_CH(prescaler, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-
-	val = (dty & PWM_DTY_MASK) | PWM_PRD(prd);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
-
-	if (clk_gate) {
-		val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-		val |= clk_gate;
-		sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-	}
-
-	spin_unlock(&sun4i_pwm->ctrl_lock);
-	clk_disable_unprepare(sun4i_pwm->clk);
-
-	return 0;
-}
-
-static int sun4i_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-				  enum pwm_polarity polarity)
-{
-	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
-	int ret;
-
-	ret = clk_prepare_enable(sun4i_pwm->clk);
-	if (ret) {
-		dev_err(chip->dev, "failed to enable PWM clock\n");
-		return ret;
-	}
-
-	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-
-	if (polarity != PWM_POLARITY_NORMAL)
-		val &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm);
-	else
-		val |= BIT_CH(PWM_ACT_STATE, pwm->hwpwm);
-
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-
-	spin_unlock(&sun4i_pwm->ctrl_lock);
-	clk_disable_unprepare(sun4i_pwm->clk);
-
-	return 0;
-}
-
-static int sun4i_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
-	int ret;
-
-	ret = clk_prepare_enable(sun4i_pwm->clk);
-	if (ret) {
-		dev_err(chip->dev, "failed to enable PWM clock\n");
-		return ret;
-	}
-
-	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val |= BIT_CH(PWM_EN, pwm->hwpwm);
-	val |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-	spin_unlock(&sun4i_pwm->ctrl_lock);
-
-	return 0;
-}
-
-static void sun4i_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
-
-	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
-	val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-	spin_unlock(&sun4i_pwm->ctrl_lock);
-
-	clk_disable_unprepare(sun4i_pwm->clk);
-}
-
 static const struct pwm_ops sun4i_pwm_ops = {
-	.config = sun4i_pwm_config,
-	.set_polarity = sun4i_pwm_set_polarity,
-	.enable = sun4i_pwm_enable,
-	.disable = sun4i_pwm_disable,
 	.apply = sun4i_pwm_apply,
 	.get_state = sun4i_pwm_get_state,
 	.owner = THIS_MODULE,
-- 
2.11.0

Re: [PATCH v2 3/3] pwm: sun4i: drop legacy callbacks

[Chen-Yu Tsai]

On Wed, May 31, 2017 at 3:32 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Remove the legacy callbacks .enable(), .disable(), .set_polarity() and
> .config().
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

FWIW,

Acked-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Tue, May 30, 2017 at 09:32:06PM +0200, Alexandre Belloni wrote:
> Switch to atomic PWM. The main goal is to properly wait for a period before
> disabling a channel to ensure the correct level is set on the output.
> 
> Changes in v2:
>  - fixed remaining checkpatch warnings
>  - split the series to ease reviews
>  - changed the delay handling to ensure the proper amount of time has elapsed
>    before disabling a channel
>  - dropped RDY bit handling as it doesn't have any real effect.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

On Tue, May 30, 2017 at 09:32:06PM +0200, Alexandre Belloni wrote:
> Switch to atomic PWM. The main goal is to properly wait for a period before
> disabling a channel to ensure the correct level is set on the output.
> 
> Changes in v2:
>  - fixed remaining checkpatch warnings
>  - split the series to ease reviews
>  - changed the delay handling to ensure the proper amount of time has elapsed
>    before disabling a channel
>  - dropped RDY bit handling as it doesn't have any real effect.
> 
> Alexandre Belloni (3):
>   pwm: sun4i: improve hardware read out
>   pwm: sun4i: switch to atomic PWM
>   pwm: sun4i: drop legacy callbacks
> 
>  drivers/pwm/pwm-sun4i.c | 263 +++++++++++++++++++++++++++---------------------
>  1 file changed, 148 insertions(+), 115 deletions(-)

Series applied, though you could've just squashed together patches 2 and
3 because they don't make sense apart.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[Alexandre Belloni]

On 06/07/2017 at 08:49:22 +0200, Thierry Reding wrote:
> On Tue, May 30, 2017 at 09:32:06PM +0200, Alexandre Belloni wrote:
> > Switch to atomic PWM. The main goal is to properly wait for a period before
> > disabling a channel to ensure the correct level is set on the output.
> > 
> > Changes in v2:
> >  - fixed remaining checkpatch warnings
> >  - split the series to ease reviews
> >  - changed the delay handling to ensure the proper amount of time has elapsed
> >    before disabling a channel
> >  - dropped RDY bit handling as it doesn't have any real effect.
> > 
> > Alexandre Belloni (3):
> >   pwm: sun4i: improve hardware read out
> >   pwm: sun4i: switch to atomic PWM
> >   pwm: sun4i: drop legacy callbacks
> > 
> >  drivers/pwm/pwm-sun4i.c | 263 +++++++++++++++++++++++++++---------------------
> >  1 file changed, 148 insertions(+), 115 deletions(-)
> 
> Series applied, though you could've just squashed together patches 2 and
> 3 because they don't make sense apart.
> 

Ok, thanks. IIRC, Boris suggested the split ;)

> Thierry



-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 0/3] pwm: sun4i: switch to atomic PWM

[Boris Brezillon]

On Thu, 6 Jul 2017 09:00:16 +0200
Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 06/07/2017 at 08:49:22 +0200, Thierry Reding wrote:
> > On Tue, May 30, 2017 at 09:32:06PM +0200, Alexandre Belloni wrote:  
> > > Switch to atomic PWM. The main goal is to properly wait for a period before
> > > disabling a channel to ensure the correct level is set on the output.
> > > 
> > > Changes in v2:
> > >  - fixed remaining checkpatch warnings
> > >  - split the series to ease reviews
> > >  - changed the delay handling to ensure the proper amount of time has elapsed
> > >    before disabling a channel
> > >  - dropped RDY bit handling as it doesn't have any real effect.
> > > 
> > > Alexandre Belloni (3):
> > >   pwm: sun4i: improve hardware read out
> > >   pwm: sun4i: switch to atomic PWM
> > >   pwm: sun4i: drop legacy callbacks
> > > 
> > >  drivers/pwm/pwm-sun4i.c | 263 +++++++++++++++++++++++++++---------------------
> > >  1 file changed, 148 insertions(+), 115 deletions(-)  
> > 
> > Series applied, though you could've just squashed together patches 2 and
> > 3 because they don't make sense apart.
> >   
> 
> Ok, thanks. IIRC, Boris suggested the split ;)

Yes, I did it because the diff was really hard to review.