Skip to contentSkip to conversation

gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

PATCH New Axel Lin 2 participants 5 messages Jun 9, 2015 GPIO

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

Pass correct gpio argument to gpio_set_value().
The calcualation of gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT); is required for calling imx_iomux_gpio_* functions so move them close to improve readability.

Signed-off-by Axel Lin <[email protected]>

---
Hi Bhuvanchandra,
I think current code does not pass correct gpio argument to gpio_set_value() in vybrid_gpio_direction_output(). It only works if gpios->chip is 0. I don't have the h/w to test, can you double check this?
Thanks,
Axel

 drivers/gpio/vybrid_gpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/gpio/vybrid_gpio.c +3 -2
index 6eaf0a9..33bbf54 100644
@@ -36,8 +36,9 @@ static int vybrid_gpio_direction_output(struct udevice *dev, unsigned gpio,
 {
 	const struct vybrid_gpios *gpios = dev_get_priv(dev);
 
-	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
 	gpio_set_value(gpio, value);
+
+	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
 	imx_iomux_gpio_set_direction(gpio, VF610_GPIO_DIRECTION_OUT);
 
 	return 0;

@@ -54,6 +55,7 @@ static int vybrid_gpio_set_value(struct udevice *dev, unsigned gpio,
 				  int value)
 {
 	const struct vybrid_gpios *gpios = dev_get_priv(dev);
+
 	if (value)
 		writel((1 << gpio), &gpios->reg->gpio_psor);
 	else

@@ -68,7 +70,6 @@ static int vybrid_gpio_get_function(struct udevice *dev, unsigned gpio)
 	u32 g_state = 0;
 
 	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
-
 	imx_iomux_gpio_get_function(gpio, &g_state);
 
 	if (((g_state & (0x07 << PAD_MUX_MODE_SHIFT)) >> PAD_MUX_MODE_SHIFT) > 0)
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/1 gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

Hello Axel,

Quoted 11 lines · Axel Lin

Pass correct gpio argument to gpio_set_value().
The calcualation of gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT); is required for calling imx_iomux_gpio_* functions so move them close to improve readability.

Signed-off-by Axel Lin <[email protected]>

---
Hi Bhuvanchandra,
I think current code does not pass correct gpio argument to gpio_set_value() in vybrid_gpio_direction_output(). It only works if gpios->chip is 0. I don't have the h/w to test, can you double check this?

gpio_set_value() needs the actual gpio number to be passed not the gpio offset of gpio chip.

The calculation is for getting the actual gpio number from gpio offset, which we will need for both imx_iomux_gpio* and gpio_*_* functions.

Tested on Colibri VF50 and VF61 modules.

Quoted 30 lines

Thanks,
Axel

  drivers/gpio/vybrid_gpio.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
drivers/gpio/vybrid_gpio.c +3 -1
index 6eaf0a9..33bbf54 100644
@@ -36,8 +36,9 @@ static int vybrid_gpio_direction_output(struct udevice *dev, unsigned gpio,
  {
  	const struct vybrid_gpios *gpios = dev_get_priv(dev);
 
-	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
  	gpio_set_value(gpio, value);
+
+	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
  	imx_iomux_gpio_set_direction(gpio, VF610_GPIO_DIRECTION_OUT);
 
  	return 0;

@@ -54,6 +55,7 @@ static int vybrid_gpio_set_value(struct udevice *dev, unsigned gpio,
  				  int value)
  {
  	const struct vybrid_gpios *gpios = dev_get_priv(dev);
+
  	if (value)
  		writel((1 << gpio), &gpios->reg->gpio_psor);
  	else

drivers/gpio/vybrid_gpio.c static int vybrid_gpio_get_function(struct udevice *dev, unsigned gpio) · line 75

Quoted 7 lines
drivers/gpio/vybrid_gpio.c +0 -1
@@ -68,7 +70,6 @@ static int vybrid_gpio_get_function(struct udevice *dev, unsigned gpio)
  	u32 g_state = 0;
 
  	gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT);
-
  	imx_iomux_gpio_get_function(gpio, &g_state);
 
  	if (((g_state & (0x07 << PAD_MUX_MODE_SHIFT)) >> PAD_MUX_MODE_SHIFT) > 0)

Best regards,
Bhuvan

author
#2 lore raw

PATCH 1/1 gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV <[email protected]>:

Quoted 17 lines

Hello Axel,

Quoted 6 lines · Axel Lin

Pass correct gpio argument to gpio_set_value().
The calcualation of gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT); is required for calling imx_iomux_gpio_* functions so move them close to improve readability.

Quoted 7 lines
Signed-off-by Axel Lin <[email protected]>

---
Hi Bhuvanchandra,
I think current code does not pass correct gpio argument to gpio_set_value()
in vybrid_gpio_direction_output(). It only works if gpios->chip is 0. I don't have the h/w to test, can you double check this?

Quoted 2 lines

gpio_set_value() needs the actual gpio number to be passed not the gpio offset of gpio chip.

Are you sure?
Please take a look at gpio_get_value()/gpio_set_value() implement in drivers/gpio/gpio-uclass.c.

#3 lore raw

PATCH 1/1 gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

Quoted 18 lines · Axel Lin

2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV <[email protected]>:

Quoted 17 lines

Hello Axel,

Quoted 13 lines · Axel Lin

Pass correct gpio argument to gpio_set_value().
The calcualation of gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT); is required for calling imx_iomux_gpio_* functions so move them close to improve readability.

Signed-off-by Axel Lin <[email protected]>

---
Hi Bhuvanchandra,
I think current code does not pass correct gpio argument to gpio_set_value()
in vybrid_gpio_direction_output(). It only works if gpios->chip is 0. I don't have the h/w to test, can you double check this?

Quoted 6 lines
Quoted 2 lines

gpio_set_value() needs the actual gpio number to be passed not the gpio offset of gpio chip.

Are you sure?
Please take a look at gpio_get_value()/gpio_set_value() implement in drivers/gpio/gpio-uclass.c.

Yes,

Toggling GPIO_34 => GPIO_01 2nd instance, with debug prints:

Colibri VFxx # gpio toggle 34
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: vybrid_gpio_direction_output: gpio(before calculation): 2
DEBUG: vybrid_gpio_direction_output: gpio(after calculation): 34
DEBUG: gpio_set_value: gpio: 34
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: dm_gpio_set_value: gpio_offset: 2
gpio: pin 34 (gpio 34) value is 0
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
Colibri VFxx # gpio toggle 34
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: vybrid_gpio_direction_output: gpio(before calculation): 2
DEBUG: vybrid_gpio_direction_output: gpio(after calculation): 34
DEBUG: gpio_set_value: gpio: 34
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2
DEBUG: dm_gpio_set_value: desc_offset: 2
gpio: pin 34 (gpio 34) value is 1
DEBUG: gpio_to_device: gpio: 34
DEBUG: gpio_to_device: desc_offset: 2

Quoted 1 line

Best regards,
Bhuvan

author
#4 lore raw

PATCH 1/1 gpio: vybrid: Fix up setting output value in vybrid_gpio_direction_output

2015-06-09 21:53 GMT+08:00 Bhuvanchandra DV <[email protected]>:

Quoted 28 lines · Axel Lin
Quoted 27 lines

2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV
<[email protected]>:

Quoted 22 lines

Hello Axel,

Quoted 7 lines · Axel Lin

Pass correct gpio argument to gpio_set_value().
The calcualation of gpio = gpio + (gpios->chip * VYBRID_GPIO_COUNT); is required for calling imx_iomux_gpio_* functions so move them close to improve readability.

Quoted 7 lines
Signed-off-by Axel Lin <[email protected]>

---
Hi Bhuvanchandra,
I think current code does not pass correct gpio argument to gpio_set_value()
in vybrid_gpio_direction_output(). It only works if gpios->chip is 0. I don't have the h/w to test, can you double check this?

gpio_set_value() needs the actual gpio number to be passed not the gpio offset of gpio chip.

Quoted 5 lines
Quoted 3 lines

Are you sure?
Please take a look at gpio_get_value()/gpio_set_value() implement in drivers/gpio/gpio-uclass.c.

Yes,

Ah, my fault actually.
Sorry for the noise.

Regards,
Axel