[PATCH 0/4] pwm: tegra: Pin configuration in suspend/resume and cleanups

[PATCH 0/4] pwm: tegra: Pin configuration in suspend/resume and cleanups

[Laxman Dewangan]

This patch series have following fixes:
- Add more precession in PWM period register value calculation
  for lower pwm frequency.
- Add support to configure PWM pins in different state in the
  suspend/resume.

Laxman Dewangan (4):
  pwm: tegra: Use DIV_ROUND_CLOSEST_ULL() instead of local
    implementation
  pwm: tegra: Increase precision in pwm rate calculation
  pwm: tegra: Add DT binding details to configure pin in suspends/resume
  pwm: tegra: Add support to configure pin state in suspends/resume

 .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++
 drivers/pwm/pwm-tegra.c                            | 77 ++++++++++++++++++++--
 2 files changed, 116 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 0/4] pwm: tegra: Pin configuration in suspend/resume and cleanups

[Laxman Dewangan]

This patch series have following fixes:
- Add more precession in PWM period register value calculation
  for lower pwm frequency.
- Add support to configure PWM pins in different state in the
  suspend/resume.

Laxman Dewangan (4):
  pwm: tegra: Use DIV_ROUND_CLOSEST_ULL() instead of local
    implementation
  pwm: tegra: Increase precision in pwm rate calculation
  pwm: tegra: Add DT binding details to configure pin in suspends/resume
  pwm: tegra: Add support to configure pin state in suspends/resume

 .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++
 drivers/pwm/pwm-tegra.c                            | 77 ++++++++++++++++++++--
 2 files changed, 116 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 1/4] pwm: tegra: Use DIV_ROUND_CLOSEST_ULL() instead of local implementation

[Laxman Dewangan]

Use macro DIV_ROUND_CLOSEST_ULL() for 64bit division to closet one
instead of implementing the same locally. This increase readability.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e464784..0a688da 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -85,8 +85,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * nearest integer during division.
 	 */
 	c *= (1 << PWM_DUTY_WIDTH);
-	c += period_ns / 2;
-	do_div(c, period_ns);
+	c = DIV_ROUND_CLOSEST_ULL(c, period_ns);
 
 	val = (u32)c << PWM_DUTY_SHIFT;
 
-- 
2.1.4

[PATCH 1/4] pwm: tegra: Use DIV_ROUND_CLOSEST_ULL() instead of local implementation

[Laxman Dewangan]

Use macro DIV_ROUND_CLOSEST_ULL() for 64bit division to closet one
instead of implementing the same locally. This increase readability.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e464784..0a688da 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -85,8 +85,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * nearest integer during division.
 	 */
 	c *= (1 << PWM_DUTY_WIDTH);
-	c += period_ns / 2;
-	do_div(c, period_ns);
+	c = DIV_ROUND_CLOSEST_ULL(c, period_ns);
 
 	val = (u32)c << PWM_DUTY_SHIFT;
 
-- 
2.1.4

[PATCH 2/4] pwm: tegra: Increase precision in pwm rate calculation

[Laxman Dewangan]

The rate of the PWM calculated as follows:
	hz = NSEC_PER_SEC / period_ns;
 	rate = (rate + (hz / 2)) / hz;

This has the precision loss in lower PWM rate.
Changing this to have more precision as:
	hz = DIV_ROUND_CLOSE(NSEC_PER_SEC * 100, period_ns);
	rate = DIV_ROUND_CLOSE(rate * 100, hz)

Example:
1. period_ns = 16672000, PWM clock rate is 200KHz.
	Based on old formula
		hz = NSEC_PER_SEC / period_ns
		   = 1000000000ul/16672000
		   = 59 (59.98)
		rate = (200K + 59/2)/59 = 3390

	Based on new method:
		hz = 5998
		rate = DIV_ROUND_CLOSE(200000*100, 5998) = 3334

	If we measure the PWM signal rate, we will get more accurate period
	with rate value of 3334 instead of 3390.

2.  period_ns = 16803898, PWM clock rate is 200KHz.
	Based on old formula:
		hz = 60, rate = 3333
	Based on new formula:
		hz = 5951, rate = 3360

	The rate of 3360 is more near to requested period then the 3333.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 0a688da..e9c4de5 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -76,6 +76,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	unsigned long long c = duty_ns;
 	unsigned long rate, hz;
+	unsigned long long ns100 = NSEC_PER_SEC;
+	unsigned long precision = 100; /* Consider 2 digit precision */
 	u32 val = 0;
 	int err;
 
@@ -94,9 +96,11 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * cycles at the PWM clock rate will take period_ns nanoseconds.
 	 */
 	rate = clk_get_rate(pc->clk) >> PWM_DUTY_WIDTH;
-	hz = NSEC_PER_SEC / period_ns;
 
-	rate = (rate + (hz / 2)) / hz;
+	/* Consider precision in PWM_SCALE_WIDTH rate calculation */
+	ns100 *= precision;
+	hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
+	rate = DIV_ROUND_CLOSEST(rate * precision, hz);
 
 	/*
 	 * Since the actual PWM divider is the register's frequency divider
-- 
2.1.4

[PATCH 2/4] pwm: tegra: Increase precision in pwm rate calculation

[Laxman Dewangan]

The rate of the PWM calculated as follows:
	hz = NSEC_PER_SEC / period_ns;
 	rate = (rate + (hz / 2)) / hz;

This has the precision loss in lower PWM rate.
Changing this to have more precision as:
	hz = DIV_ROUND_CLOSE(NSEC_PER_SEC * 100, period_ns);
	rate = DIV_ROUND_CLOSE(rate * 100, hz)

Example:
1. period_ns = 16672000, PWM clock rate is 200KHz.
	Based on old formula
		hz = NSEC_PER_SEC / period_ns
		   = 1000000000ul/16672000
		   = 59 (59.98)
		rate = (200K + 59/2)/59 = 3390

	Based on new method:
		hz = 5998
		rate = DIV_ROUND_CLOSE(200000*100, 5998) = 3334

	If we measure the PWM signal rate, we will get more accurate period
	with rate value of 3334 instead of 3390.

2.  period_ns = 16803898, PWM clock rate is 200KHz.
	Based on old formula:
		hz = 60, rate = 3333
	Based on new formula:
		hz = 5951, rate = 3360

	The rate of 3360 is more near to requested period then the 3333.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 0a688da..e9c4de5 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -76,6 +76,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	unsigned long long c = duty_ns;
 	unsigned long rate, hz;
+	unsigned long long ns100 = NSEC_PER_SEC;
+	unsigned long precision = 100; /* Consider 2 digit precision */
 	u32 val = 0;
 	int err;
 
@@ -94,9 +96,11 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * cycles at the PWM clock rate will take period_ns nanoseconds.
 	 */
 	rate = clk_get_rate(pc->clk) >> PWM_DUTY_WIDTH;
-	hz = NSEC_PER_SEC / period_ns;
 
-	rate = (rate + (hz / 2)) / hz;
+	/* Consider precision in PWM_SCALE_WIDTH rate calculation */
+	ns100 *= precision;
+	hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
+	rate = DIV_ROUND_CLOSEST(rate * precision, hz);
 
 	/*
 	 * Since the actual PWM divider is the register's frequency divider
-- 
2.1.4

[PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Laxman Dewangan]

In some of NVIDIA Tegra's platform, PWM controller is used to
control the PWM controlled regulators. PWM signal is connected to
the VID pin of the regulator where duty cycle of PWM signal decide
the voltage level of the regulator output.

The tristate (high impedance of PWM pin form Tegra) also define
one of the state of PWM regulator which needs to be configure in
suspend state of system.

Add DT binding details to provide the pin configuration state
from PWM and pinctrl DT node in suspend and active state of
the system.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
index b4e7377..145c323 100644
--- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
@@ -19,6 +19,19 @@ Required properties:
 - reset-names: Must include the following entries:
   - pwm
 
+Optional properties:
+============================
+In some of the interface like PWM based regualator device, it is required
+to configure the pins diffrently in different states, specially in suspend
+state of the system. The configuration of pin is provided via the pinctrl
+DT node as detailed in the pinctrl DT binding document
+	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+The PWM node will have following optional properties.
+pinctrl-names:	Pin state names. Must be "suspend" and "resume".
+pinctrl-0:	Node handle of the suspend state configuration of pins.
+pinctrl-1:	Node handle of the resume state configuration of pins.
+
 Example:
 
 	pwm: pwm@7000a000 {
@@ -29,3 +42,33 @@ Example:
 		resets = <&tegra_car 17>;
 		reset-names = "pwm";
 	};
+
+
+Example with the pin configuration for suspend and resume:
+=========================================================
+Here Pin PE7 is used as PWM.
+
+#include <dt-bindings/pinctrl/pinctrl-tegra.h>
+
+	pinmux@70000868 {
+		pwm_suspend: pwm_suspend_state {
+                        pe7 {
+                                nvidia,pins = "pe7";
+                                nvidia,tristate = <TEGRA_PIN_DISABLE>;
+			};
+		};
+
+		pwm_resume: pwm_resume_state {
+                        pe7 {
+                                nvidia,pins = "pe7";
+                                nvidia,tristate = <TEGRA_PIN_DISABLE>;
+			};
+		};
+	};
+
+	pwm@7000a000 {
+		/* Mandatory pwm properties */
+		pinctrl-names = "suspend", "resume";
+		pinctrl-0 = <&pwm_suspend>;
+		pinctrl-1 = <&pwm_resume>;
+	};
-- 
2.1.4

[PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Laxman Dewangan]

In some of NVIDIA Tegra's platform, PWM controller is used to
control the PWM controlled regulators. PWM signal is connected to
the VID pin of the regulator where duty cycle of PWM signal decide
the voltage level of the regulator output.

The tristate (high impedance of PWM pin form Tegra) also define
one of the state of PWM regulator which needs to be configure in
suspend state of system.

Add DT binding details to provide the pin configuration state
from PWM and pinctrl DT node in suspend and active state of
the system.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
index b4e7377..145c323 100644
--- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
@@ -19,6 +19,19 @@ Required properties:
 - reset-names: Must include the following entries:
   - pwm
 
+Optional properties:
+============================
+In some of the interface like PWM based regualator device, it is required
+to configure the pins diffrently in different states, specially in suspend
+state of the system. The configuration of pin is provided via the pinctrl
+DT node as detailed in the pinctrl DT binding document
+	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+The PWM node will have following optional properties.
+pinctrl-names:	Pin state names. Must be "suspend" and "resume".
+pinctrl-0:	Node handle of the suspend state configuration of pins.
+pinctrl-1:	Node handle of the resume state configuration of pins.
+
 Example:
 
 	pwm: pwm@7000a000 {
@@ -29,3 +42,33 @@ Example:
 		resets = <&tegra_car 17>;
 		reset-names = "pwm";
 	};
+
+
+Example with the pin configuration for suspend and resume:
+=========================================================
+Here Pin PE7 is used as PWM.
+
+#include <dt-bindings/pinctrl/pinctrl-tegra.h>
+
+	pinmux@70000868 {
+		pwm_suspend: pwm_suspend_state {
+                        pe7 {
+                                nvidia,pins = "pe7";
+                                nvidia,tristate = <TEGRA_PIN_DISABLE>;
+			};
+		};
+
+		pwm_resume: pwm_resume_state {
+                        pe7 {
+                                nvidia,pins = "pe7";
+                                nvidia,tristate = <TEGRA_PIN_DISABLE>;
+			};
+		};
+	};
+
+	pwm@7000a000 {
+		/* Mandatory pwm properties */
+		pinctrl-names = "suspend", "resume";
+		pinctrl-0 = <&pwm_suspend>;
+		pinctrl-1 = <&pwm_resume>;
+	};
-- 
2.1.4

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Jon Hunter]

On 05/04/17 15:13, Laxman Dewangan wrote:
> In some of NVIDIA Tegra's platform, PWM controller is used to
> control the PWM controlled regulators. PWM signal is connected to
> the VID pin of the regulator where duty cycle of PWM signal decide
> the voltage level of the regulator output.
> 
> The tristate (high impedance of PWM pin form Tegra) also define
> one of the state of PWM regulator which needs to be configure in
> suspend state of system.
> 
> Add DT binding details to provide the pin configuration state
> from PWM and pinctrl DT node in suspend and active state of
> the system.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> index b4e7377..145c323 100644
> --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> @@ -19,6 +19,19 @@ Required properties:
>  - reset-names: Must include the following entries:
>    - pwm
>  
> +Optional properties:
> +============================
> +In some of the interface like PWM based regualator device, it is required
> +to configure the pins diffrently in different states, specially in suspend

s/diffrently/differently
s/specially/especially

> +state of the system. The configuration of pin is provided via the pinctrl
> +DT node as detailed in the pinctrl DT binding document
> +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +
> +The PWM node will have following optional properties.
> +pinctrl-names:	Pin state names. Must be "suspend" and "resume".

Why not just use the pre-defined names here? There is a pre-defined name
for "default", "idle" and "sleep" and then you can use the following
APIs and avoid the lookup of the state ...

pinctrl_pm_select_default_state()
pinctrl_pm_select_idle_state()
pinctrl_pm_select_sleep_state()

Note for i2c [0][1], I used "default" as the active/on state (which I
know is not that descriptive) and then used 'idle' as the suspended
state. This way we don't need any custom names.

Cheers
Jon

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-tegra.c?id=718917b9875fcfa6450e7274d2727c4680c3591a
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/nvidia/tegra210.dtsi?id=66b2d6e9c93bc5bed55afda80c84bf0325c14ad4

-- 
nvpublic

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Jon Hunter]

On 05/04/17 15:13, Laxman Dewangan wrote:
> In some of NVIDIA Tegra's platform, PWM controller is used to
> control the PWM controlled regulators. PWM signal is connected to
> the VID pin of the regulator where duty cycle of PWM signal decide
> the voltage level of the regulator output.
> 
> The tristate (high impedance of PWM pin form Tegra) also define
> one of the state of PWM regulator which needs to be configure in
> suspend state of system.
> 
> Add DT binding details to provide the pin configuration state
> from PWM and pinctrl DT node in suspend and active state of
> the system.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> index b4e7377..145c323 100644
> --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> @@ -19,6 +19,19 @@ Required properties:
>  - reset-names: Must include the following entries:
>    - pwm
>  
> +Optional properties:
> +============================
> +In some of the interface like PWM based regualator device, it is required
> +to configure the pins diffrently in different states, specially in suspend

s/diffrently/differently
s/specially/especially

> +state of the system. The configuration of pin is provided via the pinctrl
> +DT node as detailed in the pinctrl DT binding document
> +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +
> +The PWM node will have following optional properties.
> +pinctrl-names:	Pin state names. Must be "suspend" and "resume".

Why not just use the pre-defined names here? There is a pre-defined name
for "default", "idle" and "sleep" and then you can use the following
APIs and avoid the lookup of the state ...

pinctrl_pm_select_default_state()
pinctrl_pm_select_idle_state()
pinctrl_pm_select_sleep_state()

Note for i2c [0][1], I used "default" as the active/on state (which I
know is not that descriptive) and then used 'idle' as the suspended
state. This way we don't need any custom names.

Cheers
Jon

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-tegra.c?id=718917b9875fcfa6450e7274d2727c4680c3591a
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/nvidia/tegra210.dtsi?id=66b2d6e9c93bc5bed55afda80c84bf0325c14ad4

-- 
nvpublic

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Thierry Reding]

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

On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
> 
> On 05/04/17 15:13, Laxman Dewangan wrote:
> > In some of NVIDIA Tegra's platform, PWM controller is used to
> > control the PWM controlled regulators. PWM signal is connected to
> > the VID pin of the regulator where duty cycle of PWM signal decide
> > the voltage level of the regulator output.
> > 
> > The tristate (high impedance of PWM pin form Tegra) also define
> > one of the state of PWM regulator which needs to be configure in
> > suspend state of system.
> > 
> > Add DT binding details to provide the pin configuration state
> > from PWM and pinctrl DT node in suspend and active state of
> > the system.
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > index b4e7377..145c323 100644
> > --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > @@ -19,6 +19,19 @@ Required properties:
> >  - reset-names: Must include the following entries:
> >    - pwm
> >  
> > +Optional properties:
> > +============================
> > +In some of the interface like PWM based regualator device, it is required
> > +to configure the pins diffrently in different states, specially in suspend
> 
> s/diffrently/differently
> s/specially/especially
> 
> > +state of the system. The configuration of pin is provided via the pinctrl
> > +DT node as detailed in the pinctrl DT binding document
> > +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > +
> > +The PWM node will have following optional properties.
> > +pinctrl-names:	Pin state names. Must be "suspend" and "resume".
> 
> Why not just use the pre-defined names here? There is a pre-defined name
> for "default", "idle" and "sleep" and then you can use the following
> APIs and avoid the lookup of the state ...
> 
> pinctrl_pm_select_default_state()
> pinctrl_pm_select_idle_state()
> pinctrl_pm_select_sleep_state()
> 
> Note for i2c [0][1], I used "default" as the active/on state (which I
> know is not that descriptive) and then used 'idle' as the suspended
> state. This way we don't need any custom names.

Agreed, I think that's how these states are meant to be used.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Thierry Reding]

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

On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
> 
> On 05/04/17 15:13, Laxman Dewangan wrote:
> > In some of NVIDIA Tegra's platform, PWM controller is used to
> > control the PWM controlled regulators. PWM signal is connected to
> > the VID pin of the regulator where duty cycle of PWM signal decide
> > the voltage level of the regulator output.
> > 
> > The tristate (high impedance of PWM pin form Tegra) also define
> > one of the state of PWM regulator which needs to be configure in
> > suspend state of system.
> > 
> > Add DT binding details to provide the pin configuration state
> > from PWM and pinctrl DT node in suspend and active state of
> > the system.
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> > ---
> >  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > index b4e7377..145c323 100644
> > --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > @@ -19,6 +19,19 @@ Required properties:
> >  - reset-names: Must include the following entries:
> >    - pwm
> >  
> > +Optional properties:
> > +============================
> > +In some of the interface like PWM based regualator device, it is required
> > +to configure the pins diffrently in different states, specially in suspend
> 
> s/diffrently/differently
> s/specially/especially
> 
> > +state of the system. The configuration of pin is provided via the pinctrl
> > +DT node as detailed in the pinctrl DT binding document
> > +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > +
> > +The PWM node will have following optional properties.
> > +pinctrl-names:	Pin state names. Must be "suspend" and "resume".
> 
> Why not just use the pre-defined names here? There is a pre-defined name
> for "default", "idle" and "sleep" and then you can use the following
> APIs and avoid the lookup of the state ...
> 
> pinctrl_pm_select_default_state()
> pinctrl_pm_select_idle_state()
> pinctrl_pm_select_sleep_state()
> 
> Note for i2c [0][1], I used "default" as the active/on state (which I
> know is not that descriptive) and then used 'idle' as the suspended
> state. This way we don't need any custom names.

Agreed, I think that's how these states are meant to be used.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Laxman Dewangan]

On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>> +state of the system. The configuration of pin is provided via the pinctrl
>>> +DT node as detailed in the pinctrl DT binding document
>>> +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>> +
>>> +The PWM node will have following optional properties.
>>> +pinctrl-names:	Pin state names. Must be "suspend" and "resume".
>> Why not just use the pre-defined names here? There is a pre-defined name
>> for "default", "idle" and "sleep" and then you can use the following
>> APIs and avoid the lookup of the state ...
>>
>> pinctrl_pm_select_default_state()
>> pinctrl_pm_select_idle_state()
>> pinctrl_pm_select_sleep_state()
>>
>> Note for i2c [0][1], I used "default" as the active/on state (which I
>> know is not that descriptive) and then used 'idle' as the suspended
>> state. This way we don't need any custom names.
> Agreed, I think that's how these states are meant to be used.
I did quick grep for the pinctrl_pm_select_* functions in the code tree 
and found usage of these APIs in some of the places.
I am taking the reference of i2c-st, i2c-nomadic and 
extcon/extcon-usb-gpio.c drivers and from this the interpretation is

default state: When interface active and transfer need to be done in IO 
interface.
idle state: Active state of the system but interface is not active, put 
in non-active state of the interface.
sleep state: When system entering into suspend and IO interface is going 
to be inactive.

So in PWM case, we will need the "default" and "sleep" state.

In suspend(), set the "sleep" state and in resume, set the "default" state.

+ Linus W as I refereed his st/nomadik driver for reference.

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Laxman Dewangan]

On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>> +state of the system. The configuration of pin is provided via the pinctrl
>>> +DT node as detailed in the pinctrl DT binding document
>>> +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>> +
>>> +The PWM node will have following optional properties.
>>> +pinctrl-names:	Pin state names. Must be "suspend" and "resume".
>> Why not just use the pre-defined names here? There is a pre-defined name
>> for "default", "idle" and "sleep" and then you can use the following
>> APIs and avoid the lookup of the state ...
>>
>> pinctrl_pm_select_default_state()
>> pinctrl_pm_select_idle_state()
>> pinctrl_pm_select_sleep_state()
>>
>> Note for i2c [0][1], I used "default" as the active/on state (which I
>> know is not that descriptive) and then used 'idle' as the suspended
>> state. This way we don't need any custom names.
> Agreed, I think that's how these states are meant to be used.
I did quick grep for the pinctrl_pm_select_* functions in the code tree 
and found usage of these APIs in some of the places.
I am taking the reference of i2c-st, i2c-nomadic and 
extcon/extcon-usb-gpio.c drivers and from this the interpretation is

default state: When interface active and transfer need to be done in IO 
interface.
idle state: Active state of the system but interface is not active, put 
in non-active state of the interface.
sleep state: When system entering into suspend and IO interface is going 
to be inactive.

So in PWM case, we will need the "default" and "sleep" state.

In suspend(), set the "sleep" state and in resume, set the "default" state.

+ Linus W as I refereed his st/nomadik driver for reference.

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Jon Hunter]

On 06/04/17 14:19, Laxman Dewangan wrote:
> 
> On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>>> +state of the system. The configuration of pin is provided via the
>>>> pinctrl
>>>> +DT node as detailed in the pinctrl DT binding document
>>>> +    Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>>> +
>>>> +The PWM node will have following optional properties.
>>>> +pinctrl-names:    Pin state names. Must be "suspend" and "resume".
>>> Why not just use the pre-defined names here? There is a pre-defined name
>>> for "default", "idle" and "sleep" and then you can use the following
>>> APIs and avoid the lookup of the state ...
>>>
>>> pinctrl_pm_select_default_state()
>>> pinctrl_pm_select_idle_state()
>>> pinctrl_pm_select_sleep_state()
>>>
>>> Note for i2c [0][1], I used "default" as the active/on state (which I
>>> know is not that descriptive) and then used 'idle' as the suspended
>>> state. This way we don't need any custom names.
>> Agreed, I think that's how these states are meant to be used.
> I did quick grep for the pinctrl_pm_select_* functions in the code tree
> and found usage of these APIs in some of the places.
> I am taking the reference of i2c-st, i2c-nomadic and
> extcon/extcon-usb-gpio.c drivers and from this the interpretation is
> 
> default state: When interface active and transfer need to be done in IO
> interface.
> idle state: Active state of the system but interface is not active, put
> in non-active state of the interface.
> sleep state: When system entering into suspend and IO interface is going
> to be inactive.
> 
> So in PWM case, we will need the "default" and "sleep" state.
> 
> In suspend(), set the "sleep" state and in resume, set the "default" state.
> 
> + Linus W as I refereed his st/nomadik driver for reference.

I mis-spoke before, I used 'idle' for i2c as I am using it in the
context of the runtime-pm callbacks and not suspend. So sleep is fine
with me.

Jon

-- 
nvpublic

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Jon Hunter]

On 06/04/17 14:19, Laxman Dewangan wrote:
> 
> On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>>> +state of the system. The configuration of pin is provided via the
>>>> pinctrl
>>>> +DT node as detailed in the pinctrl DT binding document
>>>> +    Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>>> +
>>>> +The PWM node will have following optional properties.
>>>> +pinctrl-names:    Pin state names. Must be "suspend" and "resume".
>>> Why not just use the pre-defined names here? There is a pre-defined name
>>> for "default", "idle" and "sleep" and then you can use the following
>>> APIs and avoid the lookup of the state ...
>>>
>>> pinctrl_pm_select_default_state()
>>> pinctrl_pm_select_idle_state()
>>> pinctrl_pm_select_sleep_state()
>>>
>>> Note for i2c [0][1], I used "default" as the active/on state (which I
>>> know is not that descriptive) and then used 'idle' as the suspended
>>> state. This way we don't need any custom names.
>> Agreed, I think that's how these states are meant to be used.
> I did quick grep for the pinctrl_pm_select_* functions in the code tree
> and found usage of these APIs in some of the places.
> I am taking the reference of i2c-st, i2c-nomadic and
> extcon/extcon-usb-gpio.c drivers and from this the interpretation is
> 
> default state: When interface active and transfer need to be done in IO
> interface.
> idle state: Active state of the system but interface is not active, put
> in non-active state of the interface.
> sleep state: When system entering into suspend and IO interface is going
> to be inactive.
> 
> So in PWM case, we will need the "default" and "sleep" state.
> 
> In suspend(), set the "sleep" state and in resume, set the "default" state.
> 
> + Linus W as I refereed his st/nomadik driver for reference.

I mis-spoke before, I used 'idle' for i2c as I am using it in the
context of the runtime-pm callbacks and not suspend. So sleep is fine
with me.

Jon

-- 
nvpublic

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Linus Walleij]

On Thu, Apr 6, 2017 at 3:19 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
>> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>>>
>>>> +state of the system. The configuration of pin is provided via the
>>>> pinctrl
>>>> +DT node as detailed in the pinctrl DT binding document
>>>> +       Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>>> +
>>>> +The PWM node will have following optional properties.
>>>> +pinctrl-names: Pin state names. Must be "suspend" and "resume".
>>>
>>> Why not just use the pre-defined names here? There is a pre-defined name
>>> for "default", "idle" and "sleep" and then you can use the following
>>> APIs and avoid the lookup of the state ...
>>>
>>> pinctrl_pm_select_default_state()
>>> pinctrl_pm_select_idle_state()
>>> pinctrl_pm_select_sleep_state()
>>>
>>> Note for i2c [0][1], I used "default" as the active/on state (which I
>>> know is not that descriptive) and then used 'idle' as the suspended
>>> state. This way we don't need any custom names.
>>
>> Agreed, I think that's how these states are meant to be used.
>
> I did quick grep for the pinctrl_pm_select_* functions in the code tree and
> found usage of these APIs in some of the places.
> I am taking the reference of i2c-st, i2c-nomadic and
> extcon/extcon-usb-gpio.c drivers and from this the interpretation is
>
> default state: When interface active and transfer need to be done in IO
> interface.
> idle state: Active state of the system but interface is not active, put in
> non-active state of the interface.
> sleep state: When system entering into suspend and IO interface is going to
> be inactive.
>
> So in PWM case, we will need the "default" and "sleep" state.
>
> In suspend(), set the "sleep" state and in resume, set the "default" state.
>
> + Linus W as I refereed his st/nomadik driver for reference.

It is actually documented:
include/linux/pinctrl/pinctrl-state.h

/*
 * Standard pin control state definitions
 */

/**
 * @PINCTRL_STATE_DEFAULT: the state the pinctrl handle shall be put
 *      into as default, usually this means the pins are up and ready to
 *      be used by the device driver. This state is commonly used by
 *      hogs to configure muxing and pins at boot, and also as a state
 *      to go into when returning from sleep and idle in
 *      .pm_runtime_resume() or ordinary .resume() for example.
 * @PINCTRL_STATE_INIT: normally the pinctrl will be set to "default"
 *      before the driver's probe() function is called.  There are some
 *      drivers where that is not appropriate becausing doing so would
 *      glitch the pins.  In those cases you can add an "init" pinctrl
 *      which is the state of the pins before drive probe.  After probe
 *      if the pins are still in "init" state they'll be moved to
 *      "default".
 * @PINCTRL_STATE_IDLE: the state the pinctrl handle shall be put into
 *      when the pins are idle. This is a state where the system is relaxed
 *      but not fully sleeping - some power may be on but clocks gated for
 *      example. Could typically be set from a pm_runtime_suspend() or
 *      pm_runtime_idle() operation.
 * @PINCTRL_STATE_SLEEP: the state the pinctrl handle shall be put into
 *      when the pins are sleeping. This is a state where the system is in
 *      its lowest sleep state. Could typically be set from an
 *      ordinary .suspend() function.
 */
#define PINCTRL_STATE_DEFAULT "default"
#define PINCTRL_STATE_INIT "init"
#define PINCTRL_STATE_IDLE "idle"
#define PINCTRL_STATE_SLEEP "sleep"

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

[Linus Walleij]

On Thu, Apr 6, 2017 at 3:19 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Thursday 06 April 2017 06:33 PM, Thierry Reding wrote:
>> On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
>>> On 05/04/17 15:13, Laxman Dewangan wrote:
>>>>
>>>> +state of the system. The configuration of pin is provided via the
>>>> pinctrl
>>>> +DT node as detailed in the pinctrl DT binding document
>>>> +       Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>>> +
>>>> +The PWM node will have following optional properties.
>>>> +pinctrl-names: Pin state names. Must be "suspend" and "resume".
>>>
>>> Why not just use the pre-defined names here? There is a pre-defined name
>>> for "default", "idle" and "sleep" and then you can use the following
>>> APIs and avoid the lookup of the state ...
>>>
>>> pinctrl_pm_select_default_state()
>>> pinctrl_pm_select_idle_state()
>>> pinctrl_pm_select_sleep_state()
>>>
>>> Note for i2c [0][1], I used "default" as the active/on state (which I
>>> know is not that descriptive) and then used 'idle' as the suspended
>>> state. This way we don't need any custom names.
>>
>> Agreed, I think that's how these states are meant to be used.
>
> I did quick grep for the pinctrl_pm_select_* functions in the code tree and
> found usage of these APIs in some of the places.
> I am taking the reference of i2c-st, i2c-nomadic and
> extcon/extcon-usb-gpio.c drivers and from this the interpretation is
>
> default state: When interface active and transfer need to be done in IO
> interface.
> idle state: Active state of the system but interface is not active, put in
> non-active state of the interface.
> sleep state: When system entering into suspend and IO interface is going to
> be inactive.
>
> So in PWM case, we will need the "default" and "sleep" state.
>
> In suspend(), set the "sleep" state and in resume, set the "default" state.
>
> + Linus W as I refereed his st/nomadik driver for reference.

It is actually documented:
include/linux/pinctrl/pinctrl-state.h

/*
 * Standard pin control state definitions
 */

/**
 * @PINCTRL_STATE_DEFAULT: the state the pinctrl handle shall be put
 *      into as default, usually this means the pins are up and ready to
 *      be used by the device driver. This state is commonly used by
 *      hogs to configure muxing and pins at boot, and also as a state
 *      to go into when returning from sleep and idle in
 *      .pm_runtime_resume() or ordinary .resume() for example.
 * @PINCTRL_STATE_INIT: normally the pinctrl will be set to "default"
 *      before the driver's probe() function is called.  There are some
 *      drivers where that is not appropriate becausing doing so would
 *      glitch the pins.  In those cases you can add an "init" pinctrl
 *      which is the state of the pins before drive probe.  After probe
 *      if the pins are still in "init" state they'll be moved to
 *      "default".
 * @PINCTRL_STATE_IDLE: the state the pinctrl handle shall be put into
 *      when the pins are idle. This is a state where the system is relaxed
 *      but not fully sleeping - some power may be on but clocks gated for
 *      example. Could typically be set from a pm_runtime_suspend() or
 *      pm_runtime_idle() operation.
 * @PINCTRL_STATE_SLEEP: the state the pinctrl handle shall be put into
 *      when the pins are sleeping. This is a state where the system is in
 *      its lowest sleep state. Could typically be set from an
 *      ordinary .suspend() function.
 */
#define PINCTRL_STATE_DEFAULT "default"
#define PINCTRL_STATE_INIT "init"
#define PINCTRL_STATE_IDLE "idle"
#define PINCTRL_STATE_SLEEP "sleep"

Yours,
Linus Walleij

[PATCH 4/4] pwm: tegra: Add support to configure pin state in suspends/resume

[Laxman Dewangan]

In some of NVIDIA Tegra's platform, PWM controller is used to
control the PWM controlled regulators. PWM signal is connected to
the VID pin of the regulator where duty cycle of PWM signal decide
the voltage level of the regulator output.

The tristate (high impedance of PWM pin form Tegra) also define
one of the state of PWM regulator which needs to be configure in
suspend state of system.

Add support to configure the pin state via pinctrl frameworks in
suspend and active state of the system.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/pwm/pwm-tegra.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e9c4de5..60ed522 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -29,6 +29,7 @@
 #include <linux/of_device.h>
 #include <linux/pwm.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/slab.h>
 #include <linux/reset.h>
 
@@ -52,6 +53,9 @@ struct tegra_pwm_chip {
 	void __iomem *regs;
 
 	const struct tegra_pwm_soc *soc;
+	struct pinctrl		*pinctrl;
+	struct pinctrl_state	*suspend_state;
+	struct pinctrl_state	*resume_state;
 };
 
 static inline struct tegra_pwm_chip *to_tegra_pwm_chip(struct pwm_chip *chip)
@@ -215,6 +219,27 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	pwm->chip.base = -1;
 	pwm->chip.npwm = pwm->soc->num_channels;
 
+	pwm->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!IS_ERR(pwm->pinctrl)) {
+		pwm->suspend_state = pinctrl_lookup_state(pwm->pinctrl,
+							  "suspend");
+		if (IS_ERR(pwm->suspend_state)) {
+			/* Ignore error other than PROBE_DEFER */
+			ret = PTR_ERR(pwm->suspend_state);
+			if (ret == -EPROBE_DEFER)
+				return ret;
+		}
+
+		pwm->resume_state = pinctrl_lookup_state(pwm->pinctrl,
+							 "resume");
+		if (IS_ERR(pwm->resume_state)) {
+			/* Ignore error other than PROBE_DEFER */
+			ret = PTR_ERR(pwm->resume_state);
+			if (ret == -EPROBE_DEFER)
+				return ret;
+		}
+	}
+
 	ret = pwmchip_add(&pwm->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
@@ -256,6 +281,42 @@ static int tegra_pwm_remove(struct platform_device *pdev)
 	return pwmchip_remove(&pc->chip);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int tegra_pwm_suspend(struct device *dev)
+{
+	struct tegra_pwm_chip *pc = dev_get_drvdata(dev);
+	int ret;
+
+	if (IS_ERR(pc->pinctrl) || IS_ERR(pc->suspend_state))
+		return 0;
+
+	ret = pinctrl_select_state(pc->pinctrl, pc->suspend_state);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set pin into suspend state:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tegra_pwm_resume(struct device *dev)
+{
+	struct tegra_pwm_chip *pc = dev_get_drvdata(dev);
+	int ret;
+
+	if (IS_ERR(pc->pinctrl) || IS_ERR(pc->resume_state))
+		return 0;
+
+	ret = pinctrl_select_state(pc->pinctrl, pc->resume_state);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set pin into resume state:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
 };
@@ -272,10 +333,15 @@ static const struct of_device_id tegra_pwm_of_match[] = {
 
 MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);
 
+static const struct dev_pm_ops tegra_pwm_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume)
+};
+
 static struct platform_driver tegra_pwm_driver = {
 	.driver = {
 		.name = "tegra-pwm",
 		.of_match_table = tegra_pwm_of_match,
+		.pm = &tegra_pwm_pm_ops,
 	},
 	.probe = tegra_pwm_probe,
 	.remove = tegra_pwm_remove,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] pwm: tegra: Add support to configure pin state in suspends/resume

[Laxman Dewangan]

In some of NVIDIA Tegra's platform, PWM controller is used to
control the PWM controlled regulators. PWM signal is connected to
the VID pin of the regulator where duty cycle of PWM signal decide
the voltage level of the regulator output.

The tristate (high impedance of PWM pin form Tegra) also define
one of the state of PWM regulator which needs to be configure in
suspend state of system.

Add support to configure the pin state via pinctrl frameworks in
suspend and active state of the system.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e9c4de5..60ed522 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -29,6 +29,7 @@
 #include <linux/of_device.h>
 #include <linux/pwm.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/slab.h>
 #include <linux/reset.h>
 
@@ -52,6 +53,9 @@ struct tegra_pwm_chip {
 	void __iomem *regs;
 
 	const struct tegra_pwm_soc *soc;
+	struct pinctrl		*pinctrl;
+	struct pinctrl_state	*suspend_state;
+	struct pinctrl_state	*resume_state;
 };
 
 static inline struct tegra_pwm_chip *to_tegra_pwm_chip(struct pwm_chip *chip)
@@ -215,6 +219,27 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	pwm->chip.base = -1;
 	pwm->chip.npwm = pwm->soc->num_channels;
 
+	pwm->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!IS_ERR(pwm->pinctrl)) {
+		pwm->suspend_state = pinctrl_lookup_state(pwm->pinctrl,
+							  "suspend");
+		if (IS_ERR(pwm->suspend_state)) {
+			/* Ignore error other than PROBE_DEFER */
+			ret = PTR_ERR(pwm->suspend_state);
+			if (ret == -EPROBE_DEFER)
+				return ret;
+		}
+
+		pwm->resume_state = pinctrl_lookup_state(pwm->pinctrl,
+							 "resume");
+		if (IS_ERR(pwm->resume_state)) {
+			/* Ignore error other than PROBE_DEFER */
+			ret = PTR_ERR(pwm->resume_state);
+			if (ret == -EPROBE_DEFER)
+				return ret;
+		}
+	}
+
 	ret = pwmchip_add(&pwm->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
@@ -256,6 +281,42 @@ static int tegra_pwm_remove(struct platform_device *pdev)
 	return pwmchip_remove(&pc->chip);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int tegra_pwm_suspend(struct device *dev)
+{
+	struct tegra_pwm_chip *pc = dev_get_drvdata(dev);
+	int ret;
+
+	if (IS_ERR(pc->pinctrl) || IS_ERR(pc->suspend_state))
+		return 0;
+
+	ret = pinctrl_select_state(pc->pinctrl, pc->suspend_state);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set pin into suspend state:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tegra_pwm_resume(struct device *dev)
+{
+	struct tegra_pwm_chip *pc = dev_get_drvdata(dev);
+	int ret;
+
+	if (IS_ERR(pc->pinctrl) || IS_ERR(pc->resume_state))
+		return 0;
+
+	ret = pinctrl_select_state(pc->pinctrl, pc->resume_state);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set pin into resume state:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
 };
@@ -272,10 +333,15 @@ static const struct of_device_id tegra_pwm_of_match[] = {
 
 MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);
 
+static const struct dev_pm_ops tegra_pwm_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume)
+};
+
 static struct platform_driver tegra_pwm_driver = {
 	.driver = {
 		.name = "tegra-pwm",
 		.of_match_table = tegra_pwm_of_match,
+		.pm = &tegra_pwm_pm_ops,
 	},
 	.probe = tegra_pwm_probe,
 	.remove = tegra_pwm_remove,
-- 
2.1.4