Re: [PATCH RESEND v2 32/32] drm/vkms: Introduce configfs for dynamic connector creation

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 12/19/25 21:47, Luca Ceresoli wrote:
> On Wed Oct 29, 2025 at 3:37 PM CET, Louis Chauvet wrote:
>> DRM allows the connector to be created after the device. To allows
>> emulating this, add two configfs attributes to connector to allows this.
>>
>> Using the dynamic attribute you can set if a connector will be dynamic or
>> not.
>> Using the enabled attribute, you can set at runtime if a dynamic connector
>> is present or not.
>>
>> Co-developed-by: José Expósito <jose.exposito89@gmail.com>
>> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
>> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
>> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> 
>> @@ -1215,8 +1223,10 @@ static ssize_t connector_type_store(struct config_item *item,
>>   	}
>>
>>   	scoped_guard(mutex, &connector->dev->lock) {
>> -		if (connector->dev->enabled)
>> -			return -EINVAL;
>> +		if (connector->dev->enabled) {
>> +			if (connector_is_enabled(connector->config))
>> +				return -EBUSY;
>> +		}
> 
> You are changing from -EINVAL to -EBUSY. Why? Was -EINVAL wrong in the
> first place?

EINVAL was wrong in the first place, we try to remove EBUSY when you 
can't do something because the device is already enabled and EINVAL when 
the value is false.

> 
> And you can avoid the nested if:
> 
> 		if (connector->dev->enabled && connector_is_enabled(connector->config))
> 			return -E<WHATEVER>;

It is probably a remaining of previous implementation, thanks!

>> +static ssize_t connector_dynamic_show(struct config_item *item, char *page)
>> +{
>> +	struct vkms_configfs_connector *connector;
>> +	bool enabled;
>               ^^^^^^^
>               dynamic
> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com