On Thu, 29 Jun 2017 20:27:50 +0800 David Wu <david.wu@xxxxxxxxxxxxxx> wrote: > The rk3328 soc supports atomic update, we could lock the configuration > of period and duty at first, after unlock is configured, the period and > duty are effective at the same time. > > If the polarity, period and duty need to be configured together, > the way for atomic update is "configure lock and old polarity" -> > "configure period and duty" -> "configure unlock and new polarity". > > Signed-off-by: David Wu <david.wu@xxxxxxxxxxxxxx> > --- > .../devicetree/bindings/pwm/pwm-rockchip.txt | 1 + > drivers/pwm/pwm-rockchip.c | 50 +++++++++++++++++++--- > 2 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt > index 2350ef9..152c736 100644 > --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt > +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt > @@ -4,6 +4,7 @@ Required properties: > - compatible: should be "rockchip,<name>-pwm" > "rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs > "rockchip,rk3288-pwm": found on RK3288 SoC > + "rockchip,rk3328-pwm": found on RK3328 SoC > "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC > - reg: physical base address and length of the controller's registers > - clocks: See ../clock/clock-bindings.txt > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eb630ff..d8801ae8 100644 > --- a/drivers/pwm/pwm-rockchip.c > +++ b/drivers/pwm/pwm-rockchip.c > @@ -29,6 +29,7 @@ > #define PWM_INACTIVE_POSITIVE (1 << 4) > #define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE) > #define PWM_OUTPUT_LEFT (0 << 5) > +#define PWM_LOCK_EN (1 << 6) > #define PWM_LP_DISABLE (0 << 8) > > struct rockchip_pwm_chip { > @@ -50,6 +51,7 @@ struct rockchip_pwm_data { > struct rockchip_pwm_regs regs; > unsigned int prescaler; > bool supports_polarity; > + bool supports_atomic_update; Yet another customization. Don't you think we can extract common parts, expose them as helpers and then have 3 different pwm_ops (with 3 different ->apply() implementation), one for each IP revision. > const struct pwm_ops *ops; > > void (*set_enable)(struct pwm_chip *chip, > @@ -201,6 +203,14 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip, > clk_rate = clk_get_rate(pc->clk); > > /* > + * Lock the period and duty of previous configuration, then > + * change the duty and period, that would not be effective. > + */ > + ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl); > + ctrl |= PWM_LOCK_EN; > + writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl); > + > + /* > * 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. > @@ -222,6 +232,12 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip, > else > ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > > + /* > + * Unlock and set polarity at the same time, > + * the configuration of duty, period and polarity > + * would be effective together at next period. > + */ > + ctrl &= ~PWM_LOCK_EN; > writel(ctrl, pc->base + pc->data->regs.ctrl); > } > > @@ -261,11 +277,18 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > if (ret) > return ret; > > - if (state->polarity != curstate.polarity && enabled) { > - ret = rockchip_pwm_enable(chip, pwm, false); > - if (ret) > - goto out; > - enabled = false; > + /* > + * If the atomic update is supported, then go to the pwm config, > + * no need to do this, it could ensure the atomic update for polarity > + * changed. > + */ > + if (pc->data->supports_atomic_update) { > + if (state->polarity != curstate.polarity && enabled) { > + ret = rockchip_pwm_enable(chip, pwm, false); > + if (ret) > + goto out; > + enabled = false; > + } > } > > pc->data->pwm_config(chip, pwm, state->duty_cycle, > @@ -345,9 +368,26 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > .pwm_config = rockchip_pwm_config_v2, > }; > > +static const struct rockchip_pwm_data pwm_data_v3 = { > + .regs = { > + .duty = 0x08, > + .period = 0x04, > + .cntr = 0x00, > + .ctrl = 0x0c, > + }, > + .prescaler = 1, > + .supports_polarity = true, > + .supports_atomic_update = true, > + .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[] = { > { .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1}, > { .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2}, > + { .compatible = "rockchip,rk3328-pwm", .data = &pwm_data_v3}, > { .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop}, > { /* sentinel */ } > }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html