On Thu, Jul 24, 2014 at 06:21:35PM +0800, Caesar Wang wrote: > This patch added to support the PWM controller found on > RK3288 SoC. > > Signed-off-by: Caesar Wang <caesar.wang@xxxxxxxxxxxxxx> > --- > drivers/pwm/pwm-rockchip.c | 124 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 105 insertions(+), 19 deletions(-) > > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eec2145..59c2513 100644 > --- a/drivers/pwm/pwm-rockchip.c > +++ b/drivers/pwm/pwm-rockchip.c > @@ -2,6 +2,7 @@ > * PWM driver for Rockchip SoCs > * > * Copyright (C) 2014 Beniamino Galvani <b.galvani@xxxxxxxxx> > + * Copyright (C) 2014 ROCKCHIP, Inc. > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > @@ -12,6 +13,7 @@ > #include <linux/io.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/pwm.h> > #include <linux/time.h> > @@ -25,17 +27,72 @@ > > #define PRESCALER 2 > > +#define PWM_ENABLE (1 << 0) > +#define PWM_CONTINUOUS (1 << 1) > +#define PWM_DUTY_POSITIVE (1 << 3) > +#define PWM_INACTIVE_NEGATIVE (0 << 4) > +#define PWM_OUTPUT_LEFT (0 << 5) > +#define PWM_LP_DISABLE (0 << 8) > + > struct rockchip_pwm_chip { > struct pwm_chip chip; > struct clk *clk; > + const struct rockchip_pwm_data *data; > void __iomem *base; > }; > > +struct rockchip_pwm_regs { > + unsigned long duty; > + unsigned long period; > + unsigned long cntr; > + unsigned long ctrl; > +}; > + > +struct rockchip_pwm_data { > + struct rockchip_pwm_regs regs; > + unsigned int prescaler; > + > + void (*set_enable)(struct pwm_chip *chip, bool enable); > +}; > + > static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c) > { > return container_of(c, struct rockchip_pwm_chip, chip); > } > > +static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > +static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > + PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > int duty_ns, int period_ns) > { > @@ -52,20 +109,20 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > * default prescaler value for all practical clock rate values. > */ > div = clk_rate * period_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > period = div; > > div = clk_rate * duty_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > duty = div; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - writel(period, pc->base + PWM_LRC); > - writel(duty, pc->base + PWM_HRC); > - writel(0, pc->base + PWM_CNTR); > + writel(period, pc->base + pc->data->regs.period); > + writel(duty, pc->base + pc->data->regs.duty); > + writel(0, pc->base + pc->data->regs.cntr); > > clk_disable(pc->clk); > > @@ -76,15 +133,12 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > int ret; > - u32 val; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val |= PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, true); > > return 0; > } > @@ -92,11 +146,8 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > static void rockchip_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) > { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > - u32 val; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val &= ~(PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN); > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, false); > > clk_disable(pc->clk); > } > @@ -108,12 +159,52 @@ static const struct pwm_ops rockchip_pwm_ops = { > .owner = THIS_MODULE, > }; > > +static const struct rockchip_pwm_data pwm_data_v1 = { > + .regs.duty = PWM_HRC, > + .regs.period = PWM_LRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, Perhaps a slightly more idiomatic way to write this would be: static const struct rockchip_pwm_data pwm_data_v1 = { .regs = { .duty = PWM_HRC, .period = PWM_LRC, .cntr = PWM_CNTR, .ctrl = PWM_CTRL, }, ... }; And similar for the v2 and vop structures. And like I said in another reply, since the defines are now only used in this structure it's a little redundant to give them symbolic names, so the above could equally well be: static const struct rockchip_pwm_data pwm_data_v1 = { .regs = { .duty = 0x04, .period = 0x08, .cntr = 0x00, .ctrl = 0x0c, }, ... }; > + .prescaler = PRESCALER, Similarly for the prescaler value, it can now simply be 2 here. > + .set_enable = rockchip_pwm_set_enable_v1, > +}; > + > +static const struct rockchip_pwm_data pwm_data_v2 = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, > + .prescaler = PRESCALER-1, And 1 here. > + .set_enable = rockchip_pwm_set_enable_v2, > +}; > + > +static const struct rockchip_pwm_data pwm_data_vop = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CTRL, > + .regs.ctrl = PWM_CNTR, > + .prescaler = PRESCALER-1, And 1 here. > + .set_enable = rockchip_pwm_set_enable_v2, > +}; No need for the double indirection. Thierry
Attachment:
pgp1YvzgrRyyV.pgp
Description: PGP signature