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

Groups > linux.kernel > #1594543 > unrolled thread

[PATCH fixes v4] pinctrl: Do not check previous and current state

Back to article view | Back to linux.kernel

Contents

  [PATCH fixes v4] pinctrl: Do not check previous and current state Florian Fainelli <f.fainelli@gmail.com> - 2017-03-07 20:00 +0100
    Re: [PATCH fixes v4] pinctrl: Do not check previous and current state Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-13 20:10 +0100
      Re: [PATCH fixes v4] pinctrl: Do not check previous and current state Linus Walleij <linus.walleij@linaro.org> - 2017-03-15 11:30 +0100
    Re: [PATCH fixes v4] pinctrl: Do not check previous and current state Florian Fainelli <f.fainelli@gmail.com> - 2017-03-13 20:10 +0100
    Re: [PATCH fixes v4] pinctrl: Do not check previous and current state Linus Walleij <linus.walleij@linaro.org> - 2017-03-15 11:30 +0100

#1594543 — [PATCH fixes v4] pinctrl: Do not check previous and current state

In case a platform only defaults a "default" set of pins, but not a
"sleep" set of pins, and this particular platform suspends and resumes
in a way that the pin states are not preserved by the hardware, when we
resume, we would call pinctrl_single_resume() -> pinctrl_force_default()
-> pinctrl_select_state() and the first thing we do is check that the
pins state is the same as before, and do nothing.

In order to fix this, just remove the p->state == state check from
pinctrl_select_state() since it would not allow callers of this function
to get the pins to be brought into the expected state.

Fixes: 6e5e959dde0d ("pinctrl: API changes to support multiple states per device")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v4:

- just remove the p->state == state check because it cannot allow pinctrl_select_state
  to work for callers that expect it to do the actual state change

Changes in v3:

- move the state check to pinctrl_select_state

Changes in v2:

- rename __pinctrl_select_state to pinctrl_commit_state

 drivers/pinctrl/core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index d69046537b75..33cef0a65c9c 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1203,9 +1203,6 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 	struct pinctrl_state *old_state = p->state;
 	int ret;
 
-	if (p->state == state)
-		return 0;
-
 	if (p->state) {
 		/*
 		 * For each pinmux setting in the old state, forget SW's record
-- 
2.9.3

[toc] | [next] | [standalone]

#1599696

On Mon, Mar 13, 2017 at 8:59 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 03/07/2017 10:52 AM, Florian Fainelli wrote:

> Linus am I hitting some of your spam folder, or you are really having
> way too much fun with Gemini ;) ?

A bit offtopic here, but I have almost same question. I noticed no
reaction for patches I had sent up to 3 weeks ago (yes, I understand
that far was a time of merge window, though...).

-- 
With Best Regards,
Andy Shevchenko

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

#1601240

On Mon, Mar 13, 2017 at 8:06 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Mar 13, 2017 at 8:59 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 03/07/2017 10:52 AM, Florian Fainelli wrote:
>
>> Linus am I hitting some of your spam folder, or you are really having
>> way too much fun with Gemini ;) ?
>
> A bit offtopic here, but I have almost same question. I noticed no
> reaction for patches I had sent up to 3 weeks ago (yes, I understand
> that far was a time of merge window, though...).

Yeah it's official, I'm overloaded.

I need to find a pinctrl comaintainer, I have Geert pulling together
Renesas patches, and I asked someone from Samsung to step up
for their stuff as it is creating a bit of stir.

For helping out with pinctrl core I am open to nominations :)

Yours,
Linus Walleij

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

#1599703

On 03/07/2017 10:52 AM, Florian Fainelli wrote:
> In case a platform only defaults a "default" set of pins, but not a
> "sleep" set of pins, and this particular platform suspends and resumes
> in a way that the pin states are not preserved by the hardware, when we
> resume, we would call pinctrl_single_resume() -> pinctrl_force_default()
> -> pinctrl_select_state() and the first thing we do is check that the
> pins state is the same as before, and do nothing.
> 
> In order to fix this, just remove the p->state == state check from
> pinctrl_select_state() since it would not allow callers of this function
> to get the pins to be brought into the expected state.
> 
> Fixes: 6e5e959dde0d ("pinctrl: API changes to support multiple states per device")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---

Linus am I hitting some of your spam folder, or you are really having
way too much fun with Gemini ;) ?

> Changes in v4:
> 
> - just remove the p->state == state check because it cannot allow pinctrl_select_state
>   to work for callers that expect it to do the actual state change
> 
> Changes in v3:
> 
> - move the state check to pinctrl_select_state
> 
> Changes in v2:
> 
> - rename __pinctrl_select_state to pinctrl_commit_state
> 
>  drivers/pinctrl/core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index d69046537b75..33cef0a65c9c 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -1203,9 +1203,6 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
>  	struct pinctrl_state *old_state = p->state;
>  	int ret;
>  
> -	if (p->state == state)
> -		return 0;
> -
>  	if (p->state) {
>  		/*
>  		 * For each pinmux setting in the old state, forget SW's record
> 


-- 
Florian

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

#1601243

On Tue, Mar 7, 2017 at 7:52 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:

> In case a platform only defaults a "default" set of pins, but not a
> "sleep" set of pins, and this particular platform suspends and resumes
> in a way that the pin states are not preserved by the hardware, when we
> resume, we would call pinctrl_single_resume() -> pinctrl_force_default()
> -> pinctrl_select_state() and the first thing we do is check that the
> pins state is the same as before, and do nothing.
>
> In order to fix this, just remove the p->state == state check from
> pinctrl_select_state() since it would not allow callers of this function
> to get the pins to be brought into the expected state.
>
> Fixes: 6e5e959dde0d ("pinctrl: API changes to support multiple states per device")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

I responded to some patch in the series yesterday that what we
need is to inform the pinctrl core that we lost state, so let's discuss
this in that thread.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web