Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1618081 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in suspends/resume Laxman Dewangan <ldewangan@nvidia.com> - 2017-04-06 16:50 +0200
    Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in  suspends/resume Jon Hunter <jonathanh@nvidia.com> - 2017-04-06 17:20 +0200
      Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state  in suspends/resume Laxman Dewangan <ldewangan@nvidia.com> - 2017-04-06 19:10 +0200
        Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in  suspends/resume Jon Hunter <jonathanh@nvidia.com> - 2017-04-07 10:00 +0200

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

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>
---
Changes from v1:
- Use standard pinctrl names for sleep and active state.
- Use API pinctrl_pm_select_*()

 drivers/pwm/pwm-tegra.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e9c4de5..af1bd4f 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>
 
@@ -256,6 +257,22 @@ 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)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int tegra_pwm_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+#endif
+
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
 };
@@ -272,10 +289,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

[toc] | [next] | [standalone]

#1618104 — Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in suspends/resume

On 06/04/17 15:21, 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 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>
> ---
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
> - Use API pinctrl_pm_select_*()
> 
>  drivers/pwm/pwm-tegra.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index e9c4de5..af1bd4f 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>
>  
> @@ -256,6 +257,22 @@ 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)
> +{
> +	pinctrl_pm_select_sleep_state(dev);

Why not return the error code here?

> +
> +	return 0;
> +}
> +
> +static int tegra_pwm_resume(struct device *dev)
> +{
> +	pinctrl_pm_select_default_state(dev);

And here.

By the way, do you plan to include patches to populate the bindings for
the pwm devices?

Cheers
Jon

-- 
nvpublic

[toc] | [prev] | [next] | [standalone]

#1618201 — Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in suspends/resume

Oops, it was actually v2.

On Thursday 06 April 2017 08:47 PM, Jon Hunter wrote:
> On 06/04/17 15:21, 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 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>
>> ---
>> Changes from v1:
>> - Use standard pinctrl names for sleep and active state.
>> - Use API pinctrl_pm_select_*()
>>
>>   drivers/pwm/pwm-tegra.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
>> index e9c4de5..af1bd4f 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>
>>   
>> @@ -256,6 +257,22 @@ 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)
>> +{
>> +	pinctrl_pm_select_sleep_state(dev);
> Why not return the error code here?

As the pin state in suspend is optional, I dont  want to return error if 
the sleep state is not available.

However, it seems pinctrl take care of retuning success if there is no 
sleep state. By seeing code.
Let me test this on different condition and it it works fine then we can 
return the return of pinctrl_pm_select_*()


BTW, it should be OK to have pwm_tegra_resume/suspend wrapper, not 
directly use the pinctrl_pm_select_* in pm ops suspend/resume. The 
prototype matches.



>
> By the way, do you plan to include patches to populate the bindings for
> the pwm devices?

I am planning to populate the GPU regulator which is PWM based. This 
will only populate the regulator.

[toc] | [prev] | [next] | [standalone]

#1618553 — Re: [PATCH V4 4/4] pwm: tegra: Add support to configure pin state in suspends/resume

On 06/04/17 17:40, Laxman Dewangan wrote:
> Oops, it was actually v2.
> 
> On Thursday 06 April 2017 08:47 PM, Jon Hunter wrote:
>> On 06/04/17 15:21, 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 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>
>>> ---
>>> Changes from v1:
>>> - Use standard pinctrl names for sleep and active state.
>>> - Use API pinctrl_pm_select_*()
>>>
>>>   drivers/pwm/pwm-tegra.c | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
>>> index e9c4de5..af1bd4f 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>
>>>   @@ -256,6 +257,22 @@ 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)
>>> +{
>>> +    pinctrl_pm_select_sleep_state(dev);
>> Why not return the error code here?
> 
> As the pin state in suspend is optional, I dont  want to return error if
> the sleep state is not available.
> 
> However, it seems pinctrl take care of retuning success if there is no
> sleep state. By seeing code.

Exactly, that is what I did for i2c because pinctrl it is also optional
for i2c.

> Let me test this on different condition and it it works fine then we can
> return the return of pinctrl_pm_select_*()
> 
> 
> BTW, it should be OK to have pwm_tegra_resume/suspend wrapper, not
> directly use the pinctrl_pm_select_* in pm ops suspend/resume. The
> prototype matches.

I think that I would keep the wrapper.

>> By the way, do you plan to include patches to populate the bindings for
>> the pwm devices?
> 
> I am planning to populate the GPU regulator which is PWM based. This
> will only populate the regulator.

Ok.

Jon

-- 
nvpublic

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web