pinctrl: samsung: add pinctrl_force_sleep() for "sleep" pinctrl state
Subsystems (4)
A message that is not in the archive (sent off-list or not yet received): CGME20170921123230epcas1p4d3c1de108af7d34053d2c4f40dd42119@epcas1p4.samsung.com
PATCH 1/1 pinctrl: samsung: add pinctrl_force_sleep() for "sleep" pinctrl state
This patch adds pinctrl_for_sleep() in samsung_pinctrl_suspend() to support configuration of "sleep" pinctrl state.
For example, we can configure "sleep" pinctrl state of "gpf1-6" as "INPUT, PULL DOWN" by configuring power down mode register.
&pinctrl_5 {
pinctrl-names = "default","sleep";
pinctrl-0 = <&initial5>;
pinctrl-1 = <&sleep5>;
initial5: initial-state {
samsung,pins = gpf1-6;
samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
};
sleep5: sleep-state {
samsung,pins = gpf1-6;
samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_DOWN>;
};
};--- drivers/pinctrl/samsung/pinctrl-samsung.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/pinctrl/samsung/pinctrl-samsung.c +10 -0
@@ -1099,6 +1099,11 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) {struct samsung_pinctrl_drv_data *drvdata =dev_get_drvdata (dev);int i; +int ret; + + ret =pinctrl_force_sleep (drvdata->pctl_dev); +if (ret) +dev_err (dev,"could not set sleep pinstate %d\n" , ret);for (i =0 ; i < drvdata->nr_banks; i++) {struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
@@ -1187,6 +1192,11 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev)if (drvdata->retention_ctrl && drvdata->retention_ctrl->disable) drvdata->retention_ctrl->disable (drvdata); +/* For changing state without writing register. */ +if (!IS_ERR (drvdata->pctl_dev->p) && + !IS_ERR (drvdata->pctl_dev->hog_default)) + drvdata->pctl_dev->p->state = drvdata->pctl_dev->hog_default; +return 0 ; }
Signature · 1 line
2.13.0
PATCH 1/1 pinctrl: samsung: add pinctrl_force_sleep() for "sleep" pinctrl state
Hi Youngmin,
2017-09-21 21:32 GMT+09:00 ??? <[email protected]>:
Quoted 6 lines
This patch adds
pinctrl_for_sleep()insamsung_pinctrl_suspend()to support configuration of "sleep" pinctrl state.For example, we can configure "sleep" pinctrl state of "gpf1-6" as "INPUT, PULL DOWN" by configuring power down mode register.
Quoted 13 lines
&pinctrl_5 {
pinctrl-names = "default","sleep";
pinctrl-0 = <&initial5>;
pinctrl-1 = <&sleep5>;
initial5: initial-state {
samsung,pins = gpf1-6;
samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
};
sleep5: sleep-state {
samsung,pins = gpf1-6;
samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_DOWN>;This isn't a very good example, because pin-con-pdn and pin-pud-pdn are used to program dedicated sleep state registers in the pin controller and they can be programmed at any time. Typically one would include them in "default" state.
As for pins which don't have dedicated sleep state registers, i.e. the alive banks, which retain their value, the DT node of device associated with the pins would have "sleep" pinctrl state and its driver would activate it on suspend.
The only remaining use case would be pins in alive banks not associated with any device, but I can't imagine any practical case that would need to change the value on suspend in such case.
Do you have an example of a real use case for this patch?
Quoted 25 lines
}; };Signed-off-by Youngmin Nam <[email protected]>--- drivers/pinctrl/samsung/pinctrl-samsung.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/pinctrl/samsung/pinctrl-samsung.c +5
-0@@ -1099,6 +1099,11 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) {struct samsung_pinctrl_drv_data *drvdata =dev_get_drvdata (dev);int i; +int ret; + + ret =pinctrl_force_sleep (drvdata->pctl_dev); +if (ret) +dev_err (dev,"could not set sleep pinstate %d\n" , ret);for (i =0 ; i < drvdata->nr_banks; i++) {struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
drivers/pinctrl/samsung/pinctrl-samsung.c static int __maybe_unused samsung_pinctrl_resume(struct device *dev) · line 1198
Quoted 7 lines
drivers/pinctrl/samsung/pinctrl-samsung.c +4
-0@@ -1187,6 +1192,11 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev)if (drvdata->retention_ctrl && drvdata->retention_ctrl->disable) drvdata->retention_ctrl->disable (drvdata); +/* For changing state without writing register. */ +if (!IS_ERR (drvdata->pctl_dev->p) && + !IS_ERR (drvdata->pctl_dev->hog_default)) + drvdata->pctl_dev->p->state = drvdata->pctl_dev->hog_default;
How is this supposed to work? The pins were reconfigured at suspend, where are they restored back to the values before suspend?
Best regards,
Tomasz
PATCH 1/1 pinctrl: samsung: add pinctrl_force_sleep() for "sleep" pinctrl state
Quoted 4 lines · Tomasz Figa
This isn't a very good example, because pin-con-pdn and pin-pud-pdn are used to program dedicated sleep state registers in the pin controller and they can be programmed at any time. Typically one would include them in "default" state.
Yeah. Baolin added a special flag "sleep-hardware-state" to indicate these special states, so the pin control driver can set them up at first state instantiation.
Quoted 4 lines
As for pins which don't have dedicated sleep state registers, i.e. the alive banks, which retain their value, the DT node of device associated with the pins would have "sleep" pinctrl state and its driver would activate it on suspend.
Correct.
Quoted 3 lines
The only remaining use case would be pins in alive banks not associated with any device, but I can't imagine any practical case that would need to change the value on suspend in such case.
I think those could be made to work with hogs.
What also came up recently was pin controllers that per se loose their state when the system suspends, so the whole pin controller is kind of power-cycled and comes up in some default state.
Florian is looking into a generic solution to that problem.
Yours,
Linus Walleij
PATCH 1/1 pinctrl: samsung: add pinctrl_force_sleep() for "sleep" pinctrl state
In reply to 남영민On Thu, Sep 21, 2017 at 2:32 PM, ??? <[email protected]> wrote:
I'm copying the whole patch so that everyone added to To: can see it.
Quoted 44 lines
This patch adds
pinctrl_for_sleep()insamsung_pinctrl_suspend()to support configuration of "sleep" pinctrl state.For example, we can configure "sleep" pinctrl state of "gpf1-6" as "INPUT, PULL DOWN" by configuring power down mode register.
&pinctrl_5 { pinctrl-names = "default","sleep"; pinctrl-0 = <&initial5>; pinctrl-1 = <&sleep5>; initial5: initial-state { samsung,pins = gpf1-6; samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>; }; sleep5: sleep-state { samsung,pins = gpf1-6; samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>; samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_DOWN>; }; };Signed-off-by Youngmin Nam <[email protected]>--- drivers/pinctrl/samsung/pinctrl-samsung.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/pinctrl/samsung/pinctrl-samsung.c +5
-0@@ -1099,6 +1099,11 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) {struct samsung_pinctrl_drv_data *drvdata =dev_get_drvdata (dev);int i; +int ret; + + ret =pinctrl_force_sleep (drvdata->pctl_dev); +if (ret) +dev_err (dev,"could not set sleep pinstate %d\n" , ret);for (i =0 ; i < drvdata->nr_banks; i++) {struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
drivers/pinctrl/samsung/pinctrl-samsung.c static int __maybe_unused samsung_pinctrl_resume(struct device *dev) · line 1201
Quoted 10 lines
drivers/pinctrl/samsung/pinctrl-samsung.c +5
-0@@ -1187,6 +1192,11 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev)if (drvdata->retention_ctrl && drvdata->retention_ctrl->disable) drvdata->retention_ctrl->disable (drvdata); +/* For changing state without writing register. */ +if (!IS_ERR (drvdata->pctl_dev->p) && + !IS_ERR (drvdata->pctl_dev->hog_default)) + drvdata->pctl_dev->p->state = drvdata->pctl_dev->hog_default; +return 0 ; }
Oddly this business of forcing the hardware into different "sleep" states is coming up from the left and right at the moment.
Please check a bit at:
commit 6606bc9dee63 ("pinctrl: Add sleep related state to indicate sleep related configs")
and how the Spreadtrum driver handles this.
That is for the case where the hardware autonomously defines a sleep state that need to be programmed from somewhere.
This seems to be what this hardware needs to use?
Yours,
Linus Walleij