Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163

[Lukas <lukas.metz@gmx.net>]

Thanks for the review. As i said this is my first time submitting a
patch. I have looked at already existing spi dac drivers for reference
but i seemed to have missed quite a lot. But the comments are greatly
appreciated.

On Wed, Jun 24, 2026 at 12:56:15AM +0600, Siratul Islam wrote:
> A link to the datasheet here would be nice.

I will try to add all the small suggestions i dont mention explicitly,
like style issues or using guard instead of manual lock/unlock to v2.

> > +
> > +	if (st->internal_ref) {
> > +		st->vref_uv = 2500000; /* 2.5V internal reference */
> A note on where this value came from or why this was chosen, or a reference to datasheet would be better.

I think i would add the suggestion from David Lechner to remove the
internal_ref property completly and add "the way of doing optional
voltage references". This includes using the macro
DAC8163_INTERNAL_REF_mV. Would this be acceptable?

> You have a CMD_SOFT_RST defined but not used. Should this be used to reset before doing any configuration?

Yes this is a command which isnt used at this point. But maybe it makes
sense to reset the DAC first when probing.

Best regards
Lukas