Re: [PATCH v2] iio: dac: dac5571: Fix chip id detection for OF devices

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

 



On Fri, Mar 25, 2022 at 6:55 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> From: Jose Cazarin <joseespiriki@xxxxxxxxx>
>
> 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.

It's a good fix, but...

...

> +       if (dev->of_node)

Why OF nodes?!

> +               chip_id = (uintptr_t)device_get_match_data(dev);
> +       else
> +               chip_id = id->driver_data;

> +       spec = &dac5571_spec[chip_id];


What I would rather expect here is

const ... *spec;

spec = device_get_match_data(dev);
if (!spec)
  spec = (const ...)id->driver_data;

And don't use enum in the driver data with ugly castings.

-- 
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux