Re: [PATCH v2 1/2] gpio: altera-a10sr: Set proper output level for direction_output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Axel,

On 1/18/19 5:44 PM, Axel Lin wrote:
Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx> 於 2019年1月19日 週六 上午2:50寫道:

Hi Axel,

On 1/17/19 6:40 AM, Axel Lin wrote:
The altr_a10sr_gpio_direction_output should set proper output level
based on the value argument.

Signed-off-by: Axel Lin <axel.lin@xxxxxxxxxx>
---
v2: Based on Bartosz's comment to split the patch.
      1/2 is bug fix
      2/2 is coding style fix and fix checkpatch warning
   drivers/gpio/gpio-altera-a10sr.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-altera-a10sr.c
b/drivers/gpio/gpio-altera-a10sr.c
index 6b11f1314248..7f9e0304b510 100644
--- a/drivers/gpio/gpio-altera-a10sr.c
+++ b/drivers/gpio/gpio-altera-a10sr.c
@@ -66,8 +66,10 @@ static int altr_a10sr_gpio_direction_input(struct
gpio_chip *gc,
   static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc,
                                           unsigned int nr, int value)
   {
-     if (nr <= (ALTR_A10SR_OUT_VALID_RANGE_HI -
ALTR_A10SR_LED_VALID_SHIFT))
+     if (nr <= (ALTR_A10SR_OUT_VALID_RANGE_HI -
ALTR_A10SR_LED_VALID_SHIFT)) {
+             altr_a10sr_gpio_set(gc, nr, value);
               return 0;
+     }
       return -EINVAL;
   }



Sorry, this patch is not valid. I should add a comment above these
functions to explain better. These are actually GPI and GPO on a custom
expansion chip. The directions are hard coded as INPUT or OUTPUT. If the
range is valid, it returns 0 and -EINVAL if not.

For GPO, the only valid values are from 4 to 7.

Hi Thor,
It seems you misunderstand the point.
from 4 to 7 seems to be the nr argument.
The value argument is for gpio output level HIGH or LOW.
This patch does not change the nr argument checking, what it does is
calling altr_a10sr_gpio_set(gc, nr, value); to ensure the output value is
set
to the setting of altr_a10sr_gpio_direction_output().

Regards,
Axel


Yes, I see. This patch looks good and I successfully tested on our platform.

Thanks,

Tested by: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>
Reviewed by: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>

For GPI, reading 4 to 7 will tell you what is being written on GPO.
There is another GPI bank starting at 8 and extending to 15.

I do agree that altr_a10sr_gpio_direction_input() should be better
constrained from 4 to 15.

Thanks for pointing that out.

Thor






[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux