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