There are two features of rk3328 pwm module.
- PWM APB and function clocks are different.
- Add pwm atomic hardware update
David Wu (5):
pwm: rockchip: Add APB and function both clocks support
pwm: rockchip: Remove the judge from return value of pwm_config
pwm: rockchip: Move the configuration of polarity from pwm_enable() to
pwm_config()
pwm: rockchip: Add atomic updated feature for rk3328
Arm64: dts: rockchip: Add pwm nodes for rk3328
.../devicetree/bindings/pwm/pwm-rockchip.txt | 9 +-
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 +++++
drivers/pwm/pwm-rockchip.c | 184 ++++++++++++++++-----
3 files changed, 197 insertions(+), 41 deletions(-)
--
1.9.1
New PWM module provides two individual clocks of APB clock
and function clock.
Signed-off-by: David Wu <[email protected]>
---
.../devicetree/bindings/pwm/pwm-rockchip.txt | 8 +++-
drivers/pwm/pwm-rockchip.c | 53 ++++++++++++++++++----
2 files changed, 51 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index b8be3d0..2350ef9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -6,7 +6,13 @@ Required properties:
"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
+ - clocks: See ../clock/clock-bindings.txt
+ - For older hardware (rk2928, rk3066, rk3188, rk3228, rk3288, rk3399):
+ - There is one clock that's used both to derive the functional clock
+ for the device and as the bus clock.
+ - For newer hardware (rk3328 and future socs): specified by name
+ - "pwm": This is used to derive the functional clock.
+ - "pclk": This is the APB bus clock.
- #pwm-cells: must be 2 (rk2928) or 3 (rk3288). See pwm.txt in this directory
for a description of the cell format.
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 744d561..617824c 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -33,6 +33,7 @@
struct rockchip_pwm_chip {
struct pwm_chip chip;
struct clk *clk;
+ struct clk *pclk;
const struct rockchip_pwm_data *data;
void __iomem *base;
};
@@ -145,7 +146,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
u64 tmp;
int ret;
- ret = clk_enable(pc->clk);
+ ret = clk_enable(pc->pclk);
if (ret)
return;
@@ -161,7 +162,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
pc->data->get_state(chip, pwm, state);
- clk_disable(pc->clk);
+ clk_disable(pc->pclk);
}
static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -224,7 +225,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
pwm_get_state(pwm, &curstate);
enabled = curstate.enabled;
- ret = clk_enable(pc->clk);
+ ret = clk_enable(pc->pclk);
if (ret)
return ret;
@@ -257,7 +258,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
rockchip_pwm_get_state(chip, pwm, state);
out:
- clk_disable(pc->clk);
+ clk_disable(pc->pclk);
return ret;
}
@@ -328,7 +329,7 @@ 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;
+ int ret, count;
id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
if (!id)
@@ -343,13 +344,38 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
if (IS_ERR(pc->base))
return PTR_ERR(pc->base);
- pc->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(pc->clk))
- return PTR_ERR(pc->clk);
+ pc->clk = devm_clk_get(&pdev->dev, "pwm");
+ count = of_property_count_strings(pdev->dev.of_node, "clock-names");
+ if (count == 2)
+ pc->pclk = devm_clk_get(&pdev->dev, "pclk");
+ else
+ pc->pclk = pc->clk;
+
+ if (IS_ERR(pc->clk)) {
+ ret = PTR_ERR(pc->clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Can't get bus clk: %d\n", ret);
+ return ret;
+ }
+
+ if (IS_ERR(pc->pclk)) {
+ ret = PTR_ERR(pc->pclk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Can't get APB clk: %d\n", ret);
+ return ret;
+ }
ret = clk_prepare_enable(pc->clk);
- if (ret)
+ if (ret) {
+ dev_err(&pdev->dev, "Can't prepare enable bus clk: %d\n", ret);
return ret;
+ }
+
+ ret = clk_prepare(pc->pclk);
+ if (ret) {
+ dev_err(&pdev->dev, "Can't prepare APB clk: %d\n", ret);
+ goto err_clk;
+ }
platform_set_drvdata(pdev, pc);
@@ -368,12 +394,20 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
if (ret < 0) {
clk_unprepare(pc->clk);
dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
+ goto err_pclk;
}
/* Keep the PWM clk enabled if the PWM appears to be up and running. */
if (!pwm_is_enabled(pc->chip.pwms))
clk_disable(pc->clk);
+ return 0;
+
+err_pclk:
+ clk_unprepare(pc->pclk);
+err_clk:
+ clk_disable_unprepare(pc->clk);
+
return ret;
}
@@ -395,6 +429,7 @@ static int rockchip_pwm_remove(struct platform_device *pdev)
if (pwm_is_enabled(pc->chip.pwms))
clk_disable(pc->clk);
+ clk_unprepare(pc->pclk);
clk_unprepare(pc->clk);
return pwmchip_remove(&pc->chip);
--
1.9.1
It seems the rockchip_pwm_config always returns the result 0,
so remove the judge.
Signed-off-by: David Wu <[email protected]>
---
drivers/pwm/pwm-rockchip.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 617824c..cd45f17 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -165,7 +165,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
}
-static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
@@ -188,8 +188,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
-
- return 0;
}
static int rockchip_pwm_enable(struct pwm_chip *chip,
@@ -236,13 +234,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
enabled = false;
}
- ret = rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
- if (ret) {
- if (enabled != curstate.enabled)
- rockchip_pwm_enable(chip, pwm, !enabled,
- state->polarity);
- goto out;
- }
+ rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
if (state->enabled != enabled) {
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
--
1.9.1
The rk3328 soc supports atomic update, we could lock the configuration
of period and duty at first, after unlock is configured, the period and
duty are effective at the same time.
If the polarity, period and duty need to be configured together,
the way for atomic update is "configure lock and old polarity" ->
"configure period and duty" -> "configure unlock and new polarity".
Signed-off-by: David Wu <[email protected]>
---
.../devicetree/bindings/pwm/pwm-rockchip.txt | 1 +
drivers/pwm/pwm-rockchip.c | 50 +++++++++++++++++++---
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 2350ef9..152c736 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible: should be "rockchip,<name>-pwm"
"rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
"rockchip,rk3288-pwm": found on RK3288 SoC
+ "rockchip,rk3328-pwm": found on RK3328 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
- reg: physical base address and length of the controller's registers
- clocks: See ../clock/clock-bindings.txt
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index eb630ff..d8801ae8 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -29,6 +29,7 @@
#define PWM_INACTIVE_POSITIVE (1 << 4)
#define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
#define PWM_OUTPUT_LEFT (0 << 5)
+#define PWM_LOCK_EN (1 << 6)
#define PWM_LP_DISABLE (0 << 8)
struct rockchip_pwm_chip {
@@ -50,6 +51,7 @@ struct rockchip_pwm_data {
struct rockchip_pwm_regs regs;
unsigned int prescaler;
bool supports_polarity;
+ bool supports_atomic_update;
const struct pwm_ops *ops;
void (*set_enable)(struct pwm_chip *chip,
@@ -201,6 +203,14 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip,
clk_rate = clk_get_rate(pc->clk);
/*
+ * Lock the period and duty of previous configuration, then
+ * change the duty and period, that would not be effective.
+ */
+ ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+ ctrl |= PWM_LOCK_EN;
+ writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
+
+ /*
* Since period and duty cycle registers have a width of 32
* bits, every possible input period can be obtained using the
* default prescaler value for all practical clock rate values.
@@ -222,6 +232,12 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip,
else
ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
+ /*
+ * Unlock and set polarity at the same time,
+ * the configuration of duty, period and polarity
+ * would be effective together at next period.
+ */
+ ctrl &= ~PWM_LOCK_EN;
writel(ctrl, pc->base + pc->data->regs.ctrl);
}
@@ -261,11 +277,18 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
if (ret)
return ret;
- if (state->polarity != curstate.polarity && enabled) {
- ret = rockchip_pwm_enable(chip, pwm, false);
- if (ret)
- goto out;
- enabled = false;
+ /*
+ * If the atomic update is supported, then go to the pwm config,
+ * no need to do this, it could ensure the atomic update for polarity
+ * changed.
+ */
+ if (pc->data->supports_atomic_update) {
+ if (state->polarity != curstate.polarity && enabled) {
+ ret = rockchip_pwm_enable(chip, pwm, false);
+ if (ret)
+ goto out;
+ enabled = false;
+ }
}
pc->data->pwm_config(chip, pwm, state->duty_cycle,
@@ -345,9 +368,26 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
.pwm_config = rockchip_pwm_config_v2,
};
+static const struct rockchip_pwm_data pwm_data_v3 = {
+ .regs = {
+ .duty = 0x08,
+ .period = 0x04,
+ .cntr = 0x00,
+ .ctrl = 0x0c,
+ },
+ .prescaler = 1,
+ .supports_polarity = true,
+ .supports_atomic_update = true,
+ .ops = &rockchip_pwm_ops_v2,
+ .set_enable = rockchip_pwm_set_enable_v2,
+ .get_state = rockchip_pwm_get_state_v2,
+ .pwm_config = rockchip_pwm_config_v2,
+};
+
static const struct of_device_id rockchip_pwm_dt_ids[] = {
{ .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1},
{ .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2},
+ { .compatible = "rockchip,rk3328-pwm", .data = &pwm_data_v3},
{ .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop},
{ /* sentinel */ }
};
--
1.9.1
It is usually possible to configure the polarity, cycle and duty all at once,
so that the polarity and cycle and duty should be binding together. Move it
into rockchip_pwm_config(), as well as prepared for the next atomic update
commit.
As the rk2928-pwm is different from rk3288-pwm, the rk2928-pwm doesn't support
the polarity, use the pwm_config_v1 and pwm_config_v2 to distinguish them.
Signed-off-by: David Wu <[email protected]>
---
drivers/pwm/pwm-rockchip.c | 77 ++++++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 20 deletions(-)
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index cd45f17..eb630ff 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -27,6 +27,7 @@
#define PWM_DUTY_NEGATIVE (0 << 3)
#define PWM_INACTIVE_NEGATIVE (0 << 4)
#define PWM_INACTIVE_POSITIVE (1 << 4)
+#define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
#define PWM_OUTPUT_LEFT (0 << 5)
#define PWM_LP_DISABLE (0 << 8)
@@ -52,10 +53,12 @@ struct rockchip_pwm_data {
const struct pwm_ops *ops;
void (*set_enable)(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable,
- enum pwm_polarity polarity);
+ struct pwm_device *pwm, bool enable);
void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state);
+ void (*pwm_config)(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns,
+ enum pwm_polarity polarity);
};
static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
@@ -64,8 +67,7 @@ static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
}
static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable,
- enum pwm_polarity polarity)
+ struct pwm_device *pwm, bool enable)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
@@ -95,19 +97,13 @@ static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
}
static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable,
- enum pwm_polarity polarity)
+ struct pwm_device *pwm, bool enable)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
PWM_CONTINUOUS;
u32 val;
- if (polarity == PWM_POLARITY_INVERSED)
- enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
- else
- enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
-
val = readl_relaxed(pc->base + pc->data->regs.ctrl);
if (enable)
@@ -165,12 +161,42 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
}
-static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
- int duty_ns, int period_ns)
+static void rockchip_pwm_config_v1(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ int duty_ns, int period_ns,
+ enum pwm_polarity polarity)
+{
+ struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ unsigned long period, duty;
+ u64 clk_rate, div;
+
+ clk_rate = clk_get_rate(pc->clk);
+
+ /*
+ * Since period and duty cycle registers have a width of 32
+ * bits, every possible input period can be obtained using the
+ * default prescaler value for all practical clock rate values.
+ */
+ div = clk_rate * period_ns;
+ period = DIV_ROUND_CLOSEST_ULL(div,
+ pc->data->prescaler * NSEC_PER_SEC);
+
+ div = clk_rate * duty_ns;
+ duty = DIV_ROUND_CLOSEST_ULL(div, pc->data->prescaler * NSEC_PER_SEC);
+
+ writel(period, pc->base + pc->data->regs.period);
+ writel(duty, pc->base + pc->data->regs.duty);
+}
+
+static void rockchip_pwm_config_v2(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ int duty_ns, int period_ns,
+ enum pwm_polarity polarity)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
+ u32 ctrl;
clk_rate = clk_get_rate(pc->clk);
@@ -188,12 +214,20 @@ static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
+
+ ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+ ctrl &= ~PWM_POLARITY_MASK;
+ if (polarity == PWM_POLARITY_INVERSED)
+ ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
+ else
+ ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
+
+ writel(ctrl, pc->base + pc->data->regs.ctrl);
}
static int rockchip_pwm_enable(struct pwm_chip *chip,
struct pwm_device *pwm,
- bool enable,
- enum pwm_polarity polarity)
+ bool enable)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
int ret;
@@ -204,7 +238,7 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return ret;
}
- pc->data->set_enable(chip, pwm, enable, polarity);
+ pc->data->set_enable(chip, pwm, enable);
if (!enable)
clk_disable(pc->clk);
@@ -228,17 +262,17 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
return ret;
if (state->polarity != curstate.polarity && enabled) {
- ret = rockchip_pwm_enable(chip, pwm, false, state->polarity);
+ ret = rockchip_pwm_enable(chip, pwm, false);
if (ret)
goto out;
enabled = false;
}
- rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
+ pc->data->pwm_config(chip, pwm, state->duty_cycle,
+ state->period, state->polarity);
if (state->enabled != enabled) {
- ret = rockchip_pwm_enable(chip, pwm, state->enabled,
- state->polarity);
+ ret = rockchip_pwm_enable(chip, pwm, state->enabled);
if (ret)
goto out;
}
@@ -278,6 +312,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
.ops = &rockchip_pwm_ops_v1,
.set_enable = rockchip_pwm_set_enable_v1,
.get_state = rockchip_pwm_get_state_v1,
+ .pwm_config = rockchip_pwm_config_v1,
};
static const struct rockchip_pwm_data pwm_data_v2 = {
@@ -292,6 +327,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
.ops = &rockchip_pwm_ops_v2,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
+ .pwm_config = rockchip_pwm_config_v2,
};
static const struct rockchip_pwm_data pwm_data_vop = {
@@ -306,6 +342,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
.ops = &rockchip_pwm_ops_v2,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
+ .pwm_config = rockchip_pwm_config_v2,
};
static const struct of_device_id rockchip_pwm_dt_ids[] = {
--
1.9.1
There are 4 pwm channels built in rk3328 soc, need to configure
the both APB clock and bus clock.
Signed-off-by: David Wu <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 ++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 29b3800..46f0847 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -310,6 +310,51 @@
interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
};
+ pwm0: pwm@ff1b0000 {
+ compatible = "rockchip,rk3328-pwm";
+ reg = <0x0 0xff1b0000 0x0 0x10>;
+ #pwm-cells = <3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pin>;
+ clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
+ clock-names = "pwm", "pclk";
+ status = "disabled";
+ };
+
+ pwm1: pwm@ff1b0010 {
+ compatible = "rockchip,rk3328-pwm";
+ reg = <0x0 0xff1b0010 0x0 0x10>;
+ #pwm-cells = <3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm1_pin>;
+ clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
+ clock-names = "pwm", "pclk";
+ status = "disabled";
+ };
+
+ pwm2: pwm@ff1b0020 {
+ compatible = "rockchip,rk3328-pwm";
+ reg = <0x0 0xff1b0020 0x0 0x10>;
+ #pwm-cells = <3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm2_pin>;
+ clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
+ clock-names = "pwm", "pclk";
+ status = "disabled";
+ };
+
+ pwm3: pwm@ff1b0030 {
+ compatible = "rockchip,rk3328-pwm";
+ reg = <0x0 0xff1b0030 0x0 0x10>;
+ interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+ #pwm-cells = <3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwmir_pin>;
+ clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
+ clock-names = "pwm", "pclk";
+ status = "disabled";
+ };
+
saradc: adc@ff280000 {
compatible = "rockchip,rk3328-saradc", "rockchip,rk3399-saradc";
reg = <0x0 0xff280000 0x0 0x100>;
--
1.9.1
On Thu, 29 Jun 2017 20:27:48 +0800
David Wu <[email protected]> wrote:
> It seems the rockchip_pwm_config always returns the result 0,
> so remove the judge.
>
> Signed-off-by: David Wu <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
> ---
> drivers/pwm/pwm-rockchip.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index 617824c..cd45f17 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -165,7 +165,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
> clk_disable(pc->pclk);
> }
>
> -static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> int duty_ns, int period_ns)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> @@ -188,8 +188,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> writel(period, pc->base + pc->data->regs.period);
> writel(duty, pc->base + pc->data->regs.duty);
> -
> - return 0;
> }
>
> static int rockchip_pwm_enable(struct pwm_chip *chip,
> @@ -236,13 +234,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> enabled = false;
> }
>
> - ret = rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
> - if (ret) {
> - if (enabled != curstate.enabled)
> - rockchip_pwm_enable(chip, pwm, !enabled,
> - state->polarity);
> - goto out;
> - }
> + rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
>
> if (state->enabled != enabled) {
> ret = rockchip_pwm_enable(chip, pwm, state->enabled,
On Thu, 29 Jun 2017 20:27:49 +0800
David Wu <[email protected]> wrote:
> It is usually possible to configure the polarity, cycle and duty all at once,
> so that the polarity and cycle and duty should be binding together. Move it
> into rockchip_pwm_config(), as well as prepared for the next atomic update
> commit.
>
> As the rk2928-pwm is different from rk3288-pwm, the rk2928-pwm doesn't support
> the polarity, use the pwm_config_v1 and pwm_config_v2 to distinguish them.
>
> Signed-off-by: David Wu <[email protected]>
> ---
> drivers/pwm/pwm-rockchip.c | 77 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 57 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index cd45f17..eb630ff 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -27,6 +27,7 @@
> #define PWM_DUTY_NEGATIVE (0 << 3)
> #define PWM_INACTIVE_NEGATIVE (0 << 4)
> #define PWM_INACTIVE_POSITIVE (1 << 4)
> +#define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
> #define PWM_OUTPUT_LEFT (0 << 5)
> #define PWM_LP_DISABLE (0 << 8)
>
> @@ -52,10 +53,12 @@ struct rockchip_pwm_data {
> const struct pwm_ops *ops;
>
> void (*set_enable)(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity);
> + struct pwm_device *pwm, bool enable);
> void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
> struct pwm_state *state);
> + void (*pwm_config)(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity);
> };
Hm, maybe it's time to drop these custom hooks and implement
pwm_apply_v1 and pwm_apply_v2 instead.
>
> static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
> @@ -64,8 +67,7 @@ static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
> }
>
> static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity)
> + struct pwm_device *pwm, bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
> @@ -95,19 +97,13 @@ static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
> }
>
> static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
> - struct pwm_device *pwm, bool enable,
> - enum pwm_polarity polarity)
> + struct pwm_device *pwm, bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
> PWM_CONTINUOUS;
> u32 val;
>
> - if (polarity == PWM_POLARITY_INVERSED)
> - enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
> - else
> - enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
> -
> val = readl_relaxed(pc->base + pc->data->regs.ctrl);
>
> if (enable)
> @@ -165,12 +161,42 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
> clk_disable(pc->pclk);
> }
>
> -static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> - int duty_ns, int period_ns)
> +static void rockchip_pwm_config_v1(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity)
> +{
> + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> + unsigned long period, duty;
> + u64 clk_rate, div;
> +
> + clk_rate = clk_get_rate(pc->clk);
> +
> + /*
> + * Since period and duty cycle registers have a width of 32
> + * bits, every possible input period can be obtained using the
> + * default prescaler value for all practical clock rate values.
> + */
> + div = clk_rate * period_ns;
> + period = DIV_ROUND_CLOSEST_ULL(div,
> + pc->data->prescaler * NSEC_PER_SEC);
> +
> + div = clk_rate * duty_ns;
> + duty = DIV_ROUND_CLOSEST_ULL(div, pc->data->prescaler * NSEC_PER_SEC);
> +
> + writel(period, pc->base + pc->data->regs.period);
> + writel(duty, pc->base + pc->data->regs.duty);
> +}
> +
> +static void rockchip_pwm_config_v2(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + int duty_ns, int period_ns,
> + enum pwm_polarity polarity)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> unsigned long period, duty;
> u64 clk_rate, div;
> + u32 ctrl;
>
> clk_rate = clk_get_rate(pc->clk);
>
> @@ -188,12 +214,20 @@ static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> writel(period, pc->base + pc->data->regs.period);
> writel(duty, pc->base + pc->data->regs.duty);
> +
> + ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
> + ctrl &= ~PWM_POLARITY_MASK;
> + if (polarity == PWM_POLARITY_INVERSED)
> + ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
> + else
> + ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
> +
> + writel(ctrl, pc->base + pc->data->regs.ctrl);
> }
>
> static int rockchip_pwm_enable(struct pwm_chip *chip,
> struct pwm_device *pwm,
> - bool enable,
> - enum pwm_polarity polarity)
> + bool enable)
> {
> struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
> int ret;
> @@ -204,7 +238,7 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
> return ret;
> }
>
> - pc->data->set_enable(chip, pwm, enable, polarity);
> + pc->data->set_enable(chip, pwm, enable);
>
> if (!enable)
> clk_disable(pc->clk);
> @@ -228,17 +262,17 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> return ret;
>
> if (state->polarity != curstate.polarity && enabled) {
> - ret = rockchip_pwm_enable(chip, pwm, false, state->polarity);
> + ret = rockchip_pwm_enable(chip, pwm, false);
> if (ret)
> goto out;
> enabled = false;
> }
>
> - rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
> + pc->data->pwm_config(chip, pwm, state->duty_cycle,
> + state->period, state->polarity);
>
> if (state->enabled != enabled) {
> - ret = rockchip_pwm_enable(chip, pwm, state->enabled,
> - state->polarity);
> + ret = rockchip_pwm_enable(chip, pwm, state->enabled);
> if (ret)
> goto out;
> }
> @@ -278,6 +312,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v1,
> .set_enable = rockchip_pwm_set_enable_v1,
> .get_state = rockchip_pwm_get_state_v1,
> + .pwm_config = rockchip_pwm_config_v1,
> };
>
> static const struct rockchip_pwm_data pwm_data_v2 = {
> @@ -292,6 +327,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v2,
> .set_enable = rockchip_pwm_set_enable_v2,
> .get_state = rockchip_pwm_get_state_v2,
> + .pwm_config = rockchip_pwm_config_v2,
> };
>
> static const struct rockchip_pwm_data pwm_data_vop = {
> @@ -306,6 +342,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .ops = &rockchip_pwm_ops_v2,
> .set_enable = rockchip_pwm_set_enable_v2,
> .get_state = rockchip_pwm_get_state_v2,
> + .pwm_config = rockchip_pwm_config_v2,
> };
>
> static const struct of_device_id rockchip_pwm_dt_ids[] = {
On Thu, 29 Jun 2017 20:27:50 +0800
David Wu <[email protected]> wrote:
> The rk3328 soc supports atomic update, we could lock the configuration
> of period and duty at first, after unlock is configured, the period and
> duty are effective at the same time.
>
> If the polarity, period and duty need to be configured together,
> the way for atomic update is "configure lock and old polarity" ->
> "configure period and duty" -> "configure unlock and new polarity".
>
> Signed-off-by: David Wu <[email protected]>
> ---
> .../devicetree/bindings/pwm/pwm-rockchip.txt | 1 +
> drivers/pwm/pwm-rockchip.c | 50 +++++++++++++++++++---
> 2 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
> index 2350ef9..152c736 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
> @@ -4,6 +4,7 @@ Required properties:
> - compatible: should be "rockchip,<name>-pwm"
> "rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
> "rockchip,rk3288-pwm": found on RK3288 SoC
> + "rockchip,rk3328-pwm": found on RK3328 SoC
> "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
> - reg: physical base address and length of the controller's registers
> - clocks: See ../clock/clock-bindings.txt
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index eb630ff..d8801ae8 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -29,6 +29,7 @@
> #define PWM_INACTIVE_POSITIVE (1 << 4)
> #define PWM_POLARITY_MASK (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
> #define PWM_OUTPUT_LEFT (0 << 5)
> +#define PWM_LOCK_EN (1 << 6)
> #define PWM_LP_DISABLE (0 << 8)
>
> struct rockchip_pwm_chip {
> @@ -50,6 +51,7 @@ struct rockchip_pwm_data {
> struct rockchip_pwm_regs regs;
> unsigned int prescaler;
> bool supports_polarity;
> + bool supports_atomic_update;
Yet another customization. Don't you think we can extract common parts,
expose them as helpers and then have 3 different pwm_ops (with 3
different ->apply() implementation), one for each IP revision.
> const struct pwm_ops *ops;
>
> void (*set_enable)(struct pwm_chip *chip,
> @@ -201,6 +203,14 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip,
> clk_rate = clk_get_rate(pc->clk);
>
> /*
> + * Lock the period and duty of previous configuration, then
> + * change the duty and period, that would not be effective.
> + */
> + ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
> + ctrl |= PWM_LOCK_EN;
> + writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
> +
> + /*
> * Since period and duty cycle registers have a width of 32
> * bits, every possible input period can be obtained using the
> * default prescaler value for all practical clock rate values.
> @@ -222,6 +232,12 @@ static void rockchip_pwm_config_v2(struct pwm_chip *chip,
> else
> ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
>
> + /*
> + * Unlock and set polarity at the same time,
> + * the configuration of duty, period and polarity
> + * would be effective together at next period.
> + */
> + ctrl &= ~PWM_LOCK_EN;
> writel(ctrl, pc->base + pc->data->regs.ctrl);
> }
>
> @@ -261,11 +277,18 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> if (ret)
> return ret;
>
> - if (state->polarity != curstate.polarity && enabled) {
> - ret = rockchip_pwm_enable(chip, pwm, false);
> - if (ret)
> - goto out;
> - enabled = false;
> + /*
> + * If the atomic update is supported, then go to the pwm config,
> + * no need to do this, it could ensure the atomic update for polarity
> + * changed.
> + */
> + if (pc->data->supports_atomic_update) {
> + if (state->polarity != curstate.polarity && enabled) {
> + ret = rockchip_pwm_enable(chip, pwm, false);
> + if (ret)
> + goto out;
> + enabled = false;
> + }
> }
>
> pc->data->pwm_config(chip, pwm, state->duty_cycle,
> @@ -345,9 +368,26 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> .pwm_config = rockchip_pwm_config_v2,
> };
>
> +static const struct rockchip_pwm_data pwm_data_v3 = {
> + .regs = {
> + .duty = 0x08,
> + .period = 0x04,
> + .cntr = 0x00,
> + .ctrl = 0x0c,
> + },
> + .prescaler = 1,
> + .supports_polarity = true,
> + .supports_atomic_update = true,
> + .ops = &rockchip_pwm_ops_v2,
> + .set_enable = rockchip_pwm_set_enable_v2,
> + .get_state = rockchip_pwm_get_state_v2,
> + .pwm_config = rockchip_pwm_config_v2,
> +};
> +
> static const struct of_device_id rockchip_pwm_dt_ids[] = {
> { .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1},
> { .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2},
> + { .compatible = "rockchip,rk3328-pwm", .data = &pwm_data_v3},
> { .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop},
> { /* sentinel */ }
> };
Hi Boris,
?? 2017/7/4 2:36, Boris Brezillon д??:
> Hm, maybe it's time to drop these custom hooks and implement
> pwm_apply_v1 and pwm_apply_v2 instead.
Okay, drop the enable and config hooks, only use the apply hook to
instead them.
Hi Boris,
?? 2017/7/4 2:39, Boris Brezillon д??:
> Yet another customization. Don't you think we can extract common parts,
> expose them as helpers and then have 3 different pwm_ops (with 3
> different ->apply() implementation), one for each IP revision.
Sounds reasonable.I will try to implement 3 different apply().
On Thu, Jun 29, 2017 at 08:27:47PM +0800, David Wu wrote:
> New PWM module provides two individual clocks of APB clock
> and function clock.
>
> Signed-off-by: David Wu <[email protected]>
> ---
> .../devicetree/bindings/pwm/pwm-rockchip.txt | 8 +++-
Acked-by: Rob Herring <[email protected]>
> drivers/pwm/pwm-rockchip.c | 53 ++++++++++++++++++----
> 2 files changed, 51 insertions(+), 10 deletions(-)
On Thu, Jun 29, 2017 at 08:27:50PM +0800, David Wu wrote:
> The rk3328 soc supports atomic update, we could lock the configuration
> of period and duty at first, after unlock is configured, the period and
> duty are effective at the same time.
>
> If the polarity, period and duty need to be configured together,
> the way for atomic update is "configure lock and old polarity" ->
> "configure period and duty" -> "configure unlock and new polarity".
>
> Signed-off-by: David Wu <[email protected]>
> ---
> .../devicetree/bindings/pwm/pwm-rockchip.txt | 1 +
The subject is a bit misleading as you are adding support for a new
version of the IP. Otherwise,
Acked-by: Rob Herring <[email protected]>
> drivers/pwm/pwm-rockchip.c | 50 +++++++++++++++++++---
> 2 files changed, 46 insertions(+), 5 deletions(-)