Messages in this thread | | | Date | Mon, 2 May 2022 09:16:23 -0700 | Subject | Re: [PATCH v2] hwmon: (tmp401) Add of_match_table | From | Guenter Roeck <> |
| |
On 5/2/22 07:58, Vincent Whitchurch wrote: > On Mon, May 02, 2022 at 03:57:50PM +0200, Guenter Roeck wrote: >> On 5/2/22 02:19, Camel Guo wrote: >>> When tmp401 is built as kernel module, it won't be automatically loaded >>> even through there is a device node in the devicetree. e.g: >>> i2c { >>> #address-cells = <1>; >>> #size-cells = <0>; >>> >>> sensor@4c { >>> compatible = "ti,tmp401"; >>> reg = <0x4c>; >>> }; >>> }; >>> In order to make sure it is loaded automatically, this commit adds >>> of_match_table for tmp401. >>> >> >> As mentioned before, historically i2c devices would instantiate based >> on the i2c match table. You are claiming that this is no longer the case. > > Note that while the commit message in the first version of the patch did > wrongly claim that probe would not work without the of_match_table, this > corrected description in v2 does mention the actual problem: that the > module will not be automatically loaded without the of_match_table. (If > the module is loaded manually or the driver is built-in to the kernel, > there is no problem.) >
No, it doesn't. None of the information you provided below is mentioned in the description, but is essential to understand your patch and the reason for it.
> See commit 72fc64c68decf119466 ("hwmon: (tmp103) Add OF device ID > table") or commit 98b16a09861aa85d6 ("hwmon: (max31785) Add OF device ID > table") for similar changes to other hwmon drivers. >
Those commits provide a valid and acceptable explanation.
> The potential future change mentioned in the commit messages of > 72fc64c68decf119466 and 98b16a09861aa85d6 happened in commit > af503716ac1444db61d80 ("i2c: core: report OF style module alias for > devices registered via OF"). The commit message of > af503716ac1444db61d80 has a lot of details about the change being made, > and while it says that all in-tree drivers had been converted, it looks > like some of them, like tmp401, were missed. >
And this is the missing link. If you provide that information in the commit log I have no problems. Please also provide a Fixes: tag.
Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
Thanks, Guenter
>> The above is no evidence; that would require a log output on an affected >> system showing that the sensors are not or no longer longer instantiated. > > A log would simply show nothing happening so that's probably not going > to be that useful, but here is what the modaliases look like. As you > can see, the modalias of the device in sysfs does not match any of the > alias patterns of the module without this patch: > > $ cat /sys/bus/i2c/devices/4-004c/modalias > of:Ntemperature-sensorT<NULL>Cti,tmp431 > > modinfo without this patch: > > $ modinfo ./modules/lib/modules/5.18.0-rc1/kernel/drivers/hwmon/tmp401.ko > filename: /storage2/femfyra/linux-2.6/.roadtest/./modules/lib/modules/5.18.0-rc1/kernel/drivers/hwmon/tmp401.ko > license: GPL > description: Texas Instruments TMP401 temperature sensor driver > author: Hans de Goede <hdegoede@redhat.com> > alias: i2c:tmp435 > alias: i2c:tmp432 > alias: i2c:tmp431 > alias: i2c:tmp411 > alias: i2c:tmp401 > depends: > intree: Y > name: tmp401 > vermagic: 5.18.0-rc1 mod_unload > > modinfo after this patch: > > $ modinfo ./modules/lib/modules/5.18.0-rc1/kernel/drivers/hwmon/tmp401.ko > filename: /storage2/femfyra/linux-2.6/./.roadtest/modules/lib/modules/5.18.0-rc1/kernel/drivers/hwmon/tmp401.ko > license: GPL > description: Texas Instruments TMP401 temperature sensor driver > author: Hans de Goede <hdegoede@redhat.com> > alias: i2c:tmp435 > alias: i2c:tmp432 > alias: i2c:tmp431 > alias: i2c:tmp411 > alias: i2c:tmp401 > alias: of:N*T*Cti,tmp435C* > alias: of:N*T*Cti,tmp435 > alias: of:N*T*Cti,tmp432C* > alias: of:N*T*Cti,tmp432 > alias: of:N*T*Cti,tmp431C* > alias: of:N*T*Cti,tmp431 > alias: of:N*T*Cti,tmp411C* > alias: of:N*T*Cti,tmp411 > alias: of:N*T*Cti,tmp401C* > alias: of:N*T*Cti,tmp401 > depends: > intree: Y > name: tmp401 > vermagic: 5.18.0-rc1 mod_unload
| |