2021-06-16 08:55:08

by Jitao Shi

[permalink] [raw]
Subject: [PATCH v5 2/3] pwm: mtk_disp: implement atomic API .apply()

Switch the driver to support the .apply() method.

Signed-off-by: Jitao Shi <[email protected]>
---
drivers/pwm/pwm-mtk-disp.c | 69 ++++++++++----------------------------
1 file changed, 17 insertions(+), 52 deletions(-)

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 204d76beeb26..a4766e931b68 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -67,14 +67,25 @@ static void mtk_disp_pwm_update_bits(struct mtk_disp_pwm *mdp, u32 offset,
writel(value, address);
}

-static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
- int duty_ns, int period_ns)
+static int mtk_disp_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
{
struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
u32 clk_div, period, high_width, value;
u64 div, rate;
int err;

+ if (!state->enabled) {
+ mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, mdp->data->enable_mask,
+ 0x0);
+ if (mdp->enabled) {
+ clk_disable_unprepare(mdp->clk_mm);
+ clk_disable_unprepare(mdp->clk_main);
+ }
+ mdp->enabled = false;
+ return 0;
+ }
+
if (!mdp->enabled) {
err = clk_prepare_enable(mdp->clk_main);
if (err < 0) {
@@ -102,7 +113,7 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
* high_width = (PWM_CLK_RATE * duty_ns) / (10^9 * (clk_div + 1))
*/
rate = clk_get_rate(mdp->clk_main);
- clk_div = div_u64(rate * period_ns, NSEC_PER_SEC) >>
+ clk_div = div_u64(rate * state->period, NSEC_PER_SEC) >>
PWM_PERIOD_BIT_WIDTH;
if (clk_div > PWM_CLKDIV_MAX) {
if (!mdp->enabled) {
@@ -112,11 +123,11 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
return -EINVAL;
}
div = NSEC_PER_SEC * (clk_div + 1);
- period = div64_u64(rate * period_ns, div);
+ period = div64_u64(rate * state->period, div);
if (period > 0)
period--;

- high_width = div64_u64(rate * duty_ns, div);
+ high_width = div64_u64(rate * state->duty_cycle, div);
value = period | (high_width << PWM_HIGH_WIDTH_SHIFT);

mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
@@ -141,36 +152,6 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
mdp->data->con0_sel,
mdp->data->con0_sel);
}
-
- if (!mdp->enabled) {
- clk_disable_unprepare(mdp->clk_mm);
- clk_disable_unprepare(mdp->clk_main);
- }
-
- return 0;
-}
-
-static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
- struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
- int err;
-
- if (!mdp->enabled) {
- err = clk_prepare_enable(mdp->clk_main);
- if (err < 0) {
- dev_err(chip->dev, "Can't enable mdp->clk_main: %pe\n",
- ERR_PTR(err));
- return err;
- }
- err = clk_prepare_enable(mdp->clk_mm);
- if (err < 0) {
- dev_err(chip->dev, "Can't enable mdp->clk_mm: %pe\n",
- ERR_PTR(err));
- clk_disable_unprepare(mdp->clk_main);
- return err;
- }
- }
-
mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, mdp->data->enable_mask,
mdp->data->enable_mask);
mdp->enabled = true;
@@ -178,24 +159,8 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
return 0;
}

-static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
- struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
-
- mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, mdp->data->enable_mask,
- 0x0);
-
- if (mdp->enabled) {
- clk_disable_unprepare(mdp->clk_mm);
- clk_disable_unprepare(mdp->clk_main);
- }
- mdp->enabled = false;
-}
-
static const struct pwm_ops mtk_disp_pwm_ops = {
- .config = mtk_disp_pwm_config,
- .enable = mtk_disp_pwm_enable,
- .disable = mtk_disp_pwm_disable,
+ .apply = mtk_disp_pwm_apply,
.owner = THIS_MODULE,
};

--
2.25.1


2021-06-25 17:53:49

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] pwm: mtk_disp: implement atomic API .apply()

On Wed, Jun 16, 2021 at 04:52:23PM +0800, Jitao Shi wrote:
> Switch the driver to support the .apply() method.
>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/pwm/pwm-mtk-disp.c | 69 ++++++++++----------------------------
> 1 file changed, 17 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index 204d76beeb26..a4766e931b68 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -67,14 +67,25 @@ static void mtk_disp_pwm_update_bits(struct mtk_disp_pwm *mdp, u32 offset,
> writel(value, address);
> }
>
> -static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> - int duty_ns, int period_ns)
> +static int mtk_disp_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
> {
> struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> u32 clk_div, period, high_width, value;
> u64 div, rate;
> int err;
>

Here something like:

if (state->polarity != PWM_POLARITY_NORMAL)
return -EINVAL;

is missing.

> + if (!state->enabled) {
> + mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, mdp->data->enable_mask,
> + 0x0);
> + if (mdp->enabled) {
> + clk_disable_unprepare(mdp->clk_mm);
> + clk_disable_unprepare(mdp->clk_main);
> + }
> + mdp->enabled = false;
> + return 0;
> + }
> +
> if (!mdp->enabled) {
> err = clk_prepare_enable(mdp->clk_main);
> if (err < 0) {
> @@ -102,7 +113,7 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> * high_width = (PWM_CLK_RATE * duty_ns) / (10^9 * (clk_div + 1))
> */
> rate = clk_get_rate(mdp->clk_main);
> - clk_div = div_u64(rate * period_ns, NSEC_PER_SEC) >>
> + clk_div = div_u64(rate * state->period, NSEC_PER_SEC) >>

This multiplication might overflow. But maybe it's sensible to address
this in a separate commit to keep things simple.

> PWM_PERIOD_BIT_WIDTH;
> if (clk_div > PWM_CLKDIV_MAX) {
> if (!mdp->enabled) {
> @@ -112,11 +123,11 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> return -EINVAL;
> }
> div = NSEC_PER_SEC * (clk_div + 1);
> - period = div64_u64(rate * period_ns, div);
> + period = div64_u64(rate * state->period, div);

ditto.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (2.49 kB)
signature.asc (499.00 B)
Download all attachments