| Message ID | 1433841302.4157.1.camel@ingics.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
Hello Axel, On 06/09/2015 02:45 PM, Axel Lin wrote: > 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 <axel.lin@ingics.com> > --- > 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. > Thanks, > Axel > drivers/gpio/vybrid_gpio.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/vybrid_gpio.c b/drivers/gpio/vybrid_gpio.c > index 6eaf0a9..33bbf54 100644 > --- a/drivers/gpio/vybrid_gpio.c > +++ b/drivers/gpio/vybrid_gpio.c > @@ -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) > Best regards, Bhuvan
2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>: > Hello Axel, > > On 06/09/2015 02:45 PM, Axel Lin wrote: >> >> 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 <axel.lin@ingics.com> >> --- >> 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. Are you sure? Please take a look at gpio_get_value()/gpio_set_value() implement in drivers/gpio/gpio-uclass.c.
On 06/09/2015 06:49 PM, Axel Lin wrote: > 2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>: >> Hello Axel, >> >> On 06/09/2015 02:45 PM, Axel Lin wrote: >>> >>> 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 <axel.lin@ingics.com> >>> --- >>> 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. > > 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 > Best regards, Bhuvan
2015-06-09 21:53 GMT+08:00 Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>: > On 06/09/2015 06:49 PM, Axel Lin wrote: >> >> 2015-06-09 20:58 GMT+08:00 Bhuvanchandra DV >> <bhuvanchandra.dv@toradex.com>: >>> >>> Hello Axel, >>> >>> On 06/09/2015 02:45 PM, Axel Lin wrote: >>>> >>>> >>>> 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 <axel.lin@ingics.com> >>>> --- >>>> 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. >> >> >> 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
diff --git a/drivers/gpio/vybrid_gpio.c b/drivers/gpio/vybrid_gpio.c index 6eaf0a9..33bbf54 100644 --- a/drivers/gpio/vybrid_gpio.c +++ b/drivers/gpio/vybrid_gpio.c @@ -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)
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 <axel.lin@ingics.com> --- 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(-)