[PATCH 0/2] pinctrl: ABX500: fix

[PATCH 0/2] pinctrl: ABX500: fix

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

Patrice Chotard (2):
  pinctrl: abx500: suppress hardcoded value
  pinctrl: abx500: fix abx500_gpio_get()

 drivers/pinctrl/pinctrl-abx500.c       |   34 ++++++++++++++++++++++++++------
 include/linux/mfd/abx500/ab8500-gpio.h |    5 +++++
 2 files changed, 33 insertions(+), 6 deletions(-)

-- 
1.7.10

[PATCH 1/2] pinctrl: abx500: suppress hardcoded value

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

Replace hardcoded value by corresponding #define's.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/pinctrl/pinctrl-abx500.c       |   18 ++++++++++++++----
 include/linux/mfd/abx500/ab8500-gpio.h |    5 +++++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
index 14dc078..4307b0f 100644
--- a/drivers/pinctrl/pinctrl-abx500.c
+++ b/drivers/pinctrl/pinctrl-abx500.c
@@ -264,12 +264,18 @@ static int abx500_gpio_direction_output(struct gpio_chip *chip,
 	int ret;
 
 	/* set direction as output */
-	ret = abx500_gpio_set_bits(chip, AB8500_GPIO_DIR1_REG, offset, 1);
+	ret = abx500_gpio_set_bits(chip,
+				AB8500_GPIO_DIR1_REG,
+				offset,
+				ABX500_GPIO_OUTPUT);
 	if (ret < 0)
 		return ret;
 
 	/* disable pull down */
-	ret = abx500_gpio_set_bits(chip, AB8500_GPIO_PUD1_REG, offset, 1);
+	ret = abx500_gpio_set_bits(chip,
+				AB8500_GPIO_PUD1_REG,
+				offset,
+				ABX500_GPIO_PULL_NONE);
 	if (ret < 0)
 		return ret;
 
@@ -290,7 +296,10 @@ static int abx500_gpio_direction_output(struct gpio_chip *chip,
 static int abx500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 {
 	/* set the register as input */
-	return abx500_gpio_set_bits(chip, AB8500_GPIO_DIR1_REG, offset, 0);
+	return abx500_gpio_set_bits(chip,
+				AB8500_GPIO_DIR1_REG,
+				offset,
+				ABX500_GPIO_INPUT);
 }
 
 static int abx500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
@@ -1018,7 +1027,8 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
 		else
 			/* Chip only supports pull down */
 			ret = abx500_gpio_set_bits(chip, AB8500_GPIO_PUD1_REG,
-				offset, argument ? 0 : 1);
+				offset,
+				argument ? ABX500_GPIO_PULL_DOWN : ABX500_GPIO_PULL_NONE);
 		break;
 
 	case PIN_CONFIG_BIAS_PULL_UP:
diff --git a/include/linux/mfd/abx500/ab8500-gpio.h b/include/linux/mfd/abx500/ab8500-gpio.h
index 172b2f2..dafeca4 100644
--- a/include/linux/mfd/abx500/ab8500-gpio.h
+++ b/include/linux/mfd/abx500/ab8500-gpio.h
@@ -30,4 +30,9 @@ enum abx500_gpio_vinsel {
 	ABX500_GPIO_VINSEL_VDD_BIF = 0x2,
 };
 
+enum abx500_gpio_direction {
+	ABX500_GPIO_INPUT = 0x0,
+	ABX500_GPIO_OUTPUT = 0x1,
+};
+
 #endif /* _AB8500_GPIO_H */
-- 
1.7.10

[PATCH 2/2] pinctrl: abx500: fix abx500_gpio_get()

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

_ allow to get output GPIO value.
_ as there is no GPIO0 on ABX500, use correct offset with 
  abx500_gpio_get_bit().

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/pinctrl/pinctrl-abx500.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
index 4307b0f..070442d 100644
--- a/drivers/pinctrl/pinctrl-abx500.c
+++ b/drivers/pinctrl/pinctrl-abx500.c
@@ -162,10 +162,22 @@ static int abx500_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct abx500_pinctrl *pct = to_abx500_pinctrl(chip);
 	bool bit;
+	bool is_out;
+	u8 gpio_offset = offset - 1;
 	int ret;
 
-	ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
-				  offset, &bit);
+	ret = abx500_gpio_get_bit(chip, AB8500_GPIO_DIR1_REG, gpio_offset, &is_out);
+	if (ret < 0) {
+		dev_err(pct->dev, "%s failed\n", __func__);
+		return ret;
+	}
+
+	if (is_out)
+		ret = abx500_gpio_get_bit(chip, AB8500_GPIO_OUT1_REG,
+				gpio_offset, &bit);
+	else
+		ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
+				gpio_offset, &bit);
 	if (ret < 0) {
 		dev_err(pct->dev, "%s failed\n", __func__);
 		return ret;
-- 
1.7.10

Re: [PATCH 2/2] pinctrl: abx500: fix abx500_gpio_get()

[Fabio Baltieri]

Hi Patrice,

On Thu, Jun 20, 2013 at 09:24:44AM +0200, patrice.chotard.st@gmail.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> _ allow to get output GPIO value.
> _ as there is no GPIO0 on ABX500, use correct offset with 
>   abx500_gpio_get_bit().
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/pinctrl/pinctrl-abx500.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
> index 4307b0f..070442d 100644
> --- a/drivers/pinctrl/pinctrl-abx500.c
> +++ b/drivers/pinctrl/pinctrl-abx500.c
> @@ -162,10 +162,22 @@ static int abx500_gpio_get(struct gpio_chip *chip, unsigned offset)
>  {
>  	struct abx500_pinctrl *pct = to_abx500_pinctrl(chip);
>  	bool bit;
> +	bool is_out;
> +	u8 gpio_offset = offset - 1;
>  	int ret;
>  
> -	ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
> -				  offset, &bit);
> +	ret = abx500_gpio_get_bit(chip, AB8500_GPIO_DIR1_REG, gpio_offset, &is_out);
> +	if (ret < 0) {
> +		dev_err(pct->dev, "%s failed\n", __func__);
> +		return ret;
> +	}
> +
> +	if (is_out)
> +		ret = abx500_gpio_get_bit(chip, AB8500_GPIO_OUT1_REG,
> +				gpio_offset, &bit);
> +	else
> +		ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
> +				gpio_offset, &bit);

Why would you want to read the pin state from the output register?  The
input one should be the one that reflect the real electrical value of
the pin, and I think it may be useful to detect some fault condition
too...  Is there a specific reasion to use the output register instead?

Thanks,
Fabio

>  	if (ret < 0) {
>  		dev_err(pct->dev, "%s failed\n", __func__);
>  		return ret;
> -- 
> 1.7.10
> 

-- 
Fabio Baltieri

Re: [PATCH 2/2] pinctrl: abx500: fix abx500_gpio_get()

[Patrice Chotard]

On Thu, Jun 20, 2013 at 11:00 AM, Fabio Baltieri
<fabio.baltieri@linaro.org> wrote:
> Hi Patrice,
>
> On Thu, Jun 20, 2013 at 09:24:44AM +0200, patrice.chotard.st@gmail.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> _ allow to get output GPIO value.
>> _ as there is no GPIO0 on ABX500, use correct offset with
>>   abx500_gpio_get_bit().
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>  drivers/pinctrl/pinctrl-abx500.c |   16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
>> index 4307b0f..070442d 100644
>> --- a/drivers/pinctrl/pinctrl-abx500.c
>> +++ b/drivers/pinctrl/pinctrl-abx500.c
>> @@ -162,10 +162,22 @@ static int abx500_gpio_get(struct gpio_chip *chip, unsigned offset)
>>  {
>>       struct abx500_pinctrl *pct = to_abx500_pinctrl(chip);
>>       bool bit;
>> +     bool is_out;
>> +     u8 gpio_offset = offset - 1;
>>       int ret;
>>
>> -     ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
>> -                               offset, &bit);
>> +     ret = abx500_gpio_get_bit(chip, AB8500_GPIO_DIR1_REG, gpio_offset, &is_out);
>> +     if (ret < 0) {
>> +             dev_err(pct->dev, "%s failed\n", __func__);
>> +             return ret;
>> +     }
>> +
>> +     if (is_out)
>> +             ret = abx500_gpio_get_bit(chip, AB8500_GPIO_OUT1_REG,
>> +                             gpio_offset, &bit);
>> +     else
>> +             ret = abx500_gpio_get_bit(chip, AB8500_GPIO_IN1_REG,
>> +                             gpio_offset, &bit);
>
> Why would you want to read the pin state from the output register?  The
> input one should be the one that reflect the real electrical value of
> the pin, and I think it may be useful to detect some fault condition
> too...  Is there a specific reasion to use the output register instead?
>
Hi Fabio

I want to read the pin state from the output register for debugfs print out.
In abx500_gpio_dbg_show_one(), chip->get() is used to retrieve the output
pin's level.

Patrice

> Thanks,
> Fabio
>
>>       if (ret < 0) {
>>               dev_err(pct->dev, "%s failed\n", __func__);
>>               return ret;
>> --
>> 1.7.10
>>
>
> --
> Fabio Baltieri