RE: [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-09 Thread Philip, Avinash 
On Fri, Nov 09, 2012 at 13:41:04, Thierry Reding wrote:
> On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
...
> > +   /* Some platforms require explicit tbclk gating */
> > +   if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {
> 
> Is it really necessary to have an extra boolean property for this?
> Couldn't this just be handled by not defining a clock for the tbclk
> consumer in board setup/DT

If no clk node defined, driver can still continue expecting this platform 
the requirement is not there. So I will check for tbclk with devm_clk_get()
and continue by removing Boolean property.

> 
> > +   pc->tbclk = clk_get(>dev, "tbclk");
> 
> You should be using devm_clk_get() or add a matching clk_put() in
> .remove().

Ok

Thanks
Avinash

> 
> Thierry
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-09 Thread Thierry Reding 
On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
> Some platforms (like AM33XX) requires clock gating from control module
> explicitly for TBCLK. Enabling of this clock required for the
> functioning of the time base sub module in EHRPWM module. So adding
> optional TBCLK handling if DT node populated with tbclkgating. This
> helps the driver can coexist for Davinci platforms.
> 
> Signed-off-by: Philip, Avinash 
> Cc:   Grant Likely 
> Cc: Rob Herring 
> Cc: Rob Landley 
> ---
> Changes since v1:
>   - Moved TBCLK enable from probe to .pwm_enable & disable from
> remove to .pwm_disable
> 
> :100644 100644 07911e6... 927a8ed... Mdrivers/pwm/pwm-tiehrpwm.c
>  drivers/pwm/pwm-tiehrpwm.c |   22 ++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 07911e6..927a8ed 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -126,6 +126,7 @@ struct ehrpwm_pwm_chip {
>   void __iomem*mmio_base;
>   unsigned long period_cycles[NUM_PWM_CHANNEL];
>   enum pwm_polarity polarity[NUM_PWM_CHANNEL];
> + struct  clk *tbclk;
>  };
>  
>  static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip 
> *chip)
> @@ -346,6 +347,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, 
> struct pwm_device *pwm)
>   /* Channels polarity can be configured from action qualifier module */
>   configure_polarity(pc, pwm->hwpwm);
>  
> + /*
> +  * Platforms require explicit clock enabling of TBCLK has
> +  * to enable TBCLK explicitly before enabling PWM device
> +  */
> + if (pc->tbclk)
> + clk_enable(pc->tbclk);
> +
>   /* Enable time counter for free_run */
>   ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
>   return 0;
> @@ -374,6 +382,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, 
> struct pwm_device *pwm)
>  
>   ehrpwm_modify(pc->mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
>  
> + /* Disabling TBCLK on PWM disable */
> + if (pc->tbclk)
> + clk_disable(pc->tbclk);
> +
>   /* Stop Time base counter */
>   ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
>  
> @@ -464,6 +476,16 @@ static int __devinit ehrpwm_pwm_probe(struct 
> platform_device *pdev)
>   dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
>   return ret;
>   }
> +
> + /* Some platforms require explicit tbclk gating */
> + if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {

Is it really necessary to have an extra boolean property for this?
Couldn't this just be handled by not defining a clock for the tbclk
consumer in board setup/DT

> + pc->tbclk = clk_get(>dev, "tbclk");

You should be using devm_clk_get() or add a matching clk_put() in
.remove().

Thierry


pgpVSj05EXCSr.pgp
Description: PGP signature

Re: [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-09 Thread Thierry Reding 
On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
 Some platforms (like AM33XX) requires clock gating from control module
 explicitly for TBCLK. Enabling of this clock required for the
 functioning of the time base sub module in EHRPWM module. So adding
 optional TBCLK handling if DT node populated with tbclkgating. This
 helps the driver can coexist for Davinci platforms.
 
 Signed-off-by: Philip, Avinash avinashphi...@ti.com
 Cc:   Grant Likely grant.lik...@secretlab.ca
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 ---
 Changes since v1:
   - Moved TBCLK enable from probe to .pwm_enable  disable from
 remove to .pwm_disable
 
 :100644 100644 07911e6... 927a8ed... Mdrivers/pwm/pwm-tiehrpwm.c
  drivers/pwm/pwm-tiehrpwm.c |   22 ++
  1 files changed, 22 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
 index 07911e6..927a8ed 100644
 --- a/drivers/pwm/pwm-tiehrpwm.c
 +++ b/drivers/pwm/pwm-tiehrpwm.c
 @@ -126,6 +126,7 @@ struct ehrpwm_pwm_chip {
   void __iomem*mmio_base;
   unsigned long period_cycles[NUM_PWM_CHANNEL];
   enum pwm_polarity polarity[NUM_PWM_CHANNEL];
 + struct  clk *tbclk;
  };
  
  static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip 
 *chip)
 @@ -346,6 +347,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, 
 struct pwm_device *pwm)
   /* Channels polarity can be configured from action qualifier module */
   configure_polarity(pc, pwm-hwpwm);
  
 + /*
 +  * Platforms require explicit clock enabling of TBCLK has
 +  * to enable TBCLK explicitly before enabling PWM device
 +  */
 + if (pc-tbclk)
 + clk_enable(pc-tbclk);
 +
   /* Enable time counter for free_run */
   ehrpwm_modify(pc-mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
   return 0;
 @@ -374,6 +382,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, 
 struct pwm_device *pwm)
  
   ehrpwm_modify(pc-mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
  
 + /* Disabling TBCLK on PWM disable */
 + if (pc-tbclk)
 + clk_disable(pc-tbclk);
 +
   /* Stop Time base counter */
   ehrpwm_modify(pc-mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
  
 @@ -464,6 +476,16 @@ static int __devinit ehrpwm_pwm_probe(struct 
 platform_device *pdev)
   dev_err(pdev-dev, pwmchip_add() failed: %d\n, ret);
   return ret;
   }
 +
 + /* Some platforms require explicit tbclk gating */
 + if (of_property_read_bool(pdev-dev.of_node, tbclkgating)) {

Is it really necessary to have an extra boolean property for this?
Couldn't this just be handled by not defining a clock for the tbclk
consumer in board setup/DT

 + pc-tbclk = clk_get(pdev-dev, tbclk);

You should be using devm_clk_get() or add a matching clk_put() in
.remove().

Thierry


pgpVSj05EXCSr.pgp
Description: PGP signature

RE: [PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-09 Thread Philip, Avinash 
On Fri, Nov 09, 2012 at 13:41:04, Thierry Reding wrote:
 On Thu, Nov 08, 2012 at 01:23:15PM +0530, Philip, Avinash wrote:
...
  +   /* Some platforms require explicit tbclk gating */
  +   if (of_property_read_bool(pdev-dev.of_node, tbclkgating)) {
 
 Is it really necessary to have an extra boolean property for this?
 Couldn't this just be handled by not defining a clock for the tbclk
 consumer in board setup/DT

If no clk node defined, driver can still continue expecting this platform 
the requirement is not there. So I will check for tbclk with devm_clk_get()
and continue by removing Boolean property.

 
  +   pc-tbclk = clk_get(pdev-dev, tbclk);
 
 You should be using devm_clk_get() or add a matching clk_put() in
 .remove().

Ok

Thanks
Avinash

 
 Thierry
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-08 Thread Philip, Avinash 
Some platforms (like AM33XX) requires clock gating from control module
explicitly for TBCLK. Enabling of this clock required for the
functioning of the time base sub module in EHRPWM module. So adding
optional TBCLK handling if DT node populated with tbclkgating. This
helps the driver can coexist for Davinci platforms.

Signed-off-by: Philip, Avinash 
Cc: Grant Likely 
Cc: Rob Herring 
Cc: Rob Landley 
---
Changes since v1:
- Moved TBCLK enable from probe to .pwm_enable & disable from
  remove to .pwm_disable

:100644 100644 07911e6... 927a8ed... M  drivers/pwm/pwm-tiehrpwm.c
 drivers/pwm/pwm-tiehrpwm.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 07911e6..927a8ed 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -126,6 +126,7 @@ struct ehrpwm_pwm_chip {
void __iomem*mmio_base;
unsigned long period_cycles[NUM_PWM_CHANNEL];
enum pwm_polarity polarity[NUM_PWM_CHANNEL];
+   struct  clk *tbclk;
 };
 
 static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
@@ -346,6 +347,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct 
pwm_device *pwm)
/* Channels polarity can be configured from action qualifier module */
configure_polarity(pc, pwm->hwpwm);
 
+   /*
+* Platforms require explicit clock enabling of TBCLK has
+* to enable TBCLK explicitly before enabling PWM device
+*/
+   if (pc->tbclk)
+   clk_enable(pc->tbclk);
+
/* Enable time counter for free_run */
ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
return 0;
@@ -374,6 +382,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, 
struct pwm_device *pwm)
 
ehrpwm_modify(pc->mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
 
+   /* Disabling TBCLK on PWM disable */
+   if (pc->tbclk)
+   clk_disable(pc->tbclk);
+
/* Stop Time base counter */
ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
 
@@ -464,6 +476,16 @@ static int __devinit ehrpwm_pwm_probe(struct 
platform_device *pdev)
dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
return ret;
}
+
+   /* Some platforms require explicit tbclk gating */
+   if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {
+   pc->tbclk = clk_get(>dev, "tbclk");
+   if (IS_ERR(pc->tbclk)) {
+   dev_err(>dev, "Could not get EHRPWM TBCLK\n");
+   return PTR_ERR(pc->tbclk);
+   }
+   }
+
pm_runtime_enable(>dev);
pm_runtime_get_sync(>dev);
if (!(pwmss_submodule_state_change(pdev->dev.parent, EPWMCLK_EN) &
-- 
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 08/10] pwm: pwm-tiehrpwm: Adding TBCLK gating support.

2012-11-08 Thread Philip, Avinash 
Some platforms (like AM33XX) requires clock gating from control module
explicitly for TBCLK. Enabling of this clock required for the
functioning of the time base sub module in EHRPWM module. So adding
optional TBCLK handling if DT node populated with tbclkgating. This
helps the driver can coexist for Davinci platforms.

Signed-off-by: Philip, Avinash avinashphi...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
---
Changes since v1:
- Moved TBCLK enable from probe to .pwm_enable  disable from
  remove to .pwm_disable

:100644 100644 07911e6... 927a8ed... M  drivers/pwm/pwm-tiehrpwm.c
 drivers/pwm/pwm-tiehrpwm.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 07911e6..927a8ed 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -126,6 +126,7 @@ struct ehrpwm_pwm_chip {
void __iomem*mmio_base;
unsigned long period_cycles[NUM_PWM_CHANNEL];
enum pwm_polarity polarity[NUM_PWM_CHANNEL];
+   struct  clk *tbclk;
 };
 
 static inline struct ehrpwm_pwm_chip *to_ehrpwm_pwm_chip(struct pwm_chip *chip)
@@ -346,6 +347,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct 
pwm_device *pwm)
/* Channels polarity can be configured from action qualifier module */
configure_polarity(pc, pwm-hwpwm);
 
+   /*
+* Platforms require explicit clock enabling of TBCLK has
+* to enable TBCLK explicitly before enabling PWM device
+*/
+   if (pc-tbclk)
+   clk_enable(pc-tbclk);
+
/* Enable time counter for free_run */
ehrpwm_modify(pc-mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_FREE_RUN);
return 0;
@@ -374,6 +382,10 @@ static void ehrpwm_pwm_disable(struct pwm_chip *chip, 
struct pwm_device *pwm)
 
ehrpwm_modify(pc-mmio_base, AQCSFRC, aqcsfrc_mask, aqcsfrc_val);
 
+   /* Disabling TBCLK on PWM disable */
+   if (pc-tbclk)
+   clk_disable(pc-tbclk);
+
/* Stop Time base counter */
ehrpwm_modify(pc-mmio_base, TBCTL, TBCTL_RUN_MASK, TBCTL_STOP_NEXT);
 
@@ -464,6 +476,16 @@ static int __devinit ehrpwm_pwm_probe(struct 
platform_device *pdev)
dev_err(pdev-dev, pwmchip_add() failed: %d\n, ret);
return ret;
}
+
+   /* Some platforms require explicit tbclk gating */
+   if (of_property_read_bool(pdev-dev.of_node, tbclkgating)) {
+   pc-tbclk = clk_get(pdev-dev, tbclk);
+   if (IS_ERR(pc-tbclk)) {
+   dev_err(pdev-dev, Could not get EHRPWM TBCLK\n);
+   return PTR_ERR(pc-tbclk);
+   }
+   }
+
pm_runtime_enable(pdev-dev);
pm_runtime_get_sync(pdev-dev);
if (!(pwmss_submodule_state_change(pdev-dev.parent, EPWMCLK_EN) 
-- 
1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/