Refactor the driver to use device match data instead of checking ID enums
in a switch statement.
Define a `ds4424_chip_info` structure to hold variant-specific attributes
(currently just the channel count) and attach it directly to the I2C and
OF device ID tables.
Use `client->name` instead of `id->name` to decouple the probe function
from the legacy `i2c_device_id` structure.
This simplifies the probe function and makes it easier to add support for
new variants like DS4402/DS4404.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v4:
- New patch
---
drivers/iio/dac/ds4424.c | 44 +++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index 3385f39521d9..5709d8a39d7b 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -34,9 +34,16 @@
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
}
-enum ds4424_device_ids {
- ID_DS4422,
- ID_DS4424,
+struct ds4424_chip_info {
+ u8 num_channels;
+};
+
+static const struct ds4424_chip_info ds4422_info = {
+ .num_channels = DS4422_MAX_DAC_CHANNELS,
+};
+
+static const struct ds4424_chip_info ds4424_info = {
+ .num_channels = DS4424_MAX_DAC_CHANNELS,
};
struct ds4424_data {
@@ -204,11 +211,15 @@ static const struct iio_info ds4424_iio_info = {
static int ds4424_probe(struct i2c_client *client)
{
- const struct i2c_device_id *id = i2c_client_get_device_id(client);
+ const struct ds4424_chip_info *chip_info;
struct ds4424_data *data;
struct iio_dev *indio_dev;
int ret;
+ chip_info = i2c_get_match_data(client);
+ if (!chip_info)
+ return -ENODEV;
+
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
return -ENOMEM;
@@ -216,7 +227,6 @@ static int ds4424_probe(struct i2c_client *client)
data = iio_priv(indio_dev);
i2c_set_clientdata(client, indio_dev);
data->client = client;
- indio_dev->name = id->name;
data->vcc_reg = devm_regulator_get(&client->dev, "vcc");
if (IS_ERR(data->vcc_reg))
@@ -236,20 +246,8 @@ static int ds4424_probe(struct i2c_client *client)
if (ret < 0)
goto fail;
- switch (id->driver_data) {
- case ID_DS4422:
- indio_dev->num_channels = DS4422_MAX_DAC_CHANNELS;
- break;
- case ID_DS4424:
- indio_dev->num_channels = DS4424_MAX_DAC_CHANNELS;
- break;
- default:
- dev_err(&client->dev,
- "ds4424: Invalid chip id.\n");
- ret = -ENXIO;
- goto fail;
- }
-
+ indio_dev->name = client->name;
+ indio_dev->num_channels = chip_info->num_channels;
indio_dev->channels = ds4424_channels;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &ds4424_iio_info;
@@ -278,16 +276,16 @@ static void ds4424_remove(struct i2c_client *client)
}
static const struct i2c_device_id ds4424_id[] = {
- { "ds4422", ID_DS4422 },
- { "ds4424", ID_DS4424 },
+ { "ds4422", (kernel_ulong_t)&ds4422_info },
+ { "ds4424", (kernel_ulong_t)&ds4424_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, ds4424_id);
static const struct of_device_id ds4424_of_match[] = {
- { .compatible = "maxim,ds4422" },
- { .compatible = "maxim,ds4424" },
+ { .compatible = "maxim,ds4422", .data = &ds4422_info },
+ { .compatible = "maxim,ds4424", .data = &ds4424_info },
{ }
};
--
2.47.3On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: > Refactor the driver to use device match data instead of checking ID enums > in a switch statement. > > Define a `ds4424_chip_info` structure to hold variant-specific attributes > (currently just the channel count) and attach it directly to the I2C and > OF device ID tables. > > Use `client->name` instead of `id->name` to decouple the probe function > from the legacy `i2c_device_id` structure. > > This simplifies the probe function and makes it easier to add support for > new variants like DS4402/DS4404. ... > - indio_dev->name = id->name; > + indio_dev->name = client->name; Isn't this an ABI breakage? -- With Best Regards, Andy Shevchenko
On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote: > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: > > Refactor the driver to use device match data instead of checking ID enums > > in a switch statement. > > > > Define a `ds4424_chip_info` structure to hold variant-specific attributes > > (currently just the channel count) and attach it directly to the I2C and > > OF device ID tables. > > > > Use `client->name` instead of `id->name` to decouple the probe function > > from the legacy `i2c_device_id` structure. > > > > This simplifies the probe function and makes it easier to add support for > > new variants like DS4402/DS4404. > > ... > > > - indio_dev->name = id->name; > > > + indio_dev->name = client->name; > > Isn't this an ABI breakage? I can't confirm it. before all patches: root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name ds4424 after: root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name ds4424 -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Feb 03, 2026 at 11:17:42AM +0100, Oleksij Rempel wrote: > On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote: > > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: ... > > > - indio_dev->name = id->name; > > > > > + indio_dev->name = client->name; > > > > Isn't this an ABI breakage? > > I can't confirm it. > > before all patches: > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > ds4424 > > after: > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > ds4424 In ACPI case it might look differently, but I have no means to test this. id->name comes strictly from an i2c table, while client->name is constructed using specifics of the firmware enumeration. In DT due to some (historical?) reasons the client->name has no vendor substring and hence matches 1:1 to id->name. In ACPI, IIRC, the client->name is ACPI device instance name, something like ABCD0123:00. -- With Best Regards, Andy Shevchenko
On Tue, Feb 03, 2026 at 01:51:23PM +0200, Andy Shevchenko wrote: > On Tue, Feb 03, 2026 at 11:17:42AM +0100, Oleksij Rempel wrote: > > On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote: > > > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: > > ... > > > > > - indio_dev->name = id->name; > > > > > > > + indio_dev->name = client->name; > > > > > > Isn't this an ABI breakage? > > > > I can't confirm it. > > > > before all patches: > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > ds4424 > > > > after: > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > ds4424 > > In ACPI case it might look differently, but I have no means to test this. > > id->name comes strictly from an i2c table, while client->name is constructed > using specifics of the firmware enumeration. In DT due to some (historical?) > reasons the client->name has no vendor substring and hence matches 1:1 to > id->name. In ACPI, IIRC, the client->name is ACPI device instance name, > something like ABCD0123:00. Ok, I see. Should I revert this line? Thank you, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Feb 03, 2026 at 01:00:02PM +0100, Oleksij Rempel wrote: > On Tue, Feb 03, 2026 at 01:51:23PM +0200, Andy Shevchenko wrote: > > On Tue, Feb 03, 2026 at 11:17:42AM +0100, Oleksij Rempel wrote: > > > On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote: > > > > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: ... > > > > > - indio_dev->name = id->name; > > > > > > > > > + indio_dev->name = client->name; > > > > > > > > Isn't this an ABI breakage? > > > > > > I can't confirm it. > > > > > > before all patches: > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > > ds4424 > > > > > > after: > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > > ds4424 > > > > In ACPI case it might look differently, but I have no means to test this. > > > > id->name comes strictly from an i2c table, while client->name is constructed > > using specifics of the firmware enumeration. In DT due to some (historical?) > > reasons the client->name has no vendor substring and hence matches 1:1 to > > id->name. In ACPI, IIRC, the client->name is ACPI device instance name, > > something like ABCD0123:00. > > Ok, I see. Should I revert this line? Just do not introduce that change (change of the ->name field) in the original patch, in that case no revert churn would be needed. -- With Best Regards, Andy Shevchenko
On Tue, 3 Feb 2026 16:56:00 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Tue, Feb 03, 2026 at 01:00:02PM +0100, Oleksij Rempel wrote: > > On Tue, Feb 03, 2026 at 01:51:23PM +0200, Andy Shevchenko wrote: > > > On Tue, Feb 03, 2026 at 11:17:42AM +0100, Oleksij Rempel wrote: > > > > On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote: > > > > > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote: > > ... > > > > > > > - indio_dev->name = id->name; > > > > > > > > > > > + indio_dev->name = client->name; > > > > > > > > > > Isn't this an ABI breakage? > > > > > > > > I can't confirm it. > > > > > > > > before all patches: > > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > > > ds4424 > > > > > > > > after: > > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name > > > > ds4424 > > > > > > In ACPI case it might look differently, but I have no means to test this. > > > > > > id->name comes strictly from an i2c table, while client->name is constructed > > > using specifics of the firmware enumeration. In DT due to some (historical?) > > > reasons the client->name has no vendor substring and hence matches 1:1 to > > > id->name. In ACPI, IIRC, the client->name is ACPI device instance name, > > > something like ABCD0123:00. > > > > Ok, I see. Should I revert this line? > > Just do not introduce that change (change of the ->name field) in the original > patch, in that case no revert churn would be needed. > I think this got dealt with in discussion of next version but safest route is just have an extra copy of the name in the chip_info structure. Then we know it's stable against different firmware types etc. The few places we have client->name are all ancient bugs that are really hard to fix years later without risk :( Jonathan
On Thu, Feb 05, 2026 at 08:43:25PM +0000, Jonathan Cameron wrote:
> On Tue, 3 Feb 2026 16:56:00 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Tue, Feb 03, 2026 at 01:00:02PM +0100, Oleksij Rempel wrote:
> > > On Tue, Feb 03, 2026 at 01:51:23PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Feb 03, 2026 at 11:17:42AM +0100, Oleksij Rempel wrote:
> > > > > On Tue, Feb 03, 2026 at 12:03:23PM +0200, Andy Shevchenko wrote:
> > > > > > On Tue, Feb 03, 2026 at 10:34:26AM +0100, Oleksij Rempel wrote:
> >
> > ...
> >
> > > > > > > - indio_dev->name = id->name;
> > > > > >
> > > > > > > + indio_dev->name = client->name;
> > > > > >
> > > > > > Isn't this an ABI breakage?
> > > > >
> > > > > I can't confirm it.
> > > > >
> > > > > before all patches:
> > > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name
> > > > > ds4424
> > > > >
> > > > > after:
> > > > > root@DistroKit:~ cat /sys/bus/iio/devices/iio:device3/name
> > > > > ds4424
> > > >
> > > > In ACPI case it might look differently, but I have no means to test this.
> > > >
> > > > id->name comes strictly from an i2c table, while client->name is constructed
> > > > using specifics of the firmware enumeration. In DT due to some (historical?)
> > > > reasons the client->name has no vendor substring and hence matches 1:1 to
> > > > id->name. In ACPI, IIRC, the client->name is ACPI device instance name,
> > > > something like ABCD0123:00.
> > >
> > > Ok, I see. Should I revert this line?
> >
> > Just do not introduce that change (change of the ->name field) in the original
> > patch, in that case no revert churn would be needed.
> >
> I think this got dealt with in discussion of next version but
> safest route is just have an extra copy of the name in the
> chip_info structure. Then we know it's stable against different
> firmware types etc.
Something like this?
diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index ccf36d3e0443..ffa65b22a1fd 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -50,6 +50,7 @@
}
struct ds4424_chip_info {
+ const char *name;
int vref_mV;
int scale_denom;
u8 result_mask;
@@ -57,6 +58,7 @@ struct ds4424_chip_info {
};
static const struct ds4424_chip_info ds4402_info = {
+ .name = "ds4402",
.vref_mV = 1230,
.scale_denom = 4,
.result_mask = DS4404_DAC_MASK,
@@ -64,6 +66,7 @@ static const struct ds4424_chip_info ds4402_info = {
};
static const struct ds4424_chip_info ds4404_info = {
+ .name = "ds4404",
.vref_mV = 1230,
.scale_denom = 4,
.result_mask = DS4404_DAC_MASK,
@@ -71,6 +74,7 @@ static const struct ds4424_chip_info ds4404_info = {
};
static const struct ds4424_chip_info ds4422_info = {
+ .name = "ds4422",
.vref_mV = 976,
.scale_denom = 16,
.result_mask = DS4424_DAC_MASK,
@@ -78,6 +82,7 @@ static const struct ds4424_chip_info ds4422_info = {
};
static const struct ds4424_chip_info ds4424_info = {
+ .name = "ds4424",
.vref_mV = 976,
.scale_denom = 16,
.result_mask = DS4424_DAC_MASK,
@@ -335,7 +340,7 @@ static int ds4424_probe(struct i2c_client *client)
data = iio_priv(indio_dev);
i2c_set_clientdata(client, indio_dev);
- indio_dev->name = id->name;
+ indio_dev->name = chip_info->name;
data->chip_info = chip_info;
data->vcc_reg = devm_regulator_get(&client->dev, "vcc");
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Feb 06, 2026 at 08:57:07AM +0100, Oleksij Rempel wrote: > On Thu, Feb 05, 2026 at 08:43:25PM +0000, Jonathan Cameron wrote: > > On Tue, 3 Feb 2026 16:56:00 +0200 > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: ... > > > Just do not introduce that change (change of the ->name field) in the original > > > patch, in that case no revert churn would be needed. > > > > > I think this got dealt with in discussion of next version but > > safest route is just have an extra copy of the name in the > > chip_info structure. Then we know it's stable against different > > firmware types etc. > > Something like this? Yes, but make it in the patch that introduces DT support. -- With Best Regards, Andy Shevchenko
Hi Andy, On Fri, Feb 06, 2026 at 11:59:50AM +0200, Andy Shevchenko wrote: > On Fri, Feb 06, 2026 at 08:57:07AM +0100, Oleksij Rempel wrote: > > On Thu, Feb 05, 2026 at 08:43:25PM +0000, Jonathan Cameron wrote: > > > On Tue, 3 Feb 2026 16:56:00 +0200 > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > ... > > > > > Just do not introduce that change (change of the ->name field) in the original > > > > patch, in that case no revert churn would be needed. > > > > > > > I think this got dealt with in discussion of next version but > > > safest route is just have an extra copy of the name in the > > > chip_info structure. Then we know it's stable against different > > > firmware types etc. > > > > Something like this? > > Yes, but make it in the patch that introduces DT support. Hm, I'm not sure what do you mean. There is no patch which "introduces DT support" in this series. Do you mean, this one: https://lore.kernel.org/all/20260204140045.390677-7-o.rempel@pengutronix.de/ Or should it be better a separate patch? Best Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, Feb 09, 2026 at 10:22:13AM +0100, Oleksij Rempel wrote: > On Fri, Feb 06, 2026 at 11:59:50AM +0200, Andy Shevchenko wrote: > > On Fri, Feb 06, 2026 at 08:57:07AM +0100, Oleksij Rempel wrote: > > > On Thu, Feb 05, 2026 at 08:43:25PM +0000, Jonathan Cameron wrote: > > > > On Tue, 3 Feb 2026 16:56:00 +0200 > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: ... > > > > > Just do not introduce that change (change of the ->name field) in the original > > > > > patch, in that case no revert churn would be needed. > > > > > > > > > I think this got dealt with in discussion of next version but > > > > safest route is just have an extra copy of the name in the > > > > chip_info structure. Then we know it's stable against different > > > > firmware types etc. > > > > > > Something like this? > > > > Yes, but make it in the patch that introduces DT support. > > Hm, I'm not sure what do you mean. There is no patch which "introduces > DT support" in this series. Do you mean, this one: > https://lore.kernel.org/all/20260204140045.390677-7-o.rempel@pengutronix.de/ Yes, I meant that one. > Or should it be better a separate patch? Perhaps just after the above mentioned one. Either works for me. -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.