[PATCH 2/2] nvme: Expose cntrltype and dctype through sysfs.

[Hannes Reinecke]

On 2/3/22 12:59, Belanger, Martin wrote:
>> On 2/2/22 22:34, Martin Belanger wrote:
>>> From: Martin Belanger <martin.belanger at dell.com>
>>>
>>> TP8010 introduces the Discovery Controller Type (dctype) attribute.
>>> The dctype is returned in the response to the Identify command. This
>>> patch exposes the dctype through the sysfs. Since the dctype depends
>>> on the Controller Type (cntrltype), another attribute of the Identify
>>> response, the patch also exposes the cntrltype as well.
>>>
>>> Signed-off-by: Martin Belanger <martin.belanger at dell.com>
>>> ---
>>>    drivers/nvme/host/core.c | 35
>> +++++++++++++++++++++++++++++++++++
>>>    drivers/nvme/host/nvme.h |  3 +++
>>>    include/linux/nvme.h     | 10 +++++++++-
>>>    3 files changed, 47 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index
>>> 872c389dc6d3..c65d44fb65c4 100644
>>> --- a/drivers/nvme/host/core.c
>>> +++ b/drivers/nvme/host/core.c
>>> @@ -2992,6 +2992,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; @@
>>> -3525,6 +3528,34 @@ static ssize_t
>> nvme_ctrl_fast_io_fail_tmo_store(struct device *dev,
>>>    static DEVICE_ATTR(fast_io_fail_tmo, S_IRUGO | S_IWUSR,
>>>    	nvme_ctrl_fast_io_fail_tmo_show,
>> nvme_ctrl_fast_io_fail_tmo_store);
>>>
>>> +static ssize_t nvme_ctrl_cntrltype_show(struct device *dev,
>>> +				     struct device_attribute *attr, char *buf) {
>>> +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>> +
>>> +	switch (ctrl->cntrltype) {
>>> +	case NVME_CTRL_IO:	return sysfs_emit(buf, "io\n");
>>> +	case NVME_CTRL_DISC:	return sysfs_emit(buf, "disc\n");
>>
>> 'discovery', please. 'disc' is misleading here.
>>
>>> +	case NVME_CTRL_ADMIN:	return sysfs_emit(buf, "admin\n");
>>> +	default:		return sysfs_emit(buf, "reserved\n");
>>> +	}
>>> +}
>>> +static DEVICE_ATTR(cntrltype, S_IRUGO, nvme_ctrl_cntrltype_show,
>>> +NULL);
>>> +
>>> +static ssize_t nvme_ctrl_dctype_show(struct device *dev,
>>> +				     struct device_attribute *attr, char *buf) {
>>> +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>> +
>>> +	switch (ctrl->dctype) {
>>> +	case NVME_DCTYPE_NOT_REPORTED:	return sysfs_emit(buf,
>> "nr\n");
>>
>> Please use 'none' here; 'nr' is ambiguous ('Number'?)
> 
> Thanks for the review. I will apply the changes.
> 
> Question: Since this was part of a set of patches, do I need to resubmit
> the whole set or can I just resubmit this one patch?
> 
Whole series, please. And gather up the reviews when resubmitting.

> Also, Sagi mentioned that the dctype should only be exposed for
> Discovery Controllers, which makes sense. However, I found that
> when  nvme_dev_attrs_are_visible() is invoked the ctrl object is
> not fully configured yet (i.e. the controller is not yet identified).
> Is there any other way to determine the controller type prior to
> receiving the response to the Identify command?
> 
Nope, unfortunately.

But maybe we can lump both attributes together in to a single ctrltype;
then we would have 'io', 'admin', 'discovery', 'discovery-cdc', and 
'discovery-ddc'.

Hmm?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare at suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer