Hi, On 2/29/24 21:46, Pali Rohár wrote: > On Wednesday 28 February 2024 13:50:27 Hans de Goede wrote: >> Hi, >> >> On 2/27/24 23:37, Andy Shevchenko wrote: >>> On Tue, Feb 27, 2024 at 11:50 PM Pali Rohár <pali@xxxxxxxxxx> wrote: >>>> On Tuesday 27 February 2024 23:19:19 Andy Shevchenko wrote: >>>>> On Tue, Feb 27, 2024 at 11:04 PM Pali Rohár <pali@xxxxxxxxxx> wrote: >>> >>> ... >>> >>>>> I'm wondering why we need all this. We have notifiers when a device is >>>>> added / removed. We can provide a board_info for the device and attach >>>>> it to the proper adapter, no? >>>> >>>> I do not know how flexible are notifiers. Can notifier call our callback >>>> when new "struct i2c_adapter *adapter" was instanced? >>> >>> You can follow notifications of *an* I2C adapter being added / >>> removed. With that, you can filter which one is that. Based on that >>> you may attach a saved (at __init as you talked about in the reply to >>> Hans) board_info with all necessary information. >>> >>> Something like this (combined) >>> https://elixir.bootlin.com/linux/latest/source/drivers/ptp/ptp_ocp.c#L4515 >>> https://elixir.bootlin.com/linux/latest/source/drivers/input/mouse/psmouse-smbus.c#L194 >> >> drivers/platform/x86/touchscreen_dmi.c actually already does something >> like this for i2c-clients. The problem is that this brings probe-ordering >> problems with it. If the i801 driver is loaded before the dell-smo8800 >> driver then the notifiers will not trigger since the i2c-adapter has >> already been created (1). >> >> So we would still need a "cold-plug" manual scan in smo8800_probe() >> anyways at which point we might as well just return -EPROBE_DEFER >> when the adapter is not there. > > And that it example why the current existing solution is better, it does > not have such problems like the proposed. > >> As for Pali's suggestion of having the i2c-i801 code call a symbol >> exported by dell-smo8800 > > I did not suggest that! Please do not make false statements about me. In: https://lore.kernel.org/platform-driver-x86/20240227210429.l5o52wuexqqmrpol@pali/ You write: "Location of the quirks can be moved outside of the i2c-i801.c source code relatively easily without need to change the way how parent--child relationship currently works. Relevant functions is_dell_system_with_lis3lv02d() and register_dell_lis3lv02d_i2c_device() does not use internals of i2c-i801 and could be moved into new file, lets say drivers/platform/x86/dell/dell-smo8800-plat.c" Ok I see now you suggest moving the code to a new file, sorry for misreading that. But the point is that moving the code does not help because since there is a symbol used from the new code it will still get loaded on all machines were the i2c-i801.c driver gets loaded. So it will still be taking up RAM and increases boot time (loading an extra module consumes time) on machines which do not have any SMO88xx devices. And that point is still valid even independent of the code is moved to the existing dell-smo8800 module or to a new dell-smo8800-plat module. Regards, Hans
