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: > > On Monday 19 February 2024 13:52:57 Andy Shevchenko wrote: > > > On Sat, Feb 17, 2024 at 11:33:21AM +0100, Hans de Goede wrote: > > > > On 2/13/24 17:30, Jean Delvare wrote: > > > > > On Sun, 7 Jan 2024 18:10:55 +0100, Pali Rohár wrote: > > > > >> On Saturday 06 January 2024 17:09:29 Hans de Goede wrote: > > > > > > FWIW, I agree with Hans on the location of the HW quirks. > > > If there is a possible way to make the actual driver cleaner > > > and collect quirks somewhere else, I'm full support for that. > > > > 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 > > Put this file under a new hidden "bool" config option which is auto > > enabled when CONFIG_DELL_SMO8800 is used. > > > > i2c-i801.c currently has code: > > > > if (is_dell_system_with_lis3lv02d()) > > register_dell_lis3lv02d_i2c_device(priv); > > > > This can be put into a new exported function, e.g. > > void dell_smo8800_scan_i2c(struct i2c_adapter *adapter); > > And i2c-i801.c would call it instead. > > > > register_dell_lis3lv02d_i2c_device just needs "adapter", it does not > > need whole i801 priv struct. > > 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? > > With this simple change all dell smo8800 code would be in its subdir > > drivers/platform/x86/dell/ and i2c-i801.c would get rid of smo code. > > > > This approach does not change any functionality, so should be absolutely > > safe. > > > > Future changes will be done only in drivers/platform/x86/dell/ subdir, > > touching i801 would not be needed at all. > > Still these exported functions are not the best solution we can do, > right? We should be able to decouple them without need for the custom > APIs. Well, what I described here is a simple change which get rid of the one problem: i2c-i801.c contains SMO88xx related code and changing SMO88xx logic (like adding a new device id) requires touching unrelated i2c-i801.c source file. I like small changes which can be easily reviewed and address one problem. Step by step. That is why I proposed it here. For decoupling it is needed to get newly instanced adapter (if the mentioned notifier is able to tell this information) and also it is needed to check if the adapter is the i801.
