diff mbox series

[v2,3/6] iio: dac: ad3552r: add model data structure

Message ID 20240522150141.1776196-4-adureghello@baylibre.org (mailing list archive)
State Accepted
Headers show
Series minor fixes and improvements | expand

Commit Message

Angelo Dureghello May 22, 2024, 3:01 p.m. UTC 
From: Angelo Dureghello <adureghello@baylibre.com>

Add a "model data" structure to keep useful hardware-related
information as from datasheet, avoiding id-based conditional
choices later on.

Removed id-based checks and filled model-specific structures
with device specific features, In particular, num_hw_channels
is introduced to keep the number of hardware implemented
channels, since 1-channel versions of the DACs are added
in this same patchset.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes for v2:
- patch added in v2
---
 drivers/iio/dac/ad3552r.c | 98 +++++++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 39 deletions(-)

Comments

Nuno Sá May 23, 2024, 12:43 p.m. UTC | #1 
On Wed, 2024-05-22 at 17:01 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Add a "model data" structure to keep useful hardware-related
> information as from datasheet, avoiding id-based conditional
> choices later on.
> 
> Removed id-based checks and filled model-specific structures
> with device specific features, In particular, num_hw_channels
> is introduced to keep the number of hardware implemented
> channels, since 1-channel versions of the DACs are added
> in this same patchset.
> 
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> Changes for v2:
> - patch added in v2
> ---
>  drivers/iio/dac/ad3552r.c | 98 +++++++++++++++++++++++----------------
>  1 file changed, 59 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> index a492e8f2fc0f..6a40c7eece1f 100644
> --- a/drivers/iio/dac/ad3552r.c
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -261,7 +261,17 @@ struct ad3552r_ch_data {
>  	bool	range_override;
>  };
>  
> +struct ad3552r_model_data {
> +	const char *model_name;
> +	enum ad3542r_id chip_id;
> +	unsigned int num_hw_channels;
> +	const s32 (*ranges_table)[2];
> +	int num_ranges;
> +	bool requires_output_range;
> +};
> +

nit: we often would call this (in IIO) ad3552r_chip_info. Then...
>  struct ad3552r_desc {
> +	const struct ad3552r_model_data *model_data;

*chip_info;

Anyways, not really something worth a re-spin. But if you need one, something to
consider :)

- Nuno Sá
diff mbox series

Patch

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index a492e8f2fc0f..6a40c7eece1f 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -261,7 +261,17 @@  struct ad3552r_ch_data {
 	bool	range_override;
 };
 
+struct ad3552r_model_data {
+	const char *model_name;
+	enum ad3542r_id chip_id;
+	unsigned int num_hw_channels;
+	const s32 (*ranges_table)[2];
+	int num_ranges;
+	bool requires_output_range;
+};
+
 struct ad3552r_desc {
+	const struct ad3552r_model_data *model_data;
 	/* Used to look the spi bus for atomic operations where needed */
 	struct mutex		lock;
 	struct gpio_desc	*gpio_reset;
@@ -271,7 +281,6 @@  struct ad3552r_desc {
 	struct iio_chan_spec	channels[AD3552R_NUM_CH + 1];
 	unsigned long		enabled_ch;
 	unsigned int		num_ch;
-	enum ad3542r_id		chip_id;
 };
 
 static const u16 addr_mask_map[][2] = {
@@ -745,13 +754,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) {
-			v_min = ad3542r_ch_ranges[idx][0];
-			v_max = ad3542r_ch_ranges[idx][1];
-		} else {
-			v_min = ad3552r_ch_ranges[idx][0];
-			v_max = ad3552r_ch_ranges[idx][1];
-		}
+		v_min = dac->model_data->ranges_table[idx][0];
+		v_max = dac->model_data->ranges_table[idx][1];
 	}
 
 	/*
@@ -775,22 +779,14 @@  static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
 	dac->ch_data[ch].offset_dec = div_s64(tmp, span);
 }
 
-static int ad3552r_find_range(u16 id, s32 *vals)
+static int ad3552r_find_range(const struct ad3552r_model_data *model_data,
+			      s32 *vals)
 {
-	int i, len;
-	const s32 (*ranges)[2];
+	int i;
 
-	if (id == AD3542R_ID) {
-		len = ARRAY_SIZE(ad3542r_ch_ranges);
-		ranges = ad3542r_ch_ranges;
-	} else {
-		len = ARRAY_SIZE(ad3552r_ch_ranges);
-		ranges = ad3552r_ch_ranges;
-	}
-
-	for (i = 0; i < len; i++)
-		if (vals[0] == ranges[i][0] * 1000 &&
-		    vals[1] == ranges[i][1] * 1000)
+	for (i = 0; i < model_data->num_ranges; i++)
+		if (vals[0] == model_data->ranges_table[i][0] * 1000 &&
+		    vals[1] == model_data->ranges_table[i][1] * 1000)
 			return i;
 
 	return -EINVAL;
@@ -955,9 +951,9 @@  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) {
+		if (ch >= dac->model_data->num_hw_channels) {
 			dev_err(dev, "reg must be less than %d\n",
-				AD3552R_NUM_CH);
+				dac->model_data->num_hw_channels);
 			err = -EINVAL;
 			goto put_child;
 		}
@@ -973,7 +969,7 @@  static int ad3552r_configure_device(struct ad3552r_desc *dac)
 				goto put_child;
 			}
 
-			err = ad3552r_find_range(dac->chip_id, vals);
+			err = ad3552r_find_range(dac->model_data, vals);
 			if (err < 0) {
 				dev_err(dev,
 					"Invalid adi,output-range-microvolt value\n");
@@ -987,9 +983,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->model_data->requires_output_range) {
 			dev_err(dev,
-				"adi,output-range-microvolt is required for ad3542r\n");
+				"adi,output-range-microvolt is required for %s\n",
+				dac->model_data->model_name);
 			err = -EINVAL;
 			goto put_child;
 		} else {
@@ -1011,7 +1008,8 @@  static int ad3552r_configure_device(struct ad3552r_desc *dac)
 	}
 
 	/* Disable unused channels */
-	for_each_clear_bit(ch, &dac->enabled_ch, AD3552R_NUM_CH) {
+	for_each_clear_bit(ch, &dac->enabled_ch,
+			   dac->model_data->num_hw_channels) {
 		err = ad3552r_set_ch_value(dac, AD3552R_CH_AMPLIFIER_POWERDOWN,
 					   ch, 1);
 		if (err)
@@ -1058,7 +1056,7 @@  static int ad3552r_init(struct ad3552r_desc *dac)
 	}
 
 	id |= val << 8;
-	if (id != dac->chip_id) {
+	if (id != dac->model_data->chip_id) {
 		dev_err(&dac->spi->dev, "Product id not matching\n");
 		return -ENODEV;
 	}
@@ -1068,7 +1066,6 @@  static int ad3552r_init(struct ad3552r_desc *dac)
 
 static int ad3552r_probe(struct spi_device *spi)
 {
-	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct ad3552r_desc *dac;
 	struct iio_dev *indio_dev;
 	int err;
@@ -1079,7 +1076,9 @@  static int ad3552r_probe(struct spi_device *spi)
 
 	dac = iio_priv(indio_dev);
 	dac->spi = spi;
-	dac->chip_id = id->driver_data;
+	dac->model_data = spi_get_device_match_data(spi);
+	if (!dac->model_data)
+		return -EINVAL;
 
 	mutex_init(&dac->lock);
 
@@ -1088,10 +1087,7 @@  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
-		indio_dev->name = "ad3542r";
+	indio_dev->name = dac->model_data->model_name;
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->info = &ad3552r_iio_info;
 	indio_dev->num_channels = dac->num_ch;
@@ -1109,16 +1105,40 @@  static int ad3552r_probe(struct spi_device *spi)
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
+static const struct ad3552r_model_data ad3542r_model_data = {
+	.model_name = "ad3542r",
+	.chip_id = AD3542R_ID,
+	.num_hw_channels = 2,
+	.ranges_table = ad3542r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3542r_ch_ranges),
+	.requires_output_range = true,
+};
+
+static const struct ad3552r_model_data ad3552r_model_data = {
+	.model_name = "ad3552r",
+	.chip_id = AD3552R_ID,
+	.num_hw_channels = 2,
+	.ranges_table = ad3552r_ch_ranges,
+	.num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
+	.requires_output_range = false,
+};
+
 static const struct spi_device_id ad3552r_id[] = {
-	{ "ad3542r", AD3542R_ID },
-	{ "ad3552r", AD3552R_ID },
+	{
+		.name = "ad3542r",
+		.driver_data = (kernel_ulong_t)&ad3542r_model_data
+	},
+	{
+		.name = "ad3552r",
+		.driver_data = (kernel_ulong_t)&ad3552r_model_data
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad3552r_id);
 
 static const struct of_device_id ad3552r_of_match[] = {
-	{ .compatible = "adi,ad3542r"},
-	{ .compatible = "adi,ad3552r"},
+	{ .compatible = "adi,ad3542r", .data = &ad3542r_model_data },
+	{ .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad3552r_of_match);