Marek Vasut writes: > On 3/5/19 11:07 AM, Harald Geyer wrote: > > marek.vasut@xxxxxxxxx writes: > >> From: Marek Vasut <marek.vasut+renesas@xxxxxxxxx> > >> > >> Reword the binding document to make it clear how the propeties work > >> and which properties affect which other properties. > >> > >> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx> > >> Cc: Harald Geyer <harald@xxxxxxxxx> > >> Cc: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > >> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx> > >> Cc: Mark Brown <broonie@xxxxxxxxxx> > >> Cc: Rob Herring <robh@xxxxxxxxxx> > >> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx > >> To: devicetree@xxxxxxxxxxxxxxx > >> --- > >> V2: - Make "gpios" a mandatory property > >> - Reword "gpio-states" property description > >> - Change "enable-gpio" to "enable-gpios" to match modern DT rules > >> Note: The recent gpio-regulator rework caused breakage. While the > >> changes in the gpio-regulator code were according to the DT > >> binding document, they stopped working with older DTs. Make > >> the binding document clearer to prevent such breakage in the > >> future. > > > > Thanks for the update. I think it addresses all my concerns except for > > one: > > > >> +- gpios-states : State of GPIO pins in "gpios" array that is set until > >> + changed by the first consumer. 0: LOW, 1: HIGH. > >> + Default is LOW if nothing else is specified. > > > > I still believe this not true: There is no guarantee that the regulator > > core won't change the state of GPIO pins before the first consumer comes > > up. > > Why would it do that ? Because the regulator core doesn't know about this driver specific property at all. And without any constraints placed by consumers, the core is free to choose any state whatsoever at any point in time. > That would completely invalidate any remaining > useful-ness of this property. Yes, I believe this property is mostly useless. That's what I want to get across with my wording proposal. The remaining usecase, that I can see, is when the GPIOs have been setup by the bootloader and we don't want to reset them to low during probing (which some OSes might be capable of, but linux currently doesn't). Also a state of all GPIOs low might be invalid (not in the "states" property), so we shouldn't set all GPIOs to low during probing in that case. HTH, Harald > > I still think my proposal describes the property more acurately: > > gpios-states : On operating systems, that don't support reading back gpio > > values in output mode (most notably linux), this array > > provides the state of GPIO pins set when requesting them > > from the gpio controller. Systems, that are capable of > > preserving state when requesting the lines, are free to > > ignore this property. 0: LOW, 1: HIGH. Default is LOW if > > nothing else is specified. > > > > Since we had this discussion already in the V1 thread and you clearly don't > > agree with me, the maintainers will need to decide. You can add > > Reviewed-by: Harald Geyer <harald@xxxxxxxxx> > > once Rob and/or Mark have addressed this issue. > > I think we're just looking at this from two different perspectives and > for whatever reason can't reconcile them. > > > Thanks for your work! > > Harald > > > > > -- > Best regards, > Marek Vasut -- If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via CLAM to xASPBtezLNqj4cUe8MT5nZjthRSEjrRQXN or via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w
