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. > 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? Best regards, Krzysztof
