Device match data and DT compatible string fallback

Device match data and DT compatible string fallback

[Laurent Pinchart]

Hello,

I recently ran into an issue in a driver (drivers/iio/dac/ti-dac5571.c
to be precise, but the problem is more generic) that led to a crash at
probe time. The driver supports matching to both OF devices and platform
devices by specifying of_device_id and i2c_device_id tables. It supports
multiple devices, and stores device-specific data in the i2c_device_id
table that it retrieves at probe time through the i2c_device_id pointer
passed to the probe function.

The device I'm working with is an DAC081C081, which is compatible with
the DAC5571. It's an OF device that has the compatible property set to

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

The driver doesn't support the ti,dac081c081 compatible string, so the
device is matched to the driver using the compatible fallback
"ti,dac5571". This leads to the i2c_device_id passed to the proble
function being NULL, as the i2c_device_if table doesn't contain a
"dac5571" entry, and this results in a crash.

I could fix this in the driver by calling of_device_get_match_data() in
the probe function with dev->of_node is not NULL, but I feel this is
really an issue that should be handled by the framework. Has anyone ever
given it a thought ?

-- 
Regards,

Laurent Pinchart

Re: Device match data and DT compatible string fallback

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]


> I could fix this in the driver by calling of_device_get_match_data() in
> the probe function with dev->of_node is not NULL, but I feel this is
> really an issue that should be handled by the framework. Has anyone ever
> given it a thought ?

The of entries should also have .data entries and use that.

The driver could also be converted to probe_new to become independent of
the i2c_device_id.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Device match data and DT compatible string fallback

[Laurent Pinchart]

Hi Wolfram,

On Mon, Jun 28, 2021 at 05:55:01PM +0200, Wolfram Sang wrote:
> 
> > I could fix this in the driver by calling of_device_get_match_data() in
> > the probe function with dev->of_node is not NULL, but I feel this is
> > really an issue that should be handled by the framework. Has anyone ever
> > given it a thought ?
> 
> The of entries should also have .data entries and use that.

That doesn't help unfortunately, as the i2c_device_id passed to probe()
comes from the i2c_device_id table, the of_device_id table isn't
involved.

> The driver could also be converted to probe_new to become independent of
> the i2c_device_id.

Sure, but that doesn't solve the problem, it only forces the driver to
basically implement

	if (dev->of_node)
		get data using of_device_get_match_data();
	else
		get data using i2c_match_id();

Shouldn't the I2C subsystem provide a single function to let the driver
retrieve the match data, regardless of where it comes from ?

-- 
Regards,

Laurent Pinchart