On Fri, 13 Oct 2023 11:53:33 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 13/10/2023 11:09, Jonathan Cameron wrote: > >>>>>>>> + shtdn-enable-gpios: > >>>>>>> > >>>>>>> I guess the review crossed with you sending v5. There is some > >>>>>>> feedback on v4 you need > >>>>>>> to address here. > >>>>>> > >>>>>> Jonathan, I thought I did, I've changed ena to powerdown-gpios from > >>>>>> Krzysztof's comments but about this one pin I'm still not sure, it > >>>>>> looks like *-enable-gpios (like in *-enable-gpios pins in > >>>>>> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any > >>>>>> other > >>>>>> suggestions about naming of this one? > >>>>>> > >>>>>> Thanks. > >>>>> > >>>>> shutdown-gpios and make the sense (active high / low) such that > >>>>> setting > >>>>> it results in teh device being shut down. > >>>>> Or treat it as an enable and enable-gpios > >>>>> > >>>>> Something that indicates both shutdown and enable is confusing ;) > >>>>> > >>>>> Jonathan > >>>> > >>>> > >>>> Jonathan, then I make these changes: > >>>> > >>>> powerdown-gpios: -> output-enable: > >>> Needs to retain the gpios bit as we want the standard gpio stuff to pick > >>> them up. I'm not that keen on output-enable-gpios though. The activity > >>> here is very much 'shutdown because of error or not enabled' I think. > >>> So perhaps we flip the sense and document that it needs to be active low? > >>> > >>>> shtdn-enable-gpios: -> enable-gpios: > >>>> > >>>> Is it ok? > >>> > >>> Conor, Rob, Krzysztof - you probably have a better insight into this than > >>> I do. > >>> > >> > >> "enable-gpios" are for turning on a specific feature, not powering > >> on/off entire device. For example to enable regulator output. > >> > >> "powerdown-gpios" are for turning device on/off. > >> > >> I don't know what do you have in your device. > > Ok. Sounds like that what is enable-gpios above should be shutdown-gpios. > > shutdown-gpios sounds exactly the same as powerdown-gpios and it is > already used in exactly same context. Oops. Yup. powerdown-gpios seems appropriate. > > > The other case is a device output indicating whether the device is > > shutdown. That can happen because it was told to do so (via the other gpio), > > or because it is in an error state. What's a good naming convention for that? > > There is no convention and I did not see such case so far. > powerdown-status-gpios? powerdown-state-gpios? Either seems reasonable. Thanks, J > > > > Best regards, > Krzysztof >
