On Wed, May 24, 2017 at 01:36:48PM -0700, Andrey Smirnov wrote: > On Wed, May 24, 2017 at 11:16 AM, Trent Piepho <tpiepho@xxxxxxxxxxxxxx> wrote: > > On Wed, 2017-05-24 at 10:26 +0300, Nikita Yushchenko wrote: > >> 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, > > > > I agree with this. It's pretty much random if a given signal will want > > a high value to mean asserted or not. > > Yes, and that the point of having "active low" being configurable in > device tree so it would be possible to use exactly the same driver > code for slightly different setups. > > > And plenty of signals switch > > "modes" and it's not even clear which mode should be considered > > "asserted". > > First this statement is so vague that it is hard to make any argument > about it. Second, just because a feature doesn't cover every possible > use-case doesn't mean that it doesn't have a place in the code base at > all. > > > If drivers expect me to put active low/high in the > > bindings, then it means for every signal one must get the datasheet and > > figure out what a high signal means. And then likely look though the > > driver code to make sure the driver sets 1 to mean that. > > > > I don't see how "active low" changes the way you troubleshoot things > at all. If you are not lucky and you code didn't just work from the > first try, wouldn't you want to verify that you specified correct GPIO > number by looking at the schematic? And if so wouldn't you see what > high/low means by looking at the chip's symbol? If you don't have > access to a schematic, the only way I see to proceed with debugging it > is to probe correct pin on the chip with a scope, for which you'd need > at least an abridged datasheet that would have pinout documented. > > Regardless of any of that, I seems to me that this is an argument > about personal preferences (I find the feature in question useful and > don't think it is confusing, you guys have dislike it) so I don't > think we'd resolve this any time soon. > > IMHO, whether any of likes it or not, OF_GPIO_ACTIVE_LOW is an > existing feature and the only technical question is if it should be > supported by Barebox on per-driver basis or if there should be a > central API for it. I still think that gpio_[gs]et_value should set the GPIOs to the actual logical value and not take any GPIO_ACTIVE_* flags into account. Also I still think that having an additional gpio_set_[in]active API would be useful. It's a bit unfortunate that in Linux gpio_set_value and gpiod_set_value behave differently, despite the similar name. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
