Re: [PATCH v5 1/1] staging: iio: adc: ad7280a: use devm_* APIs

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 9 Nov 2018 17:59:04 +0100
Slawomir Stepien <sst@poczta.fm> 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?
>