Re: [PATCH 2/3] iio: dac: ad3552r: add support for ad3541r and ad3551r

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

On 10/05/24 5:42 PM, David Lechner wrote:
On Fri, May 10, 2024 at 9:19 AM Angelo Dureghello
<adureghello@xxxxxxxxxxxx> wrote:
From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>

Add support for single-output dac variants.

Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
---
  drivers/iio/dac/ad3552r.c | 39 +++++++++++++++++++++++++++++----------
  1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index a492e8f2fc0f..0dd6f995c3e2 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -140,7 +140,9 @@ enum ad3552r_ch_vref_select {
  };

  enum ad3542r_id {
+       AD3541R_ID = 0x400b,
         AD3542R_ID = 0x4009,
+       AD3551R_ID = 0x400a,
         AD3552R_ID = 0x4008,
  };

@@ -745,7 +747,8 @@ static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
         } else {
                 /* Normal range */
                 idx = dac->ch_data[ch].range;
-               if (dac->chip_id == AD3542R_ID) {
+               if (dac->chip_id == AD3541R_ID ||
+                   dac->chip_id == AD3542R_ID) {
                         v_min = ad3542r_ch_ranges[idx][0];
                         v_max = ad3542r_ch_ranges[idx][1];
                 } else {
@@ -780,7 +783,7 @@ static int ad3552r_find_range(u16 id, s32 *vals)
         int i, len;
         const s32 (*ranges)[2];

-       if (id == AD3542R_ID) {
+       if (id == AD3541R_ID || id == AD3542R_ID) {
                 len = ARRAY_SIZE(ad3542r_ch_ranges);
                 ranges = ad3542r_ch_ranges;
         } else {
@@ -955,9 +958,10 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
                         dev_err(dev, "mandatory reg property missing\n");
                         goto put_child;
                 }
-               if (ch >= AD3552R_NUM_CH) {
-                       dev_err(dev, "reg must be less than %d\n",
-                               AD3552R_NUM_CH);
+               if (ch >= AD3552R_NUM_CH ||
+                       (dac->chip_id == AD3541R_ID && ch) ||
+                       (dac->chip_id == AD3551R_ID && ch)) {
+                       dev_err(dev, "channel %d is not supported\n", ch);
                         err = -EINVAL;
                         goto put_child;
                 }
@@ -987,9 +991,10 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
                                 goto put_child;

                         dac->ch_data[ch].range = val;
-               } else if (dac->chip_id == AD3542R_ID) {
+               } else if (dac->chip_id == AD3541R_ID ||
+                          dac->chip_id == AD3542R_ID) {
                         dev_err(dev,
-                               "adi,output-range-microvolt is required for ad3542r\n");
+                               "adi,output-range-microvolt is required for ad354xr\n");
                         err = -EINVAL;
                         goto put_child;
                 } else {
@@ -1088,10 +1093,20 @@ static int ad3552r_probe(struct spi_device *spi)
                 return err;

         /* Config triggered buffer device */
-       if (dac->chip_id == AD3552R_ID)
-               indio_dev->name = "ad3552r";
-       else
+       switch (dac->chip_id) {
+       case AD3541R_ID:
+               indio_dev->name = "ad3541r";
+               break;
+       case AD3542R_ID:
                 indio_dev->name = "ad3542r";
+               break;
+       case AD3551R_ID:
+               indio_dev->name = "ad3551r";
+               break;
+       case AD3552R_ID:
+               indio_dev->name = "ad3552r";
+               break;
+       }
         indio_dev->dev.parent = &spi->dev;
         indio_dev->info = &ad3552r_iio_info;
         indio_dev->num_channels = dac->num_ch;
@@ -1110,14 +1125,18 @@ static int ad3552r_probe(struct spi_device *spi)
  }

  static const struct spi_device_id ad3552r_id[] = {
+       { "ad3541r", AD3541R_ID },
         { "ad3542r", AD3542R_ID },
+       { "ad3551r", AD3551R_ID },
         { "ad3552r", AD3552R_ID },
         { }
  };
  MODULE_DEVICE_TABLE(spi, ad3552r_id);

  static const struct of_device_id ad3552r_of_match[] = {
+       { .compatible = "adi,ad3541r"},
         { .compatible = "adi,ad3542r"},
+       { .compatible = "adi,ad3551r"},
         { .compatible = "adi,ad3552r"},
         { }
  };
--
2.45.0.rc1


It looks like it is time for a chip_info struct here instead of the if
and switch statements to get chip-specific data. Most other IIO
drivers have this already and it is the preferred way to look up this
kind of information in the IIO subsystem. I prefer the drivers that
don't put all of the info structs in an array (that way the code is
less verbose). So I would suggest looking at e.g. adc/aspeed_adc,
starting with aspeed_adc_matches, to see what I mean and how to
implement it. (So one patch to add the info structs and a second patch
to add the single channel chips)

Ack, will change in that way.


Regards,
angelo






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux