Re: [PATCH 1/6] drm/sysfs: Remove version attribute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01.10.2024 12:01, Dmitry Baryshkov wrote:
> On September 30, 2024 1:49:41 PM GMT+03:00, Heiner Kallweit <hkallweit1@xxxxxxxxx> wrote:
>> On 22.09.2024 16:55, Dmitry Baryshkov wrote:
>>> On Sun, Sep 08, 2024 at 02:08:58PM GMT, Heiner Kallweit wrote:
>>>> This undocumented attribute returns a version string which hasn't been
>>>> changed for ages. libdrm doesn't use it and I also found no other user.
>>>> So I think we can remove it.
>>>
>>> This file is a part of the ABI. Commit 82d5e73f6b79 ("drm: drop obsolete
>>> drm_core.h") replaced variable string with the fixed value that we
>>> currently have, but at the same it clearly documented that the file is
>>> being preserved for the sake of binary compatibility.
>>>
>> The drm version attribute is documented neither under Documentation/gpu
>> nor under Documentation/ABI. So do we really have to consider it
>> part of the ABI? And are you aware of any actual user of this attribute?
>>
>> The author of 82d5e73f6b79 wasn't sure either, and therefore didn't
>> dare to drop the attribute (8 yrs ago). He didn't make any statement that
>> the attribute is actually used.
> 
> A very quick search points out that the file is being used: 
> 
> <https://codesearch.debian.net/search?q=drm%2Fversion>
> 

Thanks. However this script doesn't actually use the version value
and would work perfectly fine also w/o this attribute.

> 
>>
>> 6.12-rc1 is just out, so we could drop the attribute in linux-next and
>> would have several weeks before the next merge window to find out
>> whether anybody complains.
> 
> No, this is not the way to treat userspace ABI.
> 

Mileage of subsystem maintainers seems to vary in this regard.
See following example where Greg supported such an approach.
https://www.spinics.net/lists/linux-i2c/msg71821.html
But fine with me, we can also leave the version attribute in.

>>
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>>>> ---
>>>>  drivers/gpu/drm/drm_sysfs.c | 12 ------------
>>>>  1 file changed, 12 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>>>> index fb3bbb6ad..49e5faf11 100644
>>>> --- a/drivers/gpu/drm/drm_sysfs.c
>>>> +++ b/drivers/gpu/drm/drm_sysfs.c
>>>> @@ -126,8 +126,6 @@ static const struct component_ops typec_connector_ops = {
>>>>  	.unbind = typec_connector_unbind,
>>>>  };
>>>>  
>>>> -static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
>>>> -
>>>>  /**
>>>>   * drm_sysfs_init - initialize sysfs helpers
>>>>   *
>>>> @@ -140,19 +138,10 @@ static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
>>>>   */
>>>>  int drm_sysfs_init(void)
>>>>  {
>>>> -	int err;
>>>> -
>>>>  	drm_class = class_create("drm");
>>>>  	if (IS_ERR(drm_class))
>>>>  		return PTR_ERR(drm_class);
>>>>  
>>>> -	err = class_create_file(drm_class, &class_attr_version.attr);
>>>> -	if (err) {
>>>> -		class_destroy(drm_class);
>>>> -		drm_class = NULL;
>>>> -		return err;
>>>> -	}
>>>> -
>>>>  	drm_class->devnode = drm_devnode;
>>>>  
>>>>  	drm_sysfs_acpi_register();
>>>> @@ -169,7 +158,6 @@ void drm_sysfs_destroy(void)
>>>>  	if (IS_ERR_OR_NULL(drm_class))
>>>>  		return;
>>>>  	drm_sysfs_acpi_unregister();
>>>> -	class_remove_file(drm_class, &class_attr_version.attr);
>>>>  	class_destroy(drm_class);
>>>>  	drm_class = NULL;
>>>>  }
>>>> -- 
>>>> 2.46.0
>>>>
>>>>
>>>
>>
> 
> 




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux