2013-03-31 14:59:55

by Axel Lin

[permalink] [raw]
Subject: [PATCH 1/3] pwm: imx: Remove enabled field from struct imx_chip

We can test PWMF_ENABLED bit to know if pwm is enabled or not.
Thus remove enabled field from struct imx_chip.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pwm/pwm-imx.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 3f5677b..ec28798 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -43,7 +43,6 @@ struct imx_chip {
struct clk *clk_per;
struct clk *clk_ipg;

- int enabled;
void __iomem *mmio_base;

struct pwm_chip chip;
@@ -135,7 +134,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH;

- if (imx->enabled)
+ if (test_bit(PWMF_ENABLED, &pwm->flags))
cr |= MX3_PWMCR_EN;

writel(cr, imx->mmio_base + MX3_PWMCR);
@@ -186,8 +185,6 @@ static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)

imx->set_enable(chip, true);

- imx->enabled = 1;
-
return 0;
}

@@ -198,7 +195,6 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
imx->set_enable(chip, false);

clk_disable_unprepare(imx->clk_per);
- imx->enabled = 0;
}

static struct pwm_ops imx_pwm_ops = {
--
1.7.10.4



2013-03-31 15:01:18

by Axel Lin

[permalink] [raw]
Subject: [PATCH 2/3] pwm: puv3: Remove unused enabled filed from struct puv3_pwm_chip

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pwm/pwm-puv3.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/pwm/pwm-puv3.c b/drivers/pwm/pwm-puv3.c
index db964e6..d1eb499 100644
--- a/drivers/pwm/pwm-puv3.c
+++ b/drivers/pwm/pwm-puv3.c
@@ -27,7 +27,6 @@ struct puv3_pwm_chip {
struct pwm_chip chip;
void __iomem *base;
struct clk *clk;
- bool enabled;
};

static inline struct puv3_pwm_chip *to_puv3(struct pwm_chip *chip)
--
1.7.10.4


2013-03-31 15:04:38

by Axel Lin

[permalink] [raw]
Subject: [PATCH 3/3] pwm: pxa: Remove clk_enabled field from struct pxa_pwm_chip

clk_enable/clk_disable maintain an enable_count, clk_prepare and clk_unprepare
also maintain a prepare_count. These APIs will do prepare/enable when the first
user calling these APIs, and do disable/unprepare when the corresponding counter
reach 0. Thus We don't need to maintain a clk_enabled counter here.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pwm/pwm-pxa.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index 20370e6..b789882 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -48,7 +48,6 @@ struct pxa_pwm_chip {
struct device *dev;

struct clk *clk;
- int clk_enabled;
void __iomem *mmio_base;
};

@@ -108,24 +107,15 @@ static int pxa_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
static int pxa_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct pxa_pwm_chip *pc = to_pxa_pwm_chip(chip);
- int rc = 0;

- if (!pc->clk_enabled) {
- rc = clk_prepare_enable(pc->clk);
- if (!rc)
- pc->clk_enabled++;
- }
- return rc;
+ return clk_prepare_enable(pc->clk);
}

static void pxa_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct pxa_pwm_chip *pc = to_pxa_pwm_chip(chip);

- if (pc->clk_enabled) {
- clk_disable_unprepare(pc->clk);
- pc->clk_enabled--;
- }
+ clk_disable_unprepare(pc->clk);
}

static struct pwm_ops pxa_pwm_ops = {
@@ -152,8 +142,6 @@ static int pwm_probe(struct platform_device *pdev)
if (IS_ERR(pwm->clk))
return PTR_ERR(pwm->clk);

- pwm->clk_enabled = 0;
-
pwm->chip.dev = &pdev->dev;
pwm->chip.ops = &pxa_pwm_ops;
pwm->chip.base = -1;
--
1.7.10.4


2013-04-01 07:25:04

by Eric Miao

[permalink] [raw]
Subject: Re: [PATCH 3/3] pwm: pxa: Remove clk_enabled field from struct pxa_pwm_chip

On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <[email protected]> wrote:
> clk_enable/clk_disable maintain an enable_count, clk_prepare and clk_unprepare
> also maintain a prepare_count. These APIs will do prepare/enable when the first
> user calling these APIs, and do disable/unprepare when the corresponding counter
> reach 0. Thus We don't need to maintain a clk_enabled counter here.

The original intention is to keep a paired clk enable counter no matter
how the user calls pwm_enable()/pwm_disable() in pair or not, if that's
no longer the case.

>
> Signed-off-by: Axel Lin <[email protected]>
> ---
> drivers/pwm/pwm-pxa.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> index 20370e6..b789882 100644
> --- a/drivers/pwm/pwm-pxa.c
> +++ b/drivers/pwm/pwm-pxa.c
> @@ -48,7 +48,6 @@ struct pxa_pwm_chip {
> struct device *dev;
>
> struct clk *clk;
> - int clk_enabled;
> void __iomem *mmio_base;
> };
>
> @@ -108,24 +107,15 @@ static int pxa_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> static int pxa_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
> struct pxa_pwm_chip *pc = to_pxa_pwm_chip(chip);
> - int rc = 0;
>
> - if (!pc->clk_enabled) {
> - rc = clk_prepare_enable(pc->clk);
> - if (!rc)
> - pc->clk_enabled++;
> - }
> - return rc;
> + return clk_prepare_enable(pc->clk);
> }
>
> static void pxa_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
> struct pxa_pwm_chip *pc = to_pxa_pwm_chip(chip);
>
> - if (pc->clk_enabled) {
> - clk_disable_unprepare(pc->clk);
> - pc->clk_enabled--;
> - }
> + clk_disable_unprepare(pc->clk);
> }
>
> static struct pwm_ops pxa_pwm_ops = {
> @@ -152,8 +142,6 @@ static int pwm_probe(struct platform_device *pdev)
> if (IS_ERR(pwm->clk))
> return PTR_ERR(pwm->clk);
>
> - pwm->clk_enabled = 0;
> -
> pwm->chip.dev = &pdev->dev;
> pwm->chip.ops = &pxa_pwm_ops;
> pwm->chip.base = -1;
> --
> 1.7.10.4
>
>
>

2013-04-01 07:32:08

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 3/3] pwm: pxa: Remove clk_enabled field from struct pxa_pwm_chip

2013/4/1 Eric Miao <[email protected]>:
> On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <[email protected]> wrote:
>> clk_enable/clk_disable maintain an enable_count, clk_prepare and clk_unprepare
>> also maintain a prepare_count. These APIs will do prepare/enable when the first
>> user calling these APIs, and do disable/unprepare when the corresponding counter
>> reach 0. Thus We don't need to maintain a clk_enabled counter here.
>
> The original intention is to keep a paired clk enable counter no matter
> how the user calls pwm_enable()/pwm_disable() in pair or not, if that's
> no longer the case.

We don't need to worry that case:
In pwm core, both pwm_enable() and pwm_disable() will always check
PWMF_ENABLED flag.


/**
* pwm_enable() - start a PWM output toggling
* @pwm: PWM device
*/
int pwm_enable(struct pwm_device *pwm)
{
if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags))
return pwm->chip->ops->enable(pwm->chip, pwm);

return pwm ? 0 : -EINVAL;
}
EXPORT_SYMBOL_GPL(pwm_enable);

/**
* pwm_disable() - stop a PWM output toggling
* @pwm: PWM device
*/
void pwm_disable(struct pwm_device *pwm)
{
if (pwm && test_and_clear_bit(PWMF_ENABLED, &pwm->flags))
pwm->chip->ops->disable(pwm->chip, pwm);
}
EXPORT_SYMBOL_GPL(pwm_disable);

2013-04-01 07:35:39

by Eric Miao

[permalink] [raw]
Subject: Re: [PATCH 3/3] pwm: pxa: Remove clk_enabled field from struct pxa_pwm_chip

On Mon, Apr 1, 2013 at 3:32 PM, Axel Lin <[email protected]> wrote:
> 2013/4/1 Eric Miao <[email protected]>:
>> On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <[email protected]> wrote:
>>> clk_enable/clk_disable maintain an enable_count, clk_prepare and clk_unprepare
>>> also maintain a prepare_count. These APIs will do prepare/enable when the first
>>> user calling these APIs, and do disable/unprepare when the corresponding counter
>>> reach 0. Thus We don't need to maintain a clk_enabled counter here.
>>
>> The original intention is to keep a paired clk enable counter no matter
>> how the user calls pwm_enable()/pwm_disable() in pair or not, if that's
>> no longer the case.
>
> We don't need to worry that case:
> In pwm core, both pwm_enable() and pwm_disable() will always check
> PWMF_ENABLED flag.

That's good then, this part of the code was dated before the pwm core,
looks like this has been carefully handled. Have my ack on this one then:

Acked-by: Eric Miao <[email protected]>

>
>
> /**
> * pwm_enable() - start a PWM output toggling
> * @pwm: PWM device
> */
> int pwm_enable(struct pwm_device *pwm)
> {
> if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags))
> return pwm->chip->ops->enable(pwm->chip, pwm);
>
> return pwm ? 0 : -EINVAL;
> }
> EXPORT_SYMBOL_GPL(pwm_enable);
>
> /**
> * pwm_disable() - stop a PWM output toggling
> * @pwm: PWM device
> */
> void pwm_disable(struct pwm_device *pwm)
> {
> if (pwm && test_and_clear_bit(PWMF_ENABLED, &pwm->flags))
> pwm->chip->ops->disable(pwm->chip, pwm);
> }
> EXPORT_SYMBOL_GPL(pwm_disable);

2013-04-02 08:13:15

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 1/3] pwm: imx: Remove enabled field from struct imx_chip

On Sun, Mar 31, 2013 at 10:59:47PM +0800, Axel Lin wrote:
> We can test PWMF_ENABLED bit to know if pwm is enabled or not.
> Thus remove enabled field from struct imx_chip.
>
> Signed-off-by: Axel Lin <[email protected]>

Looks good.

Acked-by: Sascha Hauer <[email protected]>

Sascha

> ---
> drivers/pwm/pwm-imx.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 3f5677b..ec28798 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -43,7 +43,6 @@ struct imx_chip {
> struct clk *clk_per;
> struct clk *clk_ipg;
>
> - int enabled;
> void __iomem *mmio_base;
>
> struct pwm_chip chip;
> @@ -135,7 +134,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
> MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
> MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH;
>
> - if (imx->enabled)
> + if (test_bit(PWMF_ENABLED, &pwm->flags))
> cr |= MX3_PWMCR_EN;
>
> writel(cr, imx->mmio_base + MX3_PWMCR);
> @@ -186,8 +185,6 @@ static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>
> imx->set_enable(chip, true);
>
> - imx->enabled = 1;
> -
> return 0;
> }
>
> @@ -198,7 +195,6 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> imx->set_enable(chip, false);
>
> clk_disable_unprepare(imx->clk_per);
> - imx->enabled = 0;
> }
>
> static struct pwm_ops imx_pwm_ops = {
> --
> 1.7.10.4
>
>
>
>

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2013-04-02 09:38:03

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/3] pwm: imx: Remove enabled field from struct imx_chip

On Sun, Mar 31, 2013 at 10:59:47PM +0800, Axel Lin wrote:
> We can test PWMF_ENABLED bit to know if pwm is enabled or not.
> Thus remove enabled field from struct imx_chip.
>
> Signed-off-by: Axel Lin <[email protected]>
> ---
> drivers/pwm/pwm-imx.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)

All 3 patches applied, thanks.

Thierry


Attachments:
(No filename) (360.00 B)
(No filename) (836.00 B)
Download all attachments