[U-Boot] gpio: Question about gpio_set_value() implementation in spear_gpio.c

[Axel Lin]

2013/6/19 Marek Vasut <marex at denx.de>:
> Dear Axel Lin,
>
>> Current code looks strange because no matter the value argument is 0 or 1
>> it always calls
>>       writel(1 << gpio, &regs->gpiodata[DATA_REG_ADDR(gpio)]);
>>
>> And then gpio_get_value() always return 1.
>>
>> I'm wondering if it needs to be fixed, something like below change:
>>
>> diff --git a/drivers/gpio/spear_gpio.c b/drivers/gpio/spear_gpio.c
>> index d3c728e..8878608 100644
>> --- a/drivers/gpio/spear_gpio.c
>> +++ b/drivers/gpio/spear_gpio.c
>> @@ -52,7 +52,10 @@ int gpio_set_value(unsigned gpio, int value)
>>  {
>>       struct gpio_regs *regs = (struct gpio_regs *)CONFIG_GPIO_BASE;
>>
>> -     writel(1 << gpio, &regs->gpiodata[DATA_REG_ADDR(gpio)]);
>> +     if (value)
>> +             writel(1 << gpio, &regs->gpiodata[DATA_REG_ADDR(gpio)]);
>> +     else
>> +             writel(0, &regs->gpiodata[DATA_REG_ADDR(gpio)]);
>
>
> You might want to use clrbits_le32() and setbits_le32() here, no?
>

Honestly, I ask this because I cannot find the detail datasheet for the gpio
control of this device. ( Just found the code looks strange).

Usually we can use clrbits_le32() and setbits_le32() helpers to set/clear
register bits. But if the only meaningful bit is "1 << gpio", simply use
"write 1 << gpio" and "write 0" saves a register read operation.

So maybe Stefan  or someone from ST can provide the information about gpio
control on this hardware.

Regards,
Axel