On 1/30/21 7:43 AM, Matwey V. Kornilov wrote: > > > сб, 30 янв. 2021 г. в 18:31, Guenter Roeck <linux@xxxxxxxxxxxx <mailto:linux@xxxxxxxxxxxx>>: >> >> On 1/30/21 2:10 AM, Matwey V. Kornilov wrote: >> > There is a logical flaw in lm75_probe() function introduced in >> > >> > e97a45f1b460 ("hwmon: (lm75) Add OF device ID table") >> > >> > Note, that of_device_get_match_data() returns NULL when no match >> > found. This is the case when OF node exists but has unknown >> > compatible line, while the module is still loaded via i2c >> > detection. >> > >> > arch/powerpc/boot/dts/fsl/p2041rdb.dts: >> > >> > lm75b@48 { >> > compatible = "nxp,lm75a"; >> > reg = <0x48>; >> > }; >> > >> > In this case, the sensor is mistakenly considered as ADT75 variant. >> > The simplest way to handle this issue is to make the LM75 code >> > zero. >> > >> >> This doesn't really solve the problem since it would match _all_ >> non-existing entries with lm75 (instead of whatever is intended). > > Just exactly how it happened before e97a45f1b460 > >> That doesn't matter for lm75a, but it would matter if someone >> would enter, say, "bla,adt75". >> >> On a side note, "nxp,lm75a" (nor "nxp,lm75", for that matter) is not a >> documented compatible string for this driver. If anything, we would >> need a means to explicitly reject such undefined compatible strings. >> One option might be to define the first entry in enum lm75_type >> explicitly as invalid, check for it and reject it if returned. > > It is fine for me. I am afraid that this will break some dts files in the tree. > The following compatible strings missed in the driver are currently in use: > > ti,lm75 > nxp,lm75 > nxp,lm75a > > I suppose these boards currently rely on the i2c detection path. > Correct. But relying on a bug doesn't improve the situation. The above compatible strings need to be documented (and properly implemented), or removed. And we really need a better means to handle NULL returns from of_device_get_match_data(). Maybe we should use of_match_device() instead and check for a NULL return. Guenter
