[U-Boot] [PATCH] gpio: spear_gpio: Fix gpio_set_value() implementation

[Michael Trimarchi]

Hi

On Mon, Aug 12, 2013 at 6:57 PM, Axel Lin <axel.lin at ingics.com> wrote:
> 2013/7/1 Vipin Kumar <vipin.kumar at st.com>:
>> On 7/1/2013 11:02 AM, Axel Lin wrote:
>>>>
>>>>
>>>> The questions raised here are valid and it forced me to re-read the
>>>> datasheet. For your convenience, I must tell you that the device is
>>>> actually
>>>> pl061 from ARM, so the driver can also be named so.
>>>>
>>>> The datasheet is here
>>>>
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0190b/I1002697.html
>>>>
>>>> Quoting from the datasheet
>>>> "The GPIODATA register is the data register. In software control mode,
>>>> values written in the GPIODATA register are transferred onto the GPOUT
>>>> pins
>>>> if the respective pins have been configured as outputs through the
>>>> GPIODIR
>>>> register.
>>>>
>>>> In order to write to GPIODATA, the corresponding bits in the mask,
>>>> resulting
>>>> from the address bus, PADDR[9:2], must be HIGH. Otherwise the bit values
>>>> remain unchanged by the write.
>>>>
>>>> Similarly, the values read from this register are determined for each
>>>> bit,
>>>> by the mask bit derived from the address used to access the data
>>>> register,
>>>> PADDR[9:2]. Bits that are 1 in the address mask cause the corresponding
>>>> bits
>>>> in GPIODATA to be read, and bits that are 0 in the address mask cause the
>>>> corresponding bits in GPIODATA to be read as 0, regardless of their
>>>> value.
>>>>
>>>> A read from GPIODATA returns the last bit value written if the respective
>>>> pins are configured as output, or it returns the value on the
>>>> corresponding
>>>> input GPIN bit when these are configured as inputs. All bits are cleared
>>>> by
>>>> a reset."
>>>>
>>>> After reading all this I am confused about numbering of the gpio's. I
>>>> think
>>>> the numbering should be from 1 to 8 for a device. And this would mean
>>>> that
>>>> we should write to *&regs->datareg[1<<  (gpio - 1)]* instead of the
>>>> present
>>>> code which is _&regs->datareg[1<<  (gpio + 2)]_
>>>
>>>
>>> Hi Vipin,
>>
>>
>> Hello Alex,
>>
>>
>>> Thanks for the review and providing the datasheet information.
>>> You mentioned that this is PL061.
>>> So... I just checked the gpio-pl061 driver in linux kernel.
>>> It's writing to _&regs->datareg[1<<  (gpio + 2)]. and seems no bug
>>> report for this.
>>>
>>
>> Yes, I see it now. The difference is that we are using a writel and the
>> datareg is a u32 array.
>
> Hi,
> The merge window is going to close.
> I'm wondering if this patch is ok or not (I think it's a bug fix).
> I think the only issue is nobody has this hardware to test.
> [Sorry to back to this topic so late... just too busy recently.]
>

You are right. I read the documentation and it works

Michael

> Regards,
> Axel