[3/5] pwm: rockchip: Move the configuration of polarity from rockchip_pwm_set_enable() to rockchip_pwm_config()

Submitted by David Wu on June 29, 2017, 12:27 p.m.

Details

Message ID 1498739271-27431-4-git-send-email-david.wu@rock-chips.com
State New
Headers show

Commit Message

David Wu June 29, 2017, 12:27 p.m.
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@rock-chips.com>
---
 drivers/pwm/pwm-rockchip.c | 77 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 20 deletions(-)

Comments

Boris Brezillon July 3, 2017, 6:36 p.m.
On Thu, 29 Jun 2017 20:27:49 +0800
David Wu <david.wu@rock-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@rock-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[] = {

--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Wu July 4, 2017, 6:46 a.m.
Hi Boris,

在 2017/7/4 2:36, Boris Brezillon 写道:
> Hm, maybe it's time to drop these custom hooks and implement
> pwm_apply_v1 and pwm_apply_v2 instead.

Okay, drop the enable and config hooks, only use the apply hook to 
instead them.

--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

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);
 };
 
 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[] = {