Message ID | 80c4a898-5867-4162-ac85-bdf7c7c68746@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | i2c: Remove I2C_COMPAT config symbol and related code | expand |
Hi Heiner, On Wed, Aug 21, 2024 at 10:13:04PM +0200, Heiner Kallweit wrote: > This code was added with 2bb5095affdb ("i2c: Provide compatibility links > for i2c adapters"). Commit message stated: Provide compatibility links > for [...] the time being. We will remove them after a long transition > period. > 15 years should have been a long enough transition period. Well, in general, I totally agree. It seems, however, that this slipped through the cracks. My Debian kernelconfig has I2C_COMPAT still enabled, so I am reluctant to remove it from one kernel release to the next. I wonder if we need some printout that it is really going away for a few kernel releases. Opinions? Jean? Happy hacking, Wolfram
On 01.09.2024 11:06, Wolfram Sang wrote: > Hi Heiner, > > On Wed, Aug 21, 2024 at 10:13:04PM +0200, Heiner Kallweit wrote: >> This code was added with 2bb5095affdb ("i2c: Provide compatibility links >> for i2c adapters"). Commit message stated: Provide compatibility links >> for [...] the time being. We will remove them after a long transition >> period. >> 15 years should have been a long enough transition period. > > Well, in general, I totally agree. > > It seems, however, that this slipped through the cracks. My Debian > kernelconfig has I2C_COMPAT still enabled, so I am reluctant to remove > it from one kernel release to the next. > One reason may be that the default for I2C_COMPAT is still y. We should switch this to n, even though it doesn't help in cases like make oldconfig. > I wonder if we need some printout that it is really going away for a few > kernel releases. > This may be a compromise. However I wonder how likely it is that somebody uses the latest kernel and hasn't updated his userspace tools for > 15 yrs. > Opinions? Jean? > > Happy hacking, > > Wolfram > Heiner
> > It seems, however, that this slipped through the cracks. My Debian > > kernelconfig has I2C_COMPAT still enabled, so I am reluctant to remove > > it from one kernel release to the next. > > > One reason may be that the default for I2C_COMPAT is still y. > We should switch this to n, even though it doesn't help in cases like > make oldconfig. Yes, but we should still do it. > > I wonder if we need some printout that it is really going away for a few > > kernel releases. > > > This may be a compromise. > However I wonder how likely it is that somebody uses the latest kernel > and hasn't updated his userspace tools for > 15 yrs. I am not worrying about using old versions of lmsensors, this is super unlikely. I worry about custom applications using the deprecated path. Like, IPMI docs advertised this path up to now (I fixed it). And why should one change that path if the kernel is not complaining?
On 01.09.2024 11:06, Wolfram Sang wrote: > Hi Heiner, > > On Wed, Aug 21, 2024 at 10:13:04PM +0200, Heiner Kallweit wrote: >> This code was added with 2bb5095affdb ("i2c: Provide compatibility links >> for i2c adapters"). Commit message stated: Provide compatibility links >> for [...] the time being. We will remove them after a long transition >> period. >> 15 years should have been a long enough transition period. > > Well, in general, I totally agree. > > It seems, however, that this slipped through the cracks. My Debian > kernelconfig has I2C_COMPAT still enabled, so I am reluctant to remove > it from one kernel release to the next. > > I wonder if we need some printout that it is really going away for a few > kernel releases. > I'm not aware of an option to only warn if the deprecated ABI is accessed by userspace. So we would have to warn every user if I2C_COMPAT is enabled. That's something Greg doesn't like. I see his point, but not sure how to do better. > Opinions? Jean? > > Happy hacking, > > Wolfram >
On Wed, Aug 21, 2024 at 10:13:04PM +0200, Heiner Kallweit wrote: > This code was added with 2bb5095affdb ("i2c: Provide compatibility links > for i2c adapters"). Commit message stated: Provide compatibility links > for [...] the time being. We will remove them after a long transition > period. > 15 years should have been a long enough transition period. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Wed, Aug 21, 2024 at 10:13:04PM +0200, Heiner Kallweit wrote: > This code was added with 2bb5095affdb ("i2c: Provide compatibility links > for i2c adapters"). Commit message stated: Provide compatibility links > for [...] the time being. We will remove them after a long transition > period. > 15 years should have been a long enough transition period. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks!
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 44710267d..c232054fd 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -40,14 +40,6 @@ config I2C_BOARDINFO bool default y -config I2C_COMPAT - bool "Enable compatibility bits for old user-space" - default y - help - Say Y here if you intend to run lm-sensors 3.1.1 or older, or any - other user-space package which expects i2c adapters to be class - devices. If you don't know, say Y. - config I2C_CHARDEV tristate "I2C device interface" help diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 6cf57e321..79292bb33 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1367,10 +1367,6 @@ struct i2c_adapter *i2c_verify_adapter(struct device *dev) } EXPORT_SYMBOL(i2c_verify_adapter); -#ifdef CONFIG_I2C_COMPAT -static struct class_compat *i2c_adapter_compat_class; -#endif - static void i2c_scan_static_board_info(struct i2c_adapter *adapter) { struct i2c_devinfo *devinfo; @@ -1547,14 +1543,6 @@ static int i2c_register_adapter(struct i2c_adapter *adap) dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name); -#ifdef CONFIG_I2C_COMPAT - res = class_compat_create_link(i2c_adapter_compat_class, &adap->dev, - adap->dev.parent); - if (res) - dev_warn(&adap->dev, - "Failed to create compatibility class link\n"); -#endif - /* create pre-declared device nodes */ of_i2c_register_devices(adap); i2c_acpi_install_space_handler(adap); @@ -1761,11 +1749,6 @@ void i2c_del_adapter(struct i2c_adapter *adap) device_for_each_child(&adap->dev, NULL, __unregister_client); device_for_each_child(&adap->dev, NULL, __unregister_dummy); -#ifdef CONFIG_I2C_COMPAT - class_compat_remove_link(i2c_adapter_compat_class, &adap->dev, - adap->dev.parent); -#endif - /* device name is gone after device_unregister */ dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name); @@ -2074,13 +2057,6 @@ static int __init i2c_init(void) i2c_debugfs_root = debugfs_create_dir("i2c", NULL); -#ifdef CONFIG_I2C_COMPAT - i2c_adapter_compat_class = class_compat_register("i2c-adapter"); - if (!i2c_adapter_compat_class) { - retval = -ENOMEM; - goto bus_err; - } -#endif retval = i2c_add_driver(&dummy_driver); if (retval) goto class_err; @@ -2093,10 +2069,6 @@ static int __init i2c_init(void) return 0; class_err: -#ifdef CONFIG_I2C_COMPAT - class_compat_unregister(i2c_adapter_compat_class); -bus_err: -#endif is_registered = false; bus_unregister(&i2c_bus_type); return retval; @@ -2109,9 +2081,6 @@ static void __exit i2c_exit(void) if (IS_ENABLED(CONFIG_OF_DYNAMIC)) WARN_ON(of_reconfig_notifier_unregister(&i2c_of_notifier)); i2c_del_driver(&dummy_driver); -#ifdef CONFIG_I2C_COMPAT - class_compat_unregister(i2c_adapter_compat_class); -#endif debugfs_remove_recursive(i2c_debugfs_root); bus_unregister(&i2c_bus_type); tracepoint_synchronize_unregister();
This code was added with 2bb5095affdb ("i2c: Provide compatibility links for i2c adapters"). Commit message stated: Provide compatibility links for [...] the time being. We will remove them after a long transition period. 15 years should have been a long enough transition period. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/i2c/Kconfig | 8 -------- drivers/i2c/i2c-core-base.c | 31 ------------------------------- 2 files changed, 39 deletions(-)