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.
Changes from v1:
- Use standard pinctrl names for sleep and active state.
- Use API pinctrl_pm_select_*()
Changes from V2:
- Type fixes, rephrases commit message and use pinctrl_pm_state* return
value.
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
Use macro DIV_ROUND_CLOSEST_ULL() for 64bit division to closest one
instead of implementing the same locally. This increase readability.
Signed-off-by: Laxman Dewangan <[email protected]>
---
Changes from v1:
- None
Changes from V2:
- Fix typo in commit message.
---
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
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.
Change this to have more precision as:
hz = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC * 100, period_ns);
rate = DIV_ROUND_CLOSEST(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 = 59, rate = 3390
Based on new formula:
hz = 5951, rate = 3360
The PWM signal rate of 3360 is more near to requested period than 3333.
Signed-off-by: Laxman Dewangan <[email protected]>
---
Changes from v1:
- None
Changes from V2:
- Fix the commit message with exact formula used.
---
drivers/pwm/pwm-tegra.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 0a688da..21518be 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -76,6 +76,7 @@ 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;
u32 val = 0;
int err;
@@ -94,9 +95,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 *= 100;
+ hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
+ rate = DIV_ROUND_CLOSEST(rate * 100, hz);
/*
* Since the actual PWM divider is the register's frequency divider
--
2.1.4
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.
When system enters suspend, some PWM client/slave regulator devices
require the PWM output to be tristated.
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 <[email protected]>
---
Changes from v1:
- Use standard pinctrl names for sleep and active state.
Changes from V2:
- Fix the commit message and details
---
.../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 45 ++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
index b4e7377..c57e11b 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 regulator device, it is required
+to configure the pins differently in different states, especially 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 "default" and "sleep".
+pinctrl-0: phandle for the default/active state of pin configurations.
+pinctrl-1: phandle for the sleep state of pin configurations.
+
Example:
pwm: pwm@7000a000 {
@@ -29,3 +42,35 @@ Example:
resets = <&tegra_car 17>;
reset-names = "pwm";
};
+
+
+Example with the pin configuration for suspend and resume:
+=========================================================
+Suppose pin PE7 (On Tegra210) interfaced with the regulator device and
+it requires PWM output to be tristated when system enters suspend.
+Following will be DT binding to achieve this:
+
+#include <dt-bindings/pinctrl/pinctrl-tegra.h>
+
+ pinmux@700008d4 {
+ pwm_active_state: pwm_active_state {
+ pe7 {
+ nvidia,pins = "pe7";
+ nvidia,tristate = <TEGRA_PIN_DISABLE>;
+ };
+ };
+
+ pwm_sleep_state: pwm_sleep_state {
+ pe7 {
+ nvidia,pins = "pe7";
+ nvidia,tristate = <TEGRA_PIN_ENABLE>;
+ };
+ };
+ };
+
+ pwm@7000a000 {
+ /* Mandatory PWM properties */
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&pwm_active_state>;
+ pinctrl-1 = <&pwm_sleep_state>;
+ };
--
2.1.4
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.
When system enters suspend, some PWM client/slave regulator devices
require the PWM output to be tristated.
Add support to configure the pin state via pinctrl frameworks in
suspend and active state of the system.
Signed-off-by: Laxman Dewangan <[email protected]>
---
Changes from v1:
- Use standard pinctrl names for sleep and active state.
- Use API pinctrl_pm_select_*()
Changes from V2:
- Use returns of pinctrl_pm_select_*()
- Rephrase commit message.
---
drivers/pwm/pwm-tegra.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 21518be..9c7f180 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>
@@ -255,6 +256,18 @@ 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)
+{
+ return pinctrl_pm_select_sleep_state(dev);
+}
+
+static int tegra_pwm_resume(struct device *dev)
+{
+ return pinctrl_pm_select_default_state(dev);
+}
+#endif
+
static const struct tegra_pwm_soc tegra20_pwm_soc = {
.num_channels = 4,
};
@@ -271,10 +284,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
On 07/04/17 10:34, 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.
>
> When system enters suspend, some PWM client/slave regulator devices
> require the PWM output to be tristated.
>
> 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 <[email protected]>
>
> ---
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
>
> Changes from V2:
> - Fix the commit message and details
> ---
> .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 45 ++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> index b4e7377..c57e11b 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 regulator device, it is required
> +to configure the pins differently in different states, especially 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 "default" and "sleep".
> +pinctrl-0: phandle for the default/active state of pin configurations.
> +pinctrl-1: phandle for the sleep state of pin configurations.
> +
> Example:
>
> pwm: pwm@7000a000 {
> @@ -29,3 +42,35 @@ Example:
> resets = <&tegra_car 17>;
> reset-names = "pwm";
> };
> +
> +
> +Example with the pin configuration for suspend and resume:
> +=========================================================
> +Suppose pin PE7 (On Tegra210) interfaced with the regulator device and
> +it requires PWM output to be tristated when system enters suspend.
> +Following will be DT binding to achieve this:
> +
> +#include <dt-bindings/pinctrl/pinctrl-tegra.h>
> +
> + pinmux@700008d4 {
> + pwm_active_state: pwm_active_state {
> + pe7 {
> + nvidia,pins = "pe7";
> + nvidia,tristate = <TEGRA_PIN_DISABLE>;
> + };
> + };
> +
> + pwm_sleep_state: pwm_sleep_state {
> + pe7 {
> + nvidia,pins = "pe7";
> + nvidia,tristate = <TEGRA_PIN_ENABLE>;
> + };
> + };
> + };
> +
> + pwm@7000a000 {
> + /* Mandatory PWM properties */
Maybe these are mandatory for the platform, but given these are
optional, its a bit confusing.
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&pwm_active_state>;
> + pinctrl-1 = <&pwm_sleep_state>;
> + };
However, fine with me so ...
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
On 07/04/17 10:34, 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.
>
> When system enters suspend, some PWM client/slave regulator devices
> require the PWM output to be tristated.
>
> Add support to configure the pin state via pinctrl frameworks in
> suspend and active state of the system.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
>
> ---
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
> - Use API pinctrl_pm_select_*()
>
> Changes from V2:
> - Use returns of pinctrl_pm_select_*()
> - Rephrase commit message.
> ---
> drivers/pwm/pwm-tegra.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 21518be..9c7f180 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>
>
> @@ -255,6 +256,18 @@ 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)
> +{
> + return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int tegra_pwm_resume(struct device *dev)
> +{
> + return pinctrl_pm_select_default_state(dev);
> +}
> +#endif
> +
> static const struct tegra_pwm_soc tegra20_pwm_soc = {
> .num_channels = 4,
> };
> @@ -271,10 +284,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,
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
On Fri, Apr 07, 2017 at 03:04:01PM +0530, 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.
>
> When system enters suspend, some PWM client/slave regulator devices
> require the PWM output to be tristated.
>
> 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 <[email protected]>
>
> ---
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
>
> Changes from V2:
> - Fix the commit message and details
> ---
> .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 45 ++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
Acked-by: Rob Herring <[email protected]>
On Fri, Apr 07, 2017 at 03:03:58PM +0530, Laxman Dewangan wrote:
> 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.
>
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
> - Use API pinctrl_pm_select_*()
>
> Changes from V2:
> - Type fixes, rephrases commit message and use pinctrl_pm_state* return
> value.
>
> 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(-)
All four patches applied to for-4.12/drivers, thanks.
I've slightly modified the commit messages of some patches for "pwm" ->
"PWM".
Thierry
On Fri, Apr 07, 2017 at 03:04:00PM +0530, Laxman Dewangan wrote:
> 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.
>
> Change this to have more precision as:
> hz = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC * 100, period_ns);
> rate = DIV_ROUND_CLOSEST(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 = 59, rate = 3390
> Based on new formula:
> hz = 5951, rate = 3360
>
> The PWM signal rate of 3360 is more near to requested period than 3333.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
>
> ---
> Changes from v1:
> - None
>
> Changes from V2:
> - Fix the commit message with exact formula used.
> ---
> drivers/pwm/pwm-tegra.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 0a688da..21518be 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -76,6 +76,7 @@ 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;
> u32 val = 0;
> int err;
>
> @@ -94,9 +95,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 *= 100;
> + hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
I think hz could overflow for small enough values of period_ns. I've
sent a patch that makes hz unsigned long long. While at it, the patch
also removes the ns100 variable which isn't really necessary here.
Thierry