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