[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT
Date: Mon, 11 Apr 2016 08:47:33 -0600	[thread overview]
Message-ID: <CAPnjgZ3QM-MVkkfy_nFLAJ87jGttXcRn+nw2pZVOtwCbpA2BZg@mail.gmail.com> (raw)
In-Reply-To: <570A67B4.9000009@nelint.com>

Hi Eric,

On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
> Hi Simon,
>
> On 04/09/2016 11:33 AM, Simon Glass wrote:
>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>
>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>
>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>
>
> <snip>
>
>>>
>>> The intent of the change is good.
>>>
>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>> for clients so they don't need to know how to access driver functionality
>>> through the ops pointer, which I think is an internal/private implementation
>>> detail. Is that detail exposed to clients in other places? If so, removing
>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>
>> This seems a bit pedantic, but since Linux does it this way I think we
>> should follow along.
>>
>> Eric you still get to remove the code from all the GPIO drivers - the
>> difference is just creating a common function to call when no xlate()
>> method is available.
>>
>> Can you please take a look at what Stephen suggests?
>>
>
> Got it. I'm just not sure about where to start (before or after
> the patch set you sent) and whether to also remove offset parsing
> from gpio_find_and_xlate().
>

Which patch did I send? My understanding is:

- Add my review/ack tag to the patches as necessary
- Drop the tegra patch
- Update gpio_find_and_xlate() to call a default function if there is
no xlate() method
- Resend the series

I'm not sure about removing the existing functionality from
gpio_find_and_xlate(), but my guess is that it is best to move it to
your default function, so that gpio_find_and_xlate() doesn't include
any default behaviour in the case where there is a xlate() method.

Regards,
Simon