diff mbox series

[2/2] driver core: class: warn if a compatibility class is registered

Message ID 7bc5fa50-59f6-4455-8f77-1c89f1e17d0b@gmail.com
State Rejected
Headers show
Series i2c: core: prepare dropping support for I2C_COMPAT | expand

Commit Message

Heiner Kallweit Sept. 2, 2024, 7:02 p.m. UTC
Kernel doc for this function states:
"Compatibility class are meant as a temporary user-space compatibility
workaround when converting a family of class devices to a bus devices."

Therefore remind any potential user of the old ABI that support for it
will be dropped soon.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/base/class.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Greg KH Sept. 2, 2024, 7:20 p.m. UTC | #1
On Mon, Sep 02, 2024 at 09:02:17PM +0200, Heiner Kallweit wrote:
> Kernel doc for this function states:
> "Compatibility class are meant as a temporary user-space compatibility
> workaround when converting a family of class devices to a bus devices."
> 
> Therefore remind any potential user of the old ABI that support for it
> will be dropped soon.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/base/class.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/base/class.c b/drivers/base/class.c
> index 7b38fdf8e..f12a43736 100644
> --- a/drivers/base/class.c
> +++ b/drivers/base/class.c
> @@ -556,6 +556,9 @@ struct class_compat *class_compat_register(const char *name)
>  {
>  	struct class_compat *cls;
>  
> +	pr_warn("Compatibility class %s will go away soon, please migrate userspace tools to use bus devices\n",
> +		name);

That's not going to do anything except annoy users who have no control
over this, sorry.  Please just fix up all of the kernel and then delete
this function.

thanks,

greg k-h
Wolfram Sang Sept. 3, 2024, 9:04 a.m. UTC | #2
> > +	pr_warn("Compatibility class %s will go away soon, please migrate userspace tools to use bus devices\n",
> > +		name);
> 
> That's not going to do anything except annoy users who have no control
> over this, sorry.  Please just fix up all of the kernel and then delete
> this function.

So, we deprecated this sysfs-class 15 years ago and hid it with a
Kconfig symbol. However, we never pursued this further, so e.g. Debian
has the Kconfig symbol still enabled. Can we really remove this from one
release to the next without another transition period? I am not afraid
of tools like lm-sensors which were converted long ago. But custom code
might rely on sysfs-paths created by this class. It was even advertised
in IPMI docs until last week (fixed now).
Wolfram Sang Sept. 3, 2024, 9:06 a.m. UTC | #3
On Tue, Sep 03, 2024 at 11:04:44AM +0200, Wolfram Sang wrote:
> 
> > > +	pr_warn("Compatibility class %s will go away soon, please migrate userspace tools to use bus devices\n",
> > > +		name);
> > 
> > That's not going to do anything except annoy users who have no control
> > over this, sorry.  Please just fix up all of the kernel and then delete
> > this function.
> 
> So, we deprecated this sysfs-class 15 years ago and hid it with a
> Kconfig symbol. However, we never pursued this further, so e.g. Debian
> has the Kconfig symbol still enabled. Can we really remove this from one
> release to the next without another transition period? I am not afraid
> of tools like lm-sensors which were converted long ago. But custom code
> might rely on sysfs-paths created by this class. It was even advertised
> in IPMI docs until last week (fixed now).

I missed that Heiner was changing the driver core, not I2C core. So, to
give more details, I am talking about I2C_COMPAT and the "i2c-adapter"
class. The main question from above still stands.
Greg KH Sept. 3, 2024, 10:06 a.m. UTC | #4
On Tue, Sep 03, 2024 at 11:06:44AM +0200, Wolfram Sang wrote:
> On Tue, Sep 03, 2024 at 11:04:44AM +0200, Wolfram Sang wrote:
> > 
> > > > +	pr_warn("Compatibility class %s will go away soon, please migrate userspace tools to use bus devices\n",
> > > > +		name);
> > > 
> > > That's not going to do anything except annoy users who have no control
> > > over this, sorry.  Please just fix up all of the kernel and then delete
> > > this function.
> > 
> > So, we deprecated this sysfs-class 15 years ago and hid it with a
> > Kconfig symbol. However, we never pursued this further, so e.g. Debian
> > has the Kconfig symbol still enabled. Can we really remove this from one
> > release to the next without another transition period? I am not afraid
> > of tools like lm-sensors which were converted long ago. But custom code
> > might rely on sysfs-paths created by this class. It was even advertised
> > in IPMI docs until last week (fixed now).
> 
> I missed that Heiner was changing the driver core, not I2C core. So, to
> give more details, I am talking about I2C_COMPAT and the "i2c-adapter"
> class. The main question from above still stands.

Delete the code and see if anyone notices?  :)

greg k-h
Wolfram Sang Sept. 3, 2024, 10:43 a.m. UTC | #5
> Delete the code and see if anyone notices?  :)

"Never ever break userspace, at least until Greg says so" :D

Seriously, Heiner initially sent a patch simply removing the code in
question [1]. May I interpret your above statement as "Acked-by"?

[1] https://lore.kernel.org/r/80c4a898-5867-4162-ac85-bdf7c7c68746@gmail.com
Greg KH Sept. 3, 2024, 10:50 a.m. UTC | #6
On Tue, Sep 03, 2024 at 12:43:52PM +0200, Wolfram Sang wrote:
> 
> > Delete the code and see if anyone notices?  :)
> 
> "Never ever break userspace, at least until Greg says so" :D

"Never break userspace in a way that anyone notices" is the real rule we
have :)

> Seriously, Heiner initially sent a patch simply removing the code in
> question [1]. May I interpret your above statement as "Acked-by"?
> 
> [1] https://lore.kernel.org/r/80c4a898-5867-4162-ac85-bdf7c7c68746@gmail.com

Ack now sent there, thanks!

greg k-h
Wolfram Sang Sept. 3, 2024, 11:09 a.m. UTC | #7
> "Never break userspace in a way that anyone notices" is the real rule we
> have :)

Noted!

> > Seriously, Heiner initially sent a patch simply removing the code in
> > question [1]. May I interpret your above statement as "Acked-by"?
> > 
> > [1] https://lore.kernel.org/r/80c4a898-5867-4162-ac85-bdf7c7c68746@gmail.com
> 
> Ack now sent there, thanks!

Thank you!
diff mbox series

Patch

diff --git a/drivers/base/class.c b/drivers/base/class.c
index 7b38fdf8e..f12a43736 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -556,6 +556,9 @@  struct class_compat *class_compat_register(const char *name)
 {
 	struct class_compat *cls;
 
+	pr_warn("Compatibility class %s will go away soon, please migrate userspace tools to use bus devices\n",
+		name);
+
 	cls = kmalloc(sizeof(struct class_compat), GFP_KERNEL);
 	if (!cls)
 		return NULL;