On lis 11, 2018 15:01, Jonathan Cameron wrote: > On Fri, 9 Nov 2018 17:59:04 +0100 > Slawomir Stepien <sst@xxxxxxxxx> wrote: > > > On lis 04, 2018 16:33, Jonathan Cameron wrote: > > > The odd bit here is that I'm not entirely sure what 'power up' action > > > this power down is undoing, so not sure where exactly it should be. > > > > > > It may just be a catch all for the device being left powered up after > > > a read sometime earlier. If that's the case I would suggest a comment > > > making that clear and do it only just before the devm_iio_device_register > > > (as we don't power up anywhere in probe that I can see and this is the > > > point at which a power up 'might' occur as the interfaces are exposed. > > > > Inside the ad7280_chain_setup(), the first write to the device(s) is with > > AD7280A_CTRL_LB_SWRST bit. The next write is de-asserting this bit. Based on > > datasheet, such two writes will reset the upper part of the control register to > > its default state, that means, the device will left the software power down > > state. > > > > So inside ad7280_chain_setup() we have the power up you are talking about. > > That is why I think that action that will put the device into software power > > down should be after spi_setup(), but before ad7280_chain_setup() - and this is > > the current form (of course I will use the ...or_reset() variant as Jha pointed > > out in next patch's version). > Good explanation. I would just add a brief version of this to the code > so it's easy to follow for anyone looking at it in the future. Sending v6 (now in the form of two patches), so we can have a discussion there. Thank you. -- Slawomir Stepien
