2015-04-09 15:18:53

by Ivan T. Ivanov

Subject: [PATCH 1/3] pinctrl: qcom-spmi-gpio: Fix output type configuration

GPIO output type configuration was incorrectly overwritten
by strength value. Fix this.

Signed-off-by: Ivan T. Ivanov <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index b2d2221..4b21aac 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -417,7 +417,7 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
return ret;

val = pad->buffer_type << PMIC_GPIO_REG_OUT_TYPE_SHIFT;
- val = pad->strength << PMIC_GPIO_REG_OUT_STRENGTH_SHIFT;
+ val |= pad->strength << PMIC_GPIO_REG_OUT_STRENGTH_SHIFT;

ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_DIG_OUT_CTL, val);
if (ret < 0)
--
1.9.1

2015-04-09 15:18:52

by Ivan T. Ivanov

Subject: [PATCH 2/3] pinctrl: qcom-spmi-gpio: Fix input value report

Read input buffer when input is enabled, not when it is
disabled. Also fix interpretation of the pmic_gpio_read()
return code, negative value means an error.

Signed-off-by: Ivan T. Ivanov <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 4b21aac..e8b74c6 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -466,12 +466,13 @@ static void pmic_gpio_config_dbg_show(struct pinctrl_dev *pctldev,
seq_puts(s, " ---");
} else {

- if (!pad->input_enabled) {
+ if (pad->input_enabled) {
ret = pmic_gpio_read(state, pad, PMIC_MPP_REG_RT_STS);
- if (!ret) {
- ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
- pad->out_value = ret;
- }
+ if (ret < 0)
+ return;
+
+ ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
+ pad->out_value = ret;
}

seq_printf(s, " %-4s", pad->output_enabled ? "out" : "in");
--
1.9.1

2015-04-09 15:18:51

by Ivan T. Ivanov

Subject: [PATCH 3/3] pinctrl: qcom-spmi-mpp: Fix input value report

Fix interpretation of the pmic_mpp_read() return code,
negative value means an error.

Signed-off-by: Ivan T. Ivanov <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
index 8f36c5f..8f6c7be 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
@@ -576,10 +576,11 @@ static void pmic_mpp_config_dbg_show(struct pinctrl_dev *pctldev,

if (pad->input_enabled) {
ret = pmic_mpp_read(state, pad, PMIC_MPP_REG_RT_STS);
- if (!ret) {
- ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
- pad->out_value = ret;
- }
+ if (ret < 0)
+ return;
+
+ ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
+ pad->out_value = ret;
}

seq_printf(s, " %-4s", pad->output_enabled ? "out" : "in");
--
1.9.1

2015-04-09 15:53:28

by Bjorn Andersson

Subject: Re: [PATCH 1/3] pinctrl: qcom-spmi-gpio: Fix output type configuration

On Thu 09 Apr 08:18 PDT 2015, Ivan T. Ivanov wrote:

> GPIO output type configuration was incorrectly overwritten
> by strength value. Fix this.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>
> ---

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

Regards,
Bjorn

2015-04-09 16:01:24

by Bjorn Andersson

Subject: Re: [PATCH 2/3] pinctrl: qcom-spmi-gpio: Fix input value report

On Thu 09 Apr 08:18 PDT 2015, Ivan T. Ivanov wrote:

> Read input buffer when input is enabled, not when it is
> disabled. Also fix interpretation of the pmic_gpio_read()
> return code, negative value means an error.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>
> ---

> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index 4b21aac..e8b74c6 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -466,12 +466,13 @@ static void pmic_gpio_config_dbg_show(struct pinctrl_dev *pctldev,
> seq_puts(s, " ---");
> } else {
>
> - if (!pad->input_enabled) {
> + if (pad->input_enabled) {
> ret = pmic_gpio_read(state, pad, PMIC_MPP_REG_RT_STS);
> - if (!ret) {
> - ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
> - pad->out_value = ret;
> - }
> + if (ret < 0)
> + return;
> +
> + ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
> + pad->out_value = ret;

I'm not too happy on pmic_gpio_read() returning the register value as an
signed int, sorry for not picking on this back when we merged the
driver.

As far as I can see bit 31 is reserved in all registers in the block
though, so there shouldn't be any practical issues with this _today_.

But please consider revising pmic_gpio_read() to take a u32* parameter
for the data instead.

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

Regards,
Bjorn

2015-04-09 16:04:06

by Bjorn Andersson

Subject: Re: [PATCH 3/3] pinctrl: qcom-spmi-mpp: Fix input value report

On Thu 09 Apr 08:18 PDT 2015, Ivan T. Ivanov wrote:

> Fix interpretation of the pmic_mpp_read() return code,
> negative value means an error.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>
> ---
> drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> index 8f36c5f..8f6c7be 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-mpp.c
> @@ -576,10 +576,11 @@ static void pmic_mpp_config_dbg_show(struct pinctrl_dev *pctldev,
>
> if (pad->input_enabled) {
> ret = pmic_mpp_read(state, pad, PMIC_MPP_REG_RT_STS);
> - if (!ret) {
> - ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
> - pad->out_value = ret;
> - }
> + if (ret < 0)
> + return;
> +
> + ret &= PMIC_MPP_REG_RT_STS_VAL_MASK;
> + pad->out_value = ret;

Same as with the gpio driver.

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

Regards,
Bjrn

2015-04-28 12:48:35

by Linus Walleij

Subject: Re: [PATCH 1/3] pinctrl: qcom-spmi-gpio: Fix output type configuration

On Thu, Apr 9, 2015 at 5:18 PM, Ivan T. Ivanov <[email protected]> wrote:

> GPIO output type configuration was incorrectly overwritten
> by strength value. Fix this.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>

Patch applied for fixes.

Yours,
Linus Walleij

2015-04-28 12:49:56

by Linus Walleij

Subject: Re: [PATCH 2/3] pinctrl: qcom-spmi-gpio: Fix input value report

On Thu, Apr 9, 2015 at 5:18 PM, Ivan T. Ivanov <[email protected]> wrote:

> Read input buffer when input is enabled, not when it is
> disabled. Also fix interpretation of the pmic_gpio_read()
> return code, negative value means an error.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>

Patch applied for fixes.

Yours,
Linus Walleij

2015-04-28 12:53:31

by Linus Walleij

Subject: Re: [PATCH 3/3] pinctrl: qcom-spmi-mpp: Fix input value report

On Thu, Apr 9, 2015 at 5:18 PM, Ivan T. Ivanov <[email protected]> wrote:

> Fix interpretation of the pmic_mpp_read() return code,
> negative value means an error.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>

Patch applied for fixes.

Yours,
Linus Walleij