Re: [PATCH] iio: dac: ad5446: Add ID of compatible Texas Instruments i2c dac121c*

[Jonathan Cameron <jic23@kernel.org>]

On Sun,  7 May 2023 11:10:25 +0200
Philippe De Muyter <Philippe.DeMuyter@macq.eu> wrote:

> From: Philippe De Muyter <phdm@macqel.be>
> 
> The Texas Instruments DAC121C* chips are the I2C counterparts of
> the DAC121S* SPI chips which are already supported by this ad5446 driver.
> 
> Add them to the compatible list.

Hi Philippe,

DT binding should be updated and include the fallback to adi,ad5622.
Does this driver actually have a bindings doc?  If not please add one
as a precursor patch then add binding for this new part on top.

> 
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Michael Hennerich <Michael.Hennerich@analog.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/dac/ad5446.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> index aa3130b33456..b95c0ccbb796 100644
> --- a/drivers/iio/dac/ad5446.c
> +++ b/drivers/iio/dac/ad5446.c
> @@ -587,6 +587,7 @@ static const struct i2c_device_id ad5446_i2c_ids[] = {
>  	{"ad5602", ID_AD5602},
>  	{"ad5612", ID_AD5612},
>  	{"ad5622", ID_AD5622},
> +	{"dac121", ID_AD5622}, /* 'ti,' is dropped by 'of_modalias_node' */

True, but why is the comment needed?
Also, for consistency with the spi equivalent it should be dac121c101 or similar
I think.

I think this use of the driver with multiple vendor prefixes,
also indicates we should really add the of_device_id table for this
driver. To do that nicely will require more changes as we'd want to do
the same for the SPI side which has a single entry table (which is odd)
then deal with the data fields which should probably all be pointers
rather than enum values.

Still I'm fine with proper explicit DT support being left for a follow up patch.

I do want the missing binding doc fixed though (which is independent of the
question of how the driver binds based on the compatible values).

Thanks,

Jonathan


>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);