[PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data

Matti Vaittinen posted 3 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data
Posted by Matti Vaittinen 1 month, 3 weeks ago
The adc128s052 driver supports a few different ICs. IC specific
configuration data is stored in an array. IC data, residing in a
specific point of the array, is pointed from the SPI device match data.

There is no need to have the chip config data structures in an array
and splitting them out of an array has at least following benefits:

- Chip-specific structures can be named after the chips they support.
  This makes referring them a tad cleaner, compared to using a generic
  array name with a numerical index.

- Avoid all potential 'out of bounds' errors which can result if the
  array is changed.

Split the chip configuration data array to individual structures.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
There are couple of other series[1][2] changing this driver ongoing. I
haven't heard of those latelty (after JUN?) so those changes haven't
been taken into account and will conflict if those serieses are re-spun.
Please, let me know if I should rebase my changes.

[1]: https://lore.kernel.org/all/20250614091504.575685-1-sbellary@baylibre.com/
[2]: https://lore.kernel.org/all/20250625170218.545654-2-l.rubusch@gmail.com/
---
 drivers/iio/adc/ti-adc128s052.c | 78 +++++++++++++++++----------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 1b46a8155803..81153253529e 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -124,26 +124,30 @@ static const struct iio_chan_spec adc124s021_channels[] = {
 
 static const char * const bd79104_regulators[] = { "iovdd" };
 
-static const struct adc128_configuration adc128_config[] = {
-	{
-		.channels = adc128s052_channels,
-		.num_channels = ARRAY_SIZE(adc128s052_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc122s021_channels,
-		.num_channels = ARRAY_SIZE(adc122s021_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc124s021_channels,
-		.num_channels = ARRAY_SIZE(adc124s021_channels),
-		.refname = "vref",
-	}, {
-		.channels = adc128s052_channels,
-		.num_channels = ARRAY_SIZE(adc128s052_channels),
-		.refname = "vdd",
-		.other_regulators = &bd79104_regulators,
-		.num_other_regulators = 1,
-	},
+static const struct adc128_configuration adc122s_config = {
+	.channels = adc122s021_channels,
+	.num_channels = ARRAY_SIZE(adc122s021_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration adc124s_config = {
+	.channels = adc124s021_channels,
+	.num_channels = ARRAY_SIZE(adc124s021_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration adc128s_config = {
+	.channels = adc128s052_channels,
+	.num_channels = ARRAY_SIZE(adc128s052_channels),
+	.refname = "vref",
+};
+
+static const struct adc128_configuration bd79104_config = {
+	.channels = adc128s052_channels,
+	.num_channels = ARRAY_SIZE(adc128s052_channels),
+	.refname = "vdd",
+	.other_regulators = &bd79104_regulators,
+	.num_other_regulators = 1,
 };
 
 static const struct iio_info adc128_info = {
@@ -199,33 +203,33 @@ static int adc128_probe(struct spi_device *spi)
 }
 
 static const struct of_device_id adc128_of_match[] = {
-	{ .compatible = "ti,adc128s052", .data = &adc128_config[0] },
-	{ .compatible = "ti,adc122s021", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc122s051", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc122s101", .data = &adc128_config[1] },
-	{ .compatible = "ti,adc124s021", .data = &adc128_config[2] },
-	{ .compatible = "ti,adc124s051", .data = &adc128_config[2] },
-	{ .compatible = "ti,adc124s101", .data = &adc128_config[2] },
-	{ .compatible = "rohm,bd79104", .data = &adc128_config[3] },
+	{ .compatible = "ti,adc128s052", .data = &adc128s_config },
+	{ .compatible = "ti,adc122s021", .data = &adc122s_config },
+	{ .compatible = "ti,adc122s051", .data = &adc122s_config },
+	{ .compatible = "ti,adc122s101", .data = &adc122s_config },
+	{ .compatible = "ti,adc124s021", .data = &adc124s_config },
+	{ .compatible = "ti,adc124s051", .data = &adc124s_config },
+	{ .compatible = "ti,adc124s101", .data = &adc124s_config },
+	{ .compatible = "rohm,bd79104", .data = &bd79104_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adc128_of_match);
 
 static const struct spi_device_id adc128_id[] = {
-	{ "adc128s052", (kernel_ulong_t)&adc128_config[0] },
-	{ "adc122s021",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc122s051",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc122s101",	(kernel_ulong_t)&adc128_config[1] },
-	{ "adc124s021", (kernel_ulong_t)&adc128_config[2] },
-	{ "adc124s051", (kernel_ulong_t)&adc128_config[2] },
-	{ "adc124s101", (kernel_ulong_t)&adc128_config[2] },
-	{ "bd79104", (kernel_ulong_t)&adc128_config[3] },
+	{ "adc128s052", (kernel_ulong_t)&adc128s_config },
+	{ "adc122s021",	(kernel_ulong_t)&adc122s_config },
+	{ "adc122s051",	(kernel_ulong_t)&adc122s_config },
+	{ "adc122s101",	(kernel_ulong_t)&adc122s_config },
+	{ "adc124s021", (kernel_ulong_t)&adc124s_config },
+	{ "adc124s051", (kernel_ulong_t)&adc124s_config },
+	{ "adc124s101", (kernel_ulong_t)&adc124s_config },
+	{ "bd79104", (kernel_ulong_t)&bd79104_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, adc128_id);
 
 static const struct acpi_device_id adc128_acpi_match[] = {
-	{ "AANT1280", (kernel_ulong_t)&adc128_config[2] },
+	{ "AANT1280", (kernel_ulong_t)&adc124s_config },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
-- 
2.50.1

Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data
Posted by David Lechner 1 month, 3 weeks ago
On 8/14/25 3:35 AM, Matti Vaittinen wrote:
> The adc128s052 driver supports a few different ICs. IC specific
> configuration data is stored in an array. IC data, residing in a
> specific point of the array, is pointed from the SPI device match data.
> 
> There is no need to have the chip config data structures in an array
> and splitting them out of an array has at least following benefits:
> 
> - Chip-specific structures can be named after the chips they support.
>   This makes referring them a tad cleaner, compared to using a generic
>   array name with a numerical index.
> 
> - Avoid all potential 'out of bounds' errors which can result if the
>   array is changed.
> 
> Split the chip configuration data array to individual structures.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>
Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Thu, 14 Aug 2025 09:53:21 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/14/25 3:35 AM, Matti Vaittinen wrote:
> > The adc128s052 driver supports a few different ICs. IC specific
> > configuration data is stored in an array. IC data, residing in a
> > specific point of the array, is pointed from the SPI device match data.
> > 
> > There is no need to have the chip config data structures in an array
> > and splitting them out of an array has at least following benefits:
> > 
> > - Chip-specific structures can be named after the chips they support.
> >   This makes referring them a tad cleaner, compared to using a generic
> >   array name with a numerical index.
> > 
> > - Avoid all potential 'out of bounds' errors which can result if the
> >   array is changed.
> > 
> > Split the chip configuration data array to individual structures.
> > 
> > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > 
> > ---  
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 
Any racing series get to rebase on top of this.

Applied this patch as it is good in it's own right.

Thanks,

Jonathan
Re: [PATCH 2/3] iio: adc: adc128s052: Simplify matching chip_data
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Sat, 16 Aug 2025 13:28:23 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 14 Aug 2025 09:53:21 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 8/14/25 3:35 AM, Matti Vaittinen wrote:  
> > > The adc128s052 driver supports a few different ICs. IC specific
> > > configuration data is stored in an array. IC data, residing in a
> > > specific point of the array, is pointed from the SPI device match data.
> > > 
> > > There is no need to have the chip config data structures in an array
> > > and splitting them out of an array has at least following benefits:
> > > 
> > > - Chip-specific structures can be named after the chips they support.
> > >   This makes referring them a tad cleaner, compared to using a generic
> > >   array name with a numerical index.
> > > 
> > > - Avoid all potential 'out of bounds' errors which can result if the
> > >   array is changed.
> > > 
> > > Split the chip configuration data array to individual structures.
> > > 
> > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > > 
> > > ---    
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> >   
> Any racing series get to rebase on top of this.
> 
> Applied this patch as it is good in it's own right.
Dropped again given rename discussion on patch 3.
> 
> Thanks,
> 
> Jonathan
>