diff mbox

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

Message ID 1498739271-27431-4-git-send-email-david.wu@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Wu June 29, 2017, 12:27 p.m. UTC 
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. UTC | #1 
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[] = {
David Wu July 4, 2017, 6:46 a.m. UTC | #2 
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.
diff mbox

Patch

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