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

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

[Axel Lin]

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 <axel.lin@ingics.com>
---
 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

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

[Axel Lin]

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 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

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

[Axel Lin]

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 <axel.lin@ingics.com>
---
 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

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

[Eric Miao]

On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <axel.lin@ingics.com> 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 <axel.lin@ingics.com>
> ---
>  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
>
>
>

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

[Axel Lin]

2013/4/1 Eric Miao <eric.y.miao@gmail.com>:
> On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <axel.lin@ingics.com> 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);

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

[Eric Miao]

On Mon, Apr 1, 2013 at 3:32 PM, Axel Lin <axel.lin@ingics.com> wrote:
> 2013/4/1 Eric Miao <eric.y.miao@gmail.com>:
>> On Sun, Mar 31, 2013 at 11:04 PM, Axel Lin <axel.lin@ingics.com> 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 <eric.y.miao@gmail.com>

>
>
> /**
>  * 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);

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

[Sascha Hauer]

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 <axel.lin@ingics.com>

Looks good.

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

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 |

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

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 <axel.lin@ingics.com>
> ---
>  drivers/pwm/pwm-imx.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

All 3 patches applied, thanks.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]