Re: [PATCH 2/4] gpiolib: Add code to support "active low" GPIOs

[Nikita Yushchenko <nikita.yoush@cogentembedded.com>]

> I am not sure I really see a problem with the situation you describe
> and why you'd want to change device tree description (perhaps because
> I don't know all of the details). I agree, that, depending on you
> personal preferences, one might find it annoying that driver uses
> physical values, but other than that I don't see a technical problem
> with using physical values in the driver, since as you said, it's not
> very likely that same pin on the same chip would be active low/vs
> active high.

First point is that words "active high" (or "active low") have very
clear meaning. And situation when chip's signal is active low, but one
has to write "active high" in signal definition to make things working,
is not something I welcome in systems I deal with.

Second point is that by cleaning up the above, you make drivers depend
on correct polarity settings in dts. Which means that when writing dts,
you get need to dig over datasheets (which may be under NDA) to find out
polarity of each signal. This looks like breakage of information
locality - knowledge of chip's signals polarity belongs to chip's
driver. Common case of signals connected directly to gpio providers
should just work.  It's nice to have a way to override driver's default,
to handle signal inversion by board logic and other special cases, but
that should be purely optional.

Third point is code clearance. If routine is called set_value, has
argument that very looks like value to be set, but instead of setting
given value does something different, it's source of confusion and errors.

But perhaps this thread is wrong place to discuss all that.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox