diff mbox series

[2/2] iio: dac: dac5571: Fix chip id detection for OF devices

Message ID 20210723183114.26017-3-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series iio: ti-dac5571: Add TI DAC081C081 support | expand

Commit Message

Laurent Pinchart July 23, 2021, 6:31 p.m. UTC
From: Jose Cazarin <joseespiriki@gmail.com>

When matching an OF device, the match mechanism tries all components of
the compatible property. This can result with a device matched with a
compatible string that isn't the first in the compatible list. For
instance, with a compatible property set to

    compatible = "ti,dac081c081", "ti,dac5571";

the driver will match the second compatible string, as the first one
isn't listed in the of_device_id table. The device will however be named
"dac081c081" by the I2C core.

This causes an issue when identifying the chip. The probe function
receives a i2c_device_id that comes from the module's I2C device ID
table. There is no entry in that table for "dac081c081", which results
in a NULL pointer passed to the probe function.

To fix this, add chip_id information in the data field of the OF device
ID table, and retrieve it with of_device_get_match_data() for OF
devices.

Signed-off-by: Jose Cazarin <joseespiriki@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/iio/dac/ti-dac5571.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

Comments

kernel test robot July 23, 2021, 11:06 p.m. UTC | #1
Hi Laurent,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on robh/for-next v5.14-rc2 next-20210723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/iio-ti-dac5571-Add-TI-DAC081C081-support/20210724-023333
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-randconfig-s022-20210723 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/aea544dbbcecf5d723ede9b42a2da945ac5038f9
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Laurent-Pinchart/iio-ti-dac5571-Add-TI-DAC081C081-support/20210724-023333
        git checkout aea544dbbcecf5d723ede9b42a2da945ac5038f9
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/dac/ti-dac5571.c: In function 'dac5571_probe':
>> drivers/iio/dac/ti-dac5571.c:331:24: error: implicit declaration of function 'of_device_get_match_data'; did you mean 'device_get_match_data'? [-Werror=implicit-function-declaration]
     331 |   chip_id = (uintptr_t)of_device_get_match_data(dev);
         |                        ^~~~~~~~~~~~~~~~~~~~~~~~
         |                        device_get_match_data
   cc1: some warnings being treated as errors


vim +331 drivers/iio/dac/ti-dac5571.c

   306	
   307	static int dac5571_probe(struct i2c_client *client,
   308				 const struct i2c_device_id *id)
   309	{
   310		struct device *dev = &client->dev;
   311		const struct dac5571_spec *spec;
   312		struct dac5571_data *data;
   313		struct iio_dev *indio_dev;
   314		enum chip_id chip_id;
   315		int ret, i;
   316	
   317		indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
   318		if (!indio_dev)
   319			return -ENOMEM;
   320	
   321		data = iio_priv(indio_dev);
   322		i2c_set_clientdata(client, indio_dev);
   323		data->client = client;
   324	
   325		indio_dev->info = &dac5571_info;
   326		indio_dev->name = id->name;
   327		indio_dev->modes = INDIO_DIRECT_MODE;
   328		indio_dev->channels = dac5571_channels;
   329	
   330		if (dev->of_node)
 > 331			chip_id = (uintptr_t)of_device_get_match_data(dev);
   332		else
   333			chip_id = id->driver_data;
   334	
   335		spec = &dac5571_spec[chip_id];
   336	
   337		indio_dev->num_channels = spec->num_channels;
   338		data->spec = spec;
   339	
   340		data->vref = devm_regulator_get(dev, "vref");
   341		if (IS_ERR(data->vref))
   342			return PTR_ERR(data->vref);
   343	
   344		ret = regulator_enable(data->vref);
   345		if (ret < 0)
   346			return ret;
   347	
   348		mutex_init(&data->lock);
   349	
   350		switch (spec->num_channels) {
   351		case 1:
   352			data->dac5571_cmd = dac5571_cmd_single;
   353			data->dac5571_pwrdwn = dac5571_pwrdwn_single;
   354			break;
   355		case 4:
   356			data->dac5571_cmd = dac5571_cmd_quad;
   357			data->dac5571_pwrdwn = dac5571_pwrdwn_quad;
   358			break;
   359		default:
   360			goto err;
   361		}
   362	
   363		for (i = 0; i < spec->num_channels; i++) {
   364			ret = data->dac5571_cmd(data, i, 0);
   365			if (ret) {
   366				dev_err(dev, "failed to initialize channel %d to 0\n", i);
   367				goto err;
   368			}
   369		}
   370	
   371		ret = iio_device_register(indio_dev);
   372		if (ret)
   373			goto err;
   374	
   375		return 0;
   376	
   377	 err:
   378		regulator_disable(data->vref);
   379		return ret;
   380	}
   381	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Jonathan Cameron July 24, 2021, 2:43 p.m. UTC | #2
On Sat, 24 Jul 2021 03:06:54 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> From: Jose Cazarin <joseespiriki@gmail.com>

> 

> When matching an OF device, the match mechanism tries all components of

> the compatible property. This can result with a device matched with a

> compatible string that isn't the first in the compatible list. For

> instance, with a compatible property set to

> 

>     compatible = "ti,dac081c081", "ti,dac5571";

> 

> the driver will match the second compatible string, as the first one

> isn't listed in the of_device_id table. The device will however be named

> "dac081c081" by the I2C core.

> 

> This causes an issue when identifying the chip. The probe function

> receives a i2c_device_id that comes from the module's I2C device ID

> table. There is no entry in that table for "dac081c081", which results

> in a NULL pointer passed to the probe function.

> 

> To fix this, add chip_id information in the data field of the OF device

> ID table, and retrieve it with of_device_get_match_data() for OF

> devices.

> 

> Signed-off-by: Jose Cazarin <joseespiriki@gmail.com>

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Interesting problem that I hadn't previously realised could happen.

One request though, can we use device_get_match_data() here rather than
the of specific version?  Include property.h as well for that.

That should allow the same issue with compatible to work correctly when
using PRP0001 based ACPI methods. 
https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/acpi/bus.c#L891
Will result in acpi_of_device_get_match_data() being called which will
match to the of_device_id table.

Thanks,

Jonathan

> ---

> Changes since v1:

> 

> - Include linux/of_device.h

> ---

>  drivers/iio/dac/ti-dac5571.c | 28 ++++++++++++++++++----------

>  1 file changed, 18 insertions(+), 10 deletions(-)

> 

> diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c

> index 2a5ba1b08a1d..8ceb1b42b14e 100644

> --- a/drivers/iio/dac/ti-dac5571.c

> +++ b/drivers/iio/dac/ti-dac5571.c

> @@ -19,6 +19,7 @@

>  #include <linux/i2c.h>

>  #include <linux/module.h>

>  #include <linux/mod_devicetable.h>

> +#include <linux/of_device.h>

>  #include <linux/regulator/consumer.h>

>  

>  enum chip_id {

> @@ -311,6 +312,7 @@ static int dac5571_probe(struct i2c_client *client,

>  	const struct dac5571_spec *spec;

>  	struct dac5571_data *data;

>  	struct iio_dev *indio_dev;

> +	enum chip_id chip_id;

>  	int ret, i;

>  

>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));

> @@ -326,7 +328,13 @@ static int dac5571_probe(struct i2c_client *client,

>  	indio_dev->modes = INDIO_DIRECT_MODE;

>  	indio_dev->channels = dac5571_channels;

>  

> -	spec = &dac5571_spec[id->driver_data];

> +	if (dev->of_node)

> +		chip_id = (uintptr_t)of_device_get_match_data(dev);

> +	else

> +		chip_id = id->driver_data;

> +

> +	spec = &dac5571_spec[chip_id];

> +

>  	indio_dev->num_channels = spec->num_channels;

>  	data->spec = spec;

>  

> @@ -384,15 +392,15 @@ static int dac5571_remove(struct i2c_client *i2c)

>  }

>  

>  static const struct of_device_id dac5571_of_id[] = {

> -	{.compatible = "ti,dac5571"},

> -	{.compatible = "ti,dac6571"},

> -	{.compatible = "ti,dac7571"},

> -	{.compatible = "ti,dac5574"},

> -	{.compatible = "ti,dac6574"},

> -	{.compatible = "ti,dac7574"},

> -	{.compatible = "ti,dac5573"},

> -	{.compatible = "ti,dac6573"},

> -	{.compatible = "ti,dac7573"},

> +	{.compatible = "ti,dac5571", .data = (void *)single_8bit},

> +	{.compatible = "ti,dac6571", .data = (void *)single_10bit},

> +	{.compatible = "ti,dac7571", .data = (void *)single_12bit},

> +	{.compatible = "ti,dac5574", .data = (void *)quad_8bit},

> +	{.compatible = "ti,dac6574", .data = (void *)quad_10bit},

> +	{.compatible = "ti,dac7574", .data = (void *)quad_12bit},

> +	{.compatible = "ti,dac5573", .data = (void *)quad_8bit},

> +	{.compatible = "ti,dac6573", .data = (void *)quad_10bit},

> +	{.compatible = "ti,dac7573", .data = (void *)quad_12bit},

>  	{}

>  };

>  MODULE_DEVICE_TABLE(of, dac5571_of_id);
Laurent Pinchart July 24, 2021, 11:14 p.m. UTC | #3
Hi Jonathan,

On Sat, Jul 24, 2021 at 03:43:08PM +0100, Jonathan Cameron wrote:
> On Sat, 24 Jul 2021 03:06:54 +0300 Laurent Pinchart wrote:

> 

> > From: Jose Cazarin <joseespiriki@gmail.com>

> > 

> > When matching an OF device, the match mechanism tries all components of

> > the compatible property. This can result with a device matched with a

> > compatible string that isn't the first in the compatible list. For

> > instance, with a compatible property set to

> > 

> >     compatible = "ti,dac081c081", "ti,dac5571";

> > 

> > the driver will match the second compatible string, as the first one

> > isn't listed in the of_device_id table. The device will however be named

> > "dac081c081" by the I2C core.

> > 

> > This causes an issue when identifying the chip. The probe function

> > receives a i2c_device_id that comes from the module's I2C device ID

> > table. There is no entry in that table for "dac081c081", which results

> > in a NULL pointer passed to the probe function.

> > 

> > To fix this, add chip_id information in the data field of the OF device

> > ID table, and retrieve it with of_device_get_match_data() for OF

> > devices.

> > 

> > Signed-off-by: Jose Cazarin <joseespiriki@gmail.com>

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 

> Interesting problem that I hadn't previously realised could happen.

> 

> One request though, can we use device_get_match_data() here rather than

> the of specific version?  Include property.h as well for that.

> 

> That should allow the same issue with compatible to work correctly when

> using PRP0001 based ACPI methods. 

> https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/acpi/bus.c#L891

> Will result in acpi_of_device_get_match_data() being called which will

> match to the of_device_id table.


Good point. I wasn't aware of PRP0001. I'll submit a v2 with this fixed,
after giving a bit of time for additional review, if any (I'm in
particular interested in whether this issue should be fixed in
individual drivers or in the I2C core, as explained in the cover
letter)).

> > ---

> > Changes since v1:

> > 

> > - Include linux/of_device.h

> > ---

> >  drivers/iio/dac/ti-dac5571.c | 28 ++++++++++++++++++----------

> >  1 file changed, 18 insertions(+), 10 deletions(-)

> > 

> > diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c

> > index 2a5ba1b08a1d..8ceb1b42b14e 100644

> > --- a/drivers/iio/dac/ti-dac5571.c

> > +++ b/drivers/iio/dac/ti-dac5571.c

> > @@ -19,6 +19,7 @@

> >  #include <linux/i2c.h>

> >  #include <linux/module.h>

> >  #include <linux/mod_devicetable.h>

> > +#include <linux/of_device.h>

> >  #include <linux/regulator/consumer.h>

> >  

> >  enum chip_id {

> > @@ -311,6 +312,7 @@ static int dac5571_probe(struct i2c_client *client,

> >  	const struct dac5571_spec *spec;

> >  	struct dac5571_data *data;

> >  	struct iio_dev *indio_dev;

> > +	enum chip_id chip_id;

> >  	int ret, i;

> >  

> >  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));

> > @@ -326,7 +328,13 @@ static int dac5571_probe(struct i2c_client *client,

> >  	indio_dev->modes = INDIO_DIRECT_MODE;

> >  	indio_dev->channels = dac5571_channels;

> >  

> > -	spec = &dac5571_spec[id->driver_data];

> > +	if (dev->of_node)

> > +		chip_id = (uintptr_t)of_device_get_match_data(dev);

> > +	else

> > +		chip_id = id->driver_data;

> > +

> > +	spec = &dac5571_spec[chip_id];

> > +

> >  	indio_dev->num_channels = spec->num_channels;

> >  	data->spec = spec;

> >  

> > @@ -384,15 +392,15 @@ static int dac5571_remove(struct i2c_client *i2c)

> >  }

> >  

> >  static const struct of_device_id dac5571_of_id[] = {

> > -	{.compatible = "ti,dac5571"},

> > -	{.compatible = "ti,dac6571"},

> > -	{.compatible = "ti,dac7571"},

> > -	{.compatible = "ti,dac5574"},

> > -	{.compatible = "ti,dac6574"},

> > -	{.compatible = "ti,dac7574"},

> > -	{.compatible = "ti,dac5573"},

> > -	{.compatible = "ti,dac6573"},

> > -	{.compatible = "ti,dac7573"},

> > +	{.compatible = "ti,dac5571", .data = (void *)single_8bit},

> > +	{.compatible = "ti,dac6571", .data = (void *)single_10bit},

> > +	{.compatible = "ti,dac7571", .data = (void *)single_12bit},

> > +	{.compatible = "ti,dac5574", .data = (void *)quad_8bit},

> > +	{.compatible = "ti,dac6574", .data = (void *)quad_10bit},

> > +	{.compatible = "ti,dac7574", .data = (void *)quad_12bit},

> > +	{.compatible = "ti,dac5573", .data = (void *)quad_8bit},

> > +	{.compatible = "ti,dac6573", .data = (void *)quad_10bit},

> > +	{.compatible = "ti,dac7573", .data = (void *)quad_12bit},

> >  	{}

> >  };

> >  MODULE_DEVICE_TABLE(of, dac5571_of_id);

> 


-- 
Regards,

Laurent Pinchart
Wolfram Sang Aug. 17, 2021, 8:44 p.m. UTC | #4
> > When matching an OF device, the match mechanism tries all components of

> > the compatible property. This can result with a device matched with a

> > compatible string that isn't the first in the compatible list. For

> > instance, with a compatible property set to

> > 

> >     compatible = "ti,dac081c081", "ti,dac5571";

> > 

> > the driver will match the second compatible string, as the first one

> > isn't listed in the of_device_id table. The device will however be named

> > "dac081c081" by the I2C core.

> > 

> > This causes an issue when identifying the chip. The probe function

> > receives a i2c_device_id that comes from the module's I2C device ID

> > table. There is no entry in that table for "dac081c081", which results

> > in a NULL pointer passed to the probe function.

> > 

> > To fix this, add chip_id information in the data field of the OF device

> > ID table, and retrieve it with of_device_get_match_data() for OF

> > devices.

> > 

> > Signed-off-by: Jose Cazarin <joseespiriki@gmail.com>

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 

> Interesting problem that I hadn't previously realised could happen.

> 

> One request though, can we use device_get_match_data() here rather than

> the of specific version?  Include property.h as well for that.

> 

> That should allow the same issue with compatible to work correctly when

> using PRP0001 based ACPI methods. 

> https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/acpi/bus.c#L891

> Will result in acpi_of_device_get_match_data() being called which will

> match to the of_device_id table.


Couldn't you use the "new" probe_new() callback instead which will drop
the i2c_device_id? Kieran was interested in such conversions IIRC.
Laurent Pinchart Aug. 17, 2021, 8:52 p.m. UTC | #5
Hi Wolfram,

On Tue, Aug 17, 2021 at 10:44:20PM +0200, Wolfram Sang wrote:
> 

> > > When matching an OF device, the match mechanism tries all components of

> > > the compatible property. This can result with a device matched with a

> > > compatible string that isn't the first in the compatible list. For

> > > instance, with a compatible property set to

> > > 

> > >     compatible = "ti,dac081c081", "ti,dac5571";

> > > 

> > > the driver will match the second compatible string, as the first one

> > > isn't listed in the of_device_id table. The device will however be named

> > > "dac081c081" by the I2C core.

> > > 

> > > This causes an issue when identifying the chip. The probe function

> > > receives a i2c_device_id that comes from the module's I2C device ID

> > > table. There is no entry in that table for "dac081c081", which results

> > > in a NULL pointer passed to the probe function.

> > > 

> > > To fix this, add chip_id information in the data field of the OF device

> > > ID table, and retrieve it with of_device_get_match_data() for OF

> > > devices.

> > > 

> > > Signed-off-by: Jose Cazarin <joseespiriki@gmail.com>

> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > 

> > Interesting problem that I hadn't previously realised could happen.

> > 

> > One request though, can we use device_get_match_data() here rather than

> > the of specific version?  Include property.h as well for that.

> > 

> > That should allow the same issue with compatible to work correctly when

> > using PRP0001 based ACPI methods. 

> > https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/acpi/bus.c#L891

> > Will result in acpi_of_device_get_match_data() being called which will

> > match to the of_device_id table.

> 

> Couldn't you use the "new" probe_new() callback instead which will drop

> the i2c_device_id? Kieran was interested in such conversions IIRC.


It's a bit unrelated to this patch, but I can add another patch to the
series.

While I have your attention, there's a question for you in the cover
letter :-) Could you please have a look ?

-- 
Regards,

Laurent Pinchart
Wolfram Sang Aug. 17, 2021, 8:58 p.m. UTC | #6
> > Couldn't you use the "new" probe_new() callback instead which will drop

> > the i2c_device_id? Kieran was interested in such conversions IIRC.

> 

> It's a bit unrelated to this patch, but I can add another patch to the

> series.

> 

> While I have your attention, there's a question for you in the cover

> letter :-) Could you please have a look ?


? This was the answer to that question. Unless I misunderstood.
Laurent Pinchart Aug. 17, 2021, 9:20 p.m. UTC | #7
On Tue, Aug 17, 2021 at 10:58:19PM +0200, Wolfram Sang wrote:
> 
> > > Couldn't you use the "new" probe_new() callback instead which will drop
> > > the i2c_device_id? Kieran was interested in such conversions IIRC.
> > 
> > It's a bit unrelated to this patch, but I can add another patch to the
> > series.
> > 
> > While I have your attention, there's a question for you in the cover
> > letter :-) Could you please have a look ?
> 
> ? This was the answer to that question. Unless I misunderstood.

My point is that this patch shouldn't be needed. I'd like if the I2C
core could get the driver data from the i2c_device_id table instead of
duplicating it in the of_device_id. This isn't possible today as
i2c_match_id() doesn't have the fallback mechanism that OF matching has.
Laurent Pinchart March 24, 2022, 11:25 p.m. UTC | #8
Hi Wolfram,

On Wed, Aug 18, 2021 at 12:20:03AM +0300, Laurent Pinchart wrote:
> On Tue, Aug 17, 2021 at 10:58:19PM +0200, Wolfram Sang wrote:
> > 
> > > > Couldn't you use the "new" probe_new() callback instead which will drop
> > > > the i2c_device_id? Kieran was interested in such conversions IIRC.
> > > 
> > > It's a bit unrelated to this patch, but I can add another patch to the
> > > series.
> > > 
> > > While I have your attention, there's a question for you in the cover
> > > letter :-) Could you please have a look ?
> > 
> > ? This was the answer to that question. Unless I misunderstood.
> 
> My point is that this patch shouldn't be needed. I'd like if the I2C
> core could get the driver data from the i2c_device_id table instead of
> duplicating it in the of_device_id. This isn't possible today as
> i2c_match_id() doesn't have the fallback mechanism that OF matching has.

Any thought on this ?
diff mbox series

Patch

diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index 2a5ba1b08a1d..bd005b3a976b 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -311,6 +311,7 @@  static int dac5571_probe(struct i2c_client *client,
 	const struct dac5571_spec *spec;
 	struct dac5571_data *data;
 	struct iio_dev *indio_dev;
+	enum chip_id chip_id;
 	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -326,7 +327,13 @@  static int dac5571_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = dac5571_channels;
 
-	spec = &dac5571_spec[id->driver_data];
+	if (dev->of_node)
+		chip_id = (uintptr_t)of_device_get_match_data(dev);
+	else
+		chip_id = id->driver_data;
+
+	spec = &dac5571_spec[chip_id];
+
 	indio_dev->num_channels = spec->num_channels;
 	data->spec = spec;
 
@@ -384,15 +391,15 @@  static int dac5571_remove(struct i2c_client *i2c)
 }
 
 static const struct of_device_id dac5571_of_id[] = {
-	{.compatible = "ti,dac5571"},
-	{.compatible = "ti,dac6571"},
-	{.compatible = "ti,dac7571"},
-	{.compatible = "ti,dac5574"},
-	{.compatible = "ti,dac6574"},
-	{.compatible = "ti,dac7574"},
-	{.compatible = "ti,dac5573"},
-	{.compatible = "ti,dac6573"},
-	{.compatible = "ti,dac7573"},
+	{.compatible = "ti,dac5571", .data = (void *)single_8bit},
+	{.compatible = "ti,dac6571", .data = (void *)single_10bit},
+	{.compatible = "ti,dac7571", .data = (void *)single_12bit},
+	{.compatible = "ti,dac5574", .data = (void *)quad_8bit},
+	{.compatible = "ti,dac6574", .data = (void *)quad_10bit},
+	{.compatible = "ti,dac7574", .data = (void *)quad_12bit},
+	{.compatible = "ti,dac5573", .data = (void *)quad_8bit},
+	{.compatible = "ti,dac6573", .data = (void *)quad_10bit},
+	{.compatible = "ti,dac7573", .data = (void *)quad_12bit},
 	{}
 };
 MODULE_DEVICE_TABLE(of, dac5571_of_id);