Re: [PATCHv2 3/3] nvme: Expose cntrltype and dctype through sysfs

[Sagi Grimberg <sagi@grimberg.me>]

Hey Greg,

>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index b5e452aa3c0e..4e3db5ec3924 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -2990,6 +2990,9 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
>>   	ctrl->max_namespaces = le32_to_cpu(id->mnan);
>>   	ctrl->ctratt = le32_to_cpu(id->ctratt);
>>   
>> +	ctrl->cntrltype = id->cntrltype;
>> +	ctrl->dctype = id->dctype;
>> +
>>   	if (id->rtd3e) {
>>   		/* us -> s */
>>   		u32 transition_time = le32_to_cpu(id->rtd3e) / USEC_PER_SEC;
>> @@ -3115,6 +3118,10 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
>>   			return ret;
>>   	}
>>   
>> +	ret = device_update_groups(ctrl->device);
>> +	if (ret)
>> +		return ret;
> 
> Why is this needed here?  How did the class or type just change?  That
> should never change over the lifespan of a device.  If it does, you need
> to tear down the old device and create a new one as something really
> wrong just happened.

The class and type do not change. The fundamental need here is to update
visible attributes of the device.

In nvme, we create the device node with its sysfs attributes and next
comes a process where we interrogate it for capabilities/personality.
In this example, the attributes cntrltype and dctype are learned from
the device identification, and the desire is that dctype will only be
visible for specific cntrltype (i.e. cntrltype is I/O controllers vs.
discovery controllers and dctype is the discovery controller type).

So perhaps a more narrowed interface to update attributes
visibility of the controller would be more appropriate instead?

Something like:
--
	ret = device_update_visibility(ctrl->device->groups);
--