| Message ID | 1406097521-6457-3-git-send-email-caesar.wang@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Caesar. Am Mittwoch, 23. Juli 2014, 14:38:41 schrieb Caesar Wang: > This patch added to support the PWM controller found on > RK3288 SoC. > > Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com> > --- > drivers/pwm/pwm-rockchip.c | 141 > +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 122 > insertions(+), 19 deletions(-) > > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eec2145..8d72a98 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@gmail.com> > + * Copyright (C) 2014 Caesar Wang <caesar.wang@rock-chips.com> you might want to check who actually holds the copyright for your contributions, I guess a "Copyright (C) 2014 Rockchip"-something would be more appropriate? > * > * 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,89 @@ > > #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 void rockchip_pwm_set_enable_vop(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); > +} not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and rockchip_pwm_set_enable_vop differ at all? If they don't differ, I guess pwm_data_vop should just use rockchip_pwm_set_enable_v2 instead of duplicating it. Heiko > + > static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, int duty_ns, int period_ns) > { > @@ -52,20 +126,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 +150,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 +163,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 +176,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, > + .prescaler = PRESCALER, > + .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, > + .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, > + .set_enable = rockchip_pwm_set_enable_vop, > +}; > + > +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,vop-pwm", .data = &pwm_data_vop}, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > + > static int rockchip_pwm_probe(struct platform_device *pdev) > { > + const struct of_device_id *id; > struct rockchip_pwm_chip *pc; > struct resource *r; > int ret; > > + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); > + if (!id) > + return -EINVAL; > + > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > if (!pc) > return -ENOMEM; > @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > > platform_set_drvdata(pdev, pc); > > + pc->data = id->data; > pc->chip.dev = &pdev->dev; > pc->chip.ops = &rockchip_pwm_ops; > pc->chip.base = -1; > @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct platform_device > *pdev) return pwmchip_remove(&pc->chip); > } > > -static const struct of_device_id rockchip_pwm_dt_ids[] = { > - { .compatible = "rockchip,rk2928-pwm" }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > - > static struct platform_driver rockchip_pwm_driver = { > .driver = { > .name = "rockchip-pwm",
Hi Heiko & thierry, Thank you for your suggestion. ? 2014?07?24? 00:01, Heiko Stübner ??: > Hi Caesar. > > Am Mittwoch, 23. Juli 2014, 14:38:41 schrieb Caesar Wang: >> This patch added to support the PWM controller found on >> RK3288 SoC. >> >> Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com> >> --- >> drivers/pwm/pwm-rockchip.c | 141 >> +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 122 >> insertions(+), 19 deletions(-) >> >> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c >> index eec2145..8d72a98 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@gmail.com> >> + * Copyright (C) 2014 Caesar Wang <caesar.wang@rock-chips.com> > you might want to check who actually holds the copyright for your > contributions, I guess a "Copyright (C) 2014 Rockchip"-something would be more > appropriate? > Yes,you are right. >> * >> * 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,89 @@ >> >> #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 void rockchip_pwm_set_enable_vop(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); >> +} > not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and > rockchip_pwm_set_enable_vop differ at all? > > If they don't differ, I guess pwm_data_vop should just use > rockchip_pwm_set_enable_v2 instead of duplicating it. > > > Heiko Yes, the rockchip_pwm_set_enable_v1 & v2 & vop is similar. So my v2 patch use "u32 enable_conf" instead of it . +struct rockchip_pwm_data { > + ......... > + u32 enable_conf; > +}; The thierry has suggested it [1] in my v2 patch: For this I think it would be more readable to provide function pointers rather than a variable. That is: struct rockchip_pwm_data { ... int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm); int (*disable)(struct pwm_chip *chip, struct pwm_device *pwm); }; Then you can implement these for each variant of the chip and call them from the common rockchip_pwm_enable(), somewhat like this. Perhaps,thierry's suggestion I got it wrong. Hi thierry& Heiko :-) Maybe,could you suggest solve it reasonable? thanks. [1]: https://lkml.org/lkml/2014/7/21/113 >> + >> static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device >> *pwm, int duty_ns, int period_ns) >> { >> @@ -52,20 +126,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 +150,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 +163,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 +176,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, >> + .prescaler = PRESCALER, >> + .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, >> + .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, >> + .set_enable = rockchip_pwm_set_enable_vop, >> +}; >> + >> +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,vop-pwm", .data = &pwm_data_vop}, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); >> + >> static int rockchip_pwm_probe(struct platform_device *pdev) >> { >> + const struct of_device_id *id; >> struct rockchip_pwm_chip *pc; >> struct resource *r; >> int ret; >> >> + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); >> + if (!id) >> + return -EINVAL; >> + >> pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); >> if (!pc) >> return -ENOMEM; >> @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device >> *pdev) >> >> platform_set_drvdata(pdev, pc); >> >> + pc->data = id->data; >> pc->chip.dev = &pdev->dev; >> pc->chip.ops = &rockchip_pwm_ops; >> pc->chip.base = -1; >> @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct platform_device >> *pdev) return pwmchip_remove(&pc->chip); >> } >> >> -static const struct of_device_id rockchip_pwm_dt_ids[] = { >> - { .compatible = "rockchip,rk2928-pwm" }, >> - { /* sentinel */ } >> -}; >> -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); >> - >> static struct platform_driver rockchip_pwm_driver = { >> .driver = { >> .name = "rockchip-pwm", > > >
Hi caesar. Am Donnerstag, 24. Juli 2014, 10:13:55 schrieb caesar: > >> +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 void rockchip_pwm_set_enable_vop(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); > >> +} > > > > not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and > > rockchip_pwm_set_enable_vop differ at all? > > > > If they don't differ, I guess pwm_data_vop should just use > > rockchip_pwm_set_enable_v2 instead of duplicating it. > > > > Yes, the rockchip_pwm_set_enable_v1 & v2 & vop is similar. > > So my v2 patch use "u32 enable_conf" instead of it . > +struct rockchip_pwm_data { > > > + ......... > > + u32 enable_conf; > > +}; > > The thierry has suggested it [1] in my v2 patch: > > For this I think it would be more readable to provide function pointers > rather than a variable. That is: > > struct rockchip_pwm_data { > ... > int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm); > int (*disable)(struct pwm_chip *chip, struct pwm_device *pwm); > }; > Then you can implement these for each variant of the chip and call them > from the common rockchip_pwm_enable(), somewhat like this. > > > Perhaps,thierry's suggestion I got it wrong. Using the function pointers like Thierry suggested looks nice, so no I don't think you got it wrong :-) What I meant was to simply reuse the existing function rockchip_pwm_set_enable_v2 when there is _no_ difference at all to rockchip_pwm_set_enable_vop, like 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, .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, .set_enable = rockchip_pwm_set_enable_v2, }; Heiko > > Hi thierry& Heiko :-) > Maybe,could you suggest solve it reasonable? thanks. > > [1]: https://lkml.org/lkml/2014/7/21/113 > > >> + > >> > >> static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device > >> > >> *pwm, int duty_ns, int period_ns) > >> > >> { > >> > >> @@ -52,20 +126,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 +150,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 +163,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 +176,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, > >> + .prescaler = PRESCALER, > >> + .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, > >> + .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, > >> + .set_enable = rockchip_pwm_set_enable_vop, > >> +}; > >> + > >> +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,vop-pwm", .data = &pwm_data_vop}, > >> + { /* sentinel */ } > >> +}; > >> +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > >> + > >> > >> static int rockchip_pwm_probe(struct platform_device *pdev) > >> { > >> > >> + const struct of_device_id *id; > >> > >> struct rockchip_pwm_chip *pc; > >> struct resource *r; > >> int ret; > >> > >> + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); > >> + if (!id) > >> + return -EINVAL; > >> + > >> > >> pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > >> if (!pc) > >> > >> return -ENOMEM; > >> > >> @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device > >> *pdev) > >> > >> platform_set_drvdata(pdev, pc); > >> > >> + pc->data = id->data; > >> > >> pc->chip.dev = &pdev->dev; > >> pc->chip.ops = &rockchip_pwm_ops; > >> pc->chip.base = -1; > >> > >> @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct > >> platform_device > >> *pdev) return pwmchip_remove(&pc->chip); > >> > >> } > >> > >> -static const struct of_device_id rockchip_pwm_dt_ids[] = { > >> - { .compatible = "rockchip,rk2928-pwm" }, > >> - { /* sentinel */ } > >> -}; > >> -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > >> - > >> > >> static struct platform_driver rockchip_pwm_driver = { > >> > >> .driver = { > >> > >> .name = "rockchip-pwm",
Hi Heiko, ? 2014?07?24? 16:05, Heiko Stübner ??: > Hi caesar. > > Am Donnerstag, 24. Juli 2014, 10:13:55 schrieb caesar: >>>> +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 void rockchip_pwm_set_enable_vop(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); >>>> +} >>> not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and >>> rockchip_pwm_set_enable_vop differ at all? >>> >>> If they don't differ, I guess pwm_data_vop should just use >>> rockchip_pwm_set_enable_v2 instead of duplicating it. >>> >> Yes, the rockchip_pwm_set_enable_v1 & v2 & vop is similar. >> >> So my v2 patch use "u32 enable_conf" instead of it . >> +struct rockchip_pwm_data { >> >> > + ......... >> > + u32 enable_conf; >> > +}; >> >> The thierry has suggested it [1] in my v2 patch: >> >> For this I think it would be more readable to provide function pointers >> rather than a variable. That is: >> >> struct rockchip_pwm_data { >> ... >> int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm); >> int (*disable)(struct pwm_chip *chip, struct pwm_device *pwm); >> }; >> Then you can implement these for each variant of the chip and call them >> from the common rockchip_pwm_enable(), somewhat like this. >> >> >> Perhaps,thierry's suggestion I got it wrong. > Using the function pointers like Thierry suggested looks nice, so no I don't > think you got it wrong :-) > > What I meant was to simply reuse the existing function > rockchip_pwm_set_enable_v2 when there is _no_ difference at all to > rockchip_pwm_set_enable_vop, like > > 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, > .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, > .set_enable = rockchip_pwm_set_enable_v2, > }; > > > Heiko :-( ok, I will fix this and the other issuses in v4, thanks. >> Hi thierry& Heiko :-) >> Maybe,could you suggest solve it reasonable? thanks. >> >> [1]: https://lkml.org/lkml/2014/7/21/113 >> >>>> + >>>> >>>> static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device >>>> >>>> *pwm, int duty_ns, int period_ns) >>>> >>>> { >>>> >>>> @@ -52,20 +126,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 +150,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 +163,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 +176,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, >>>> + .prescaler = PRESCALER, >>>> + .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, >>>> + .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, >>>> + .set_enable = rockchip_pwm_set_enable_vop, >>>> +}; >>>> + >>>> +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,vop-pwm", .data = &pwm_data_vop}, >>>> + { /* sentinel */ } >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); >>>> + >>>> >>>> static int rockchip_pwm_probe(struct platform_device *pdev) >>>> { >>>> >>>> + const struct of_device_id *id; >>>> >>>> struct rockchip_pwm_chip *pc; >>>> struct resource *r; >>>> int ret; >>>> >>>> + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); >>>> + if (!id) >>>> + return -EINVAL; >>>> + >>>> >>>> pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); >>>> if (!pc) >>>> >>>> return -ENOMEM; >>>> >>>> @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device >>>> *pdev) >>>> >>>> platform_set_drvdata(pdev, pc); >>>> >>>> + pc->data = id->data; >>>> >>>> pc->chip.dev = &pdev->dev; >>>> pc->chip.ops = &rockchip_pwm_ops; >>>> pc->chip.base = -1; >>>> >>>> @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct >>>> platform_device >>>> *pdev) return pwmchip_remove(&pc->chip); >>>> >>>> } >>>> >>>> -static const struct of_device_id rockchip_pwm_dt_ids[] = { >>>> - { .compatible = "rockchip,rk2928-pwm" }, >>>> - { /* sentinel */ } >>>> -}; >>>> -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); >>>> - >>>> >>>> static struct platform_driver rockchip_pwm_driver = { >>>> >>>> .driver = { >>>> >>>> .name = "rockchip-pwm", > > >
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c index eec2145..8d72a98 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@gmail.com> + * Copyright (C) 2014 Caesar Wang <caesar.wang@rock-chips.com> * * 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,89 @@ #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 void rockchip_pwm_set_enable_vop(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 +126,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 +150,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 +163,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 +176,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, + .prescaler = PRESCALER, + .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, + .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, + .set_enable = rockchip_pwm_set_enable_vop, +}; + +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,vop-pwm", .data = &pwm_data_vop}, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); + static int rockchip_pwm_probe(struct platform_device *pdev) { + const struct of_device_id *id; struct rockchip_pwm_chip *pc; struct resource *r; int ret; + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); + if (!id) + return -EINVAL; + pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); if (!pc) return -ENOMEM; @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pc); + pc->data = id->data; pc->chip.dev = &pdev->dev; pc->chip.ops = &rockchip_pwm_ops; pc->chip.base = -1; @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct platform_device *pdev) return pwmchip_remove(&pc->chip); } -static const struct of_device_id rockchip_pwm_dt_ids[] = { - { .compatible = "rockchip,rk2928-pwm" }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); - static struct platform_driver rockchip_pwm_driver = { .driver = { .name = "rockchip-pwm",
This patch added to support the PWM controller found on RK3288 SoC. Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com> --- drivers/pwm/pwm-rockchip.c | 141 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 122 insertions(+), 19 deletions(-)