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. Thanks, Jonathan > > What do you think? >
