Re: AD 5766 driver software reset during runtime

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 30 Apr 2020 06:52:50 +0000
"Pop, Cristian" <Cristian.Pop@analog.com> wrote:

> Hello to all,
> 
> For those who are receiving this email for the second time, it seems
> that linux-iio@vger.kernel.org, doesn't accept HTML format emails, so
> this is a plain text version.
> 
> I have been developing a linux kernel drive for a DAC converter,
> ad5766 and it supports changing the output span range, from eight
> available output ranges during runtime. This is done setting scale
> and offset attributes trough a `write_raw` call. When changing the
> range during runtime for this chip, a software reset is necessary,
> before writing to the span register. Do you think this is acceptable
> upstream? Should we make the span ranges selectable at runtime? From
> what I heard, it's not always a good idea to do reset of a device at
> runtime.

There is no fundamental issue with doing resets at runtime - we often
have to do this to get out of error conditions etc and it also often
happens if we remove and reprobe a device.

What I really don't want to see is a userspace reset control.  If
some other control requires a reset to actually set the value, then fine.

I would suggest you want to block that control unless all the dac
channels are disabled.  Right now it looks like the driver
doesn't support power down, but I think the hardware does from
a quick glance at the datasheet.  Resetting with powered up channels
sounds like a bad idea to me.

Thanks,

Jonathan

> 
> AD5722 software reset in driver before setting a new offset / range:
> https://github.com/analogdevicesinc/linux/blob/4c7d019397e696dcaf735c52169a92fcbe5b5b8b/drivers/iio/dac/ad5766.c#L309
> 
> AD5722 software reset in driver, before setting a new scale / range:
> https://github.com/analogdevicesinc/linux/blob/4c7d019397e696dcaf735c52169a92fcbe5b5b8b/drivers/iio/dac/ad5766.c#L357
> 
> AD5722 datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD5722_5732_5752.pdf
> 
> Thanks and best regards,
> Cristian Pop