2018-10-01 21:49:47

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH] pinctrl: msm: Actually use function 0 for gpio selection

This code needs to select function #0, which is the first int in the
array of functions, not the number 0 which may or may not be the
function for "GPIO mode" per the enum mapping. We were getting lucky on
SDM845, where this was tested, because the function 0 matched the enum
value for "GPIO mode". On other platforms, e.g. MSM8996, the gpio enum
value is the last one in the list so this code doesn't work and we see a
warning at boot. Fix it by grabbing the first element out of the array
of functions.

Cc: Doug Anderson <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Niklas Cassel <[email protected]>
Reported-by: Niklas Cassel <[email protected]>
Fixes: 1de7ddb3a15c ("pinctrl: msm: Mux out gpio function with gpio_request()")
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 1684b2da09d5..b925b8feac95 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -188,7 +188,7 @@ static int msm_pinmux_request_gpio(struct pinctrl_dev *pctldev,
return 0;

/* For now assume function 0 is GPIO because it always is */
- return msm_pinmux_set_mux(pctldev, 0, offset);
+ return msm_pinmux_set_mux(pctldev, g->funcs[0], offset);
}

static const struct pinmux_ops msm_pinmux_ops = {
--
Sent by a computer through tubes



2018-10-02 03:50:43

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: msm: Actually use function 0 for gpio selection

On Mon 01 Oct 14:49 PDT 2018, Stephen Boyd wrote:

> This code needs to select function #0, which is the first int in the
> array of functions, not the number 0 which may or may not be the
> function for "GPIO mode" per the enum mapping. We were getting lucky on
> SDM845, where this was tested, because the function 0 matched the enum
> value for "GPIO mode". On other platforms, e.g. MSM8996, the gpio enum
> value is the last one in the list so this code doesn't work and we see a
> warning at boot. Fix it by grabbing the first element out of the array
> of functions.
>
> Cc: Doug Anderson <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Niklas Cassel <[email protected]>
> Reported-by: Niklas Cassel <[email protected]>
> Fixes: 1de7ddb3a15c ("pinctrl: msm: Mux out gpio function with gpio_request()")
> Signed-off-by: Stephen Boyd <[email protected]>

Oops...

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 1684b2da09d5..b925b8feac95 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -188,7 +188,7 @@ static int msm_pinmux_request_gpio(struct pinctrl_dev *pctldev,
> return 0;
>
> /* For now assume function 0 is GPIO because it always is */
> - return msm_pinmux_set_mux(pctldev, 0, offset);
> + return msm_pinmux_set_mux(pctldev, g->funcs[0], offset);
> }
>
> static const struct pinmux_ops msm_pinmux_ops = {
> --
> Sent by a computer through tubes
>

2018-10-02 08:48:13

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: msm: Actually use function 0 for gpio selection

On Mon, Oct 01, 2018 at 02:49:05PM -0700, Stephen Boyd wrote:
> This code needs to select function #0, which is the first int in the
> array of functions, not the number 0 which may or may not be the
> function for "GPIO mode" per the enum mapping. We were getting lucky on
> SDM845, where this was tested, because the function 0 matched the enum
> value for "GPIO mode". On other platforms, e.g. MSM8996, the gpio enum
> value is the last one in the list so this code doesn't work and we see a
> warning at boot. Fix it by grabbing the first element out of the array
> of functions.
>
> Cc: Doug Anderson <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Niklas Cassel <[email protected]>
> Reported-by: Niklas Cassel <[email protected]>
> Fixes: 1de7ddb3a15c ("pinctrl: msm: Mux out gpio function with gpio_request()")
> Signed-off-by: Stephen Boyd <[email protected]>

Tested-by: Niklas Cassel <[email protected]>

2018-10-02 08:51:41

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: msm: Actually use function 0 for gpio selection

On Mon, Oct 1, 2018 at 11:49 PM Stephen Boyd <[email protected]> wrote:

> This code needs to select function #0, which is the first int in the
> array of functions, not the number 0 which may or may not be the
> function for "GPIO mode" per the enum mapping. We were getting lucky on
> SDM845, where this was tested, because the function 0 matched the enum
> value for "GPIO mode". On other platforms, e.g. MSM8996, the gpio enum
> value is the last one in the list so this code doesn't work and we see a
> warning at boot. Fix it by grabbing the first element out of the array
> of functions.
>
> Cc: Doug Anderson <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Niklas Cassel <[email protected]>
> Reported-by: Niklas Cassel <[email protected]>
> Fixes: 1de7ddb3a15c ("pinctrl: msm: Mux out gpio function with gpio_request()")
> Signed-off-by: Stephen Boyd <[email protected]>

Patch applied with all the tags.

Yours,
Linus Walleij