On 04/11/2016 09:12 AM, Simon Glass wrote:
Hi Eric,
On 11 April 2016 at 09:10, Eric Nelson <e...@nelint.com> wrote:
Hi Simon,
On 04/11/2016 07:59 AM, Simon Glass wrote:
On 11 April 2016 at 08:55, Eric Nelson <e...@nelint.com> wrote:
On 04/11/2016 07:47 AM, Simon Glass wrote:
On 10 April 2016 at 08:48, Eric Nelson <e...@nelint.com> wrote:
On 04/09/2016 11:33 AM, Simon Glass wrote:
On 4 April 2016 at 11:50, Stephen Warren <swar...@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:
At the time I sent this, you had just submitted the patch set adding
more driver-model support for block devices.
http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
- 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.
Reviewing the use of the offset field did yield some information about
the broken sunxi support and also that Vybrid was also missing
the xlate routine.
Since reviewing your patch sets (driver model updates for blk and also
driver model updates for mmc) will take some time, so I'll base an
updated patch set on master. My guess is that any merge issues will
be trivial.
Yes, that's right.
I'll remove your acks in the updated patch set, since the updates
to the drivers won't drop the xlate field, but will connect them
to the common (__maybe_unused) routine. This will prevent the code
from leaking into machines like Tegra that don't need the common code.
I'm pretty sure you can drop the xlate() implementations from the
functions, though, and those at the patches I acked.
I don't think you need __maybe_unused
static int gpio_find_and_xlate(...)
{
get ops...
if (ops->xlate)
return ops->xlate(....)
else
return gpio_default_xlate()...
}
gpio_default_xlate() (or whatever name you use) should be exported so
drivers can use it.
This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
into machines that don't need it.
I can go the route you suggest above, but it will cost the tegra
and sandbox builds ~64 bytes ;)
Sure, but we can live with that.
You can avoid that by requiring that ops->xlate always be non-NULL, and
simply referencing the default function from drivers that want to use
it. Not a big deal either way though.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
