* [PATCH v3 0/2] This series adds support for RK3288 SoC integrated PWM
@ 2014-07-23 6:38 ` Caesar Wang
0 siblings, 0 replies; 14+ messages in thread
From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw)
To: heiko, thierry.reding, b.galvani
Cc: linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq,
addy.ke, Caesar Wang
This patch will make applying on the top of Beniamino's submission,
the Beniamino's submission come from [1].
[1]:
https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git/log/?h=for-next
Beniamino's submission won't be used from genenation Rockchip SoCs.
So I have to add the new pwm for next genenation Rockchip SoCs.
Tested on RK3288 SDK board.
Changes in v3:
* address comments from Thierry Reding:
- fix PWM document deccribes.
- add a description for [PATCH v3 2/2].
- renamed the PWM registers
- Changed in rockchip_pwm_data struct
- remove the devm_ioremap(),fixed in lcdc driver.
Changes in v2:
* address comments from Beniamino Galvani:
- remove #include <linux/of_address.h>.
- of_iomap be removed,and devm_ioremap replace it.
- remove a line no be used.
Caesar Wang (2):
pwm: rockchip: document RK3288 SoC compatible
pwm: rockchip: Added to support for RK3288 SoC
.../devicetree/bindings/pwm/pwm-rockchip.txt | 5 +-
drivers/pwm/pwm-rockchip.c | 141 ++++++++++++++++++---
2 files changed, 126 insertions(+), 20 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v3 0/2] This series adds support for RK3288 SoC integrated PWM @ 2014-07-23 6:38 ` Caesar Wang 0 siblings, 0 replies; 14+ messages in thread From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw) To: linux-arm-kernel This patch will make applying on the top of Beniamino's submission, the Beniamino's submission come from [1]. [1]: https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git/log/?h=for-next Beniamino's submission won't be used from genenation Rockchip SoCs. So I have to add the new pwm for next genenation Rockchip SoCs. Tested on RK3288 SDK board. Changes in v3: * address comments from Thierry Reding: - fix PWM document deccribes. - add a description for [PATCH v3 2/2]. - renamed the PWM registers - Changed in rockchip_pwm_data struct - remove the devm_ioremap(),fixed in lcdc driver. Changes in v2: * address comments from Beniamino Galvani: - remove #include <linux/of_address.h>. - of_iomap be removed,and devm_ioremap replace it. - remove a line no be used. Caesar Wang (2): pwm: rockchip: document RK3288 SoC compatible pwm: rockchip: Added to support for RK3288 SoC .../devicetree/bindings/pwm/pwm-rockchip.txt | 5 +- drivers/pwm/pwm-rockchip.c | 141 ++++++++++++++++++--- 2 files changed, 126 insertions(+), 20 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] pwm: rockchip: document RK3288 SoC compatible 2014-07-23 6:38 ` Caesar Wang @ 2014-07-23 6:38 ` Caesar Wang -1 siblings, 0 replies; 14+ messages in thread From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw) To: heiko, thierry.reding, b.galvani Cc: linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke, Caesar Wang Document new compatible for PWM founding on RK3288 SoC Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com> --- Documentation/devicetree/bindings/pwm/pwm-rockchip.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt index 3182126..2b7c109 100644 --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt @@ -1,7 +1,10 @@ Rockchip PWM controller Required properties: - - compatible: should be "rockchip,rk2928-pwm" + - compatible: should be "rockchip,<name>-pwm" + "rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs + "rockchip,rk3288-pwm": found on RK3288 SoC + "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC - reg: physical base address and length of the controller's registers - clocks: phandle and clock specifier of the PWM reference clock - #pwm-cells: should be 2. See pwm.txt in this directory for a -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] pwm: rockchip: document RK3288 SoC compatible @ 2014-07-23 6:38 ` Caesar Wang 0 siblings, 0 replies; 14+ messages in thread From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw) To: linux-arm-kernel Document new compatible for PWM founding on RK3288 SoC Signed-off-by: Caesar Wang <caesar.wang@rock-chips.com> --- Documentation/devicetree/bindings/pwm/pwm-rockchip.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt index 3182126..2b7c109 100644 --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt @@ -1,7 +1,10 @@ Rockchip PWM controller Required properties: - - compatible: should be "rockchip,rk2928-pwm" + - compatible: should be "rockchip,<name>-pwm" + "rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs + "rockchip,rk3288-pwm": found on RK3288 SoC + "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC - reg: physical base address and length of the controller's registers - clocks: phandle and clock specifier of the PWM reference clock - #pwm-cells: should be 2. See pwm.txt in this directory for a -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC 2014-07-23 6:38 ` Caesar Wang @ 2014-07-23 6:38 ` Caesar Wang -1 siblings, 0 replies; 14+ messages in thread From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw) To: heiko, thierry.reding, b.galvani Cc: linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke, 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> * * 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", -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC @ 2014-07-23 6:38 ` Caesar Wang 0 siblings, 0 replies; 14+ messages in thread From: Caesar Wang @ 2014-07-23 6:38 UTC (permalink / raw) To: linux-arm-kernel 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> * * 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", -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC 2014-07-23 6:38 ` Caesar Wang @ 2014-07-23 16:01 ` Heiko Stübner -1 siblings, 0 replies; 14+ messages in thread From: Heiko Stübner @ 2014-07-23 16:01 UTC (permalink / raw) To: Caesar Wang Cc: thierry.reding, b.galvani, linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke 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", ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC @ 2014-07-23 16:01 ` Heiko Stübner 0 siblings, 0 replies; 14+ messages in thread From: Heiko Stübner @ 2014-07-23 16:01 UTC (permalink / raw) To: linux-arm-kernel 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", ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC 2014-07-23 16:01 ` Heiko Stübner @ 2014-07-24 2:13 ` caesar -1 siblings, 0 replies; 14+ messages in thread From: caesar @ 2014-07-24 2:13 UTC (permalink / raw) To: Heiko Stübner Cc: thierry.reding, b.galvani, linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke 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", > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC @ 2014-07-24 2:13 ` caesar 0 siblings, 0 replies; 14+ messages in thread From: caesar @ 2014-07-24 2:13 UTC (permalink / raw) To: linux-arm-kernel 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", > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC 2014-07-24 2:13 ` caesar @ 2014-07-24 8:05 ` Heiko Stübner -1 siblings, 0 replies; 14+ messages in thread From: Heiko Stübner @ 2014-07-24 8:05 UTC (permalink / raw) To: caesar Cc: thierry.reding, b.galvani, linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke 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", ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC @ 2014-07-24 8:05 ` Heiko Stübner 0 siblings, 0 replies; 14+ messages in thread From: Heiko Stübner @ 2014-07-24 8:05 UTC (permalink / raw) To: linux-arm-kernel 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", ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC 2014-07-24 8:05 ` Heiko Stübner @ 2014-07-24 9:29 ` caesar -1 siblings, 0 replies; 14+ messages in thread From: caesar @ 2014-07-24 9:29 UTC (permalink / raw) To: Heiko Stübner Cc: thierry.reding, b.galvani, linux-arm-kernel, linux-pwm, linux-kernel, cf, huangtao, hj, xjq, addy.ke 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", > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC @ 2014-07-24 9:29 ` caesar 0 siblings, 0 replies; 14+ messages in thread From: caesar @ 2014-07-24 9:29 UTC (permalink / raw) To: linux-arm-kernel 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", > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-07-24 9:30 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-23 6:38 [PATCH v3 0/2] This series adds support for RK3288 SoC integrated PWM Caesar Wang 2014-07-23 6:38 ` Caesar Wang 2014-07-23 6:38 ` [PATCH v3 1/2] pwm: rockchip: document RK3288 SoC compatible Caesar Wang 2014-07-23 6:38 ` Caesar Wang 2014-07-23 6:38 ` [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC Caesar Wang 2014-07-23 6:38 ` Caesar Wang 2014-07-23 16:01 ` Heiko Stübner 2014-07-23 16:01 ` Heiko Stübner 2014-07-24 2:13 ` caesar 2014-07-24 2:13 ` caesar 2014-07-24 8:05 ` Heiko Stübner 2014-07-24 8:05 ` Heiko Stübner 2014-07-24 9:29 ` caesar 2014-07-24 9:29 ` caesar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.