[PATCH 1/4] nvme-fabrics: add discovery controller type

[Belanger, Martin]

> On 1/25/22 15:59, Martin Belanger wrote:
> > From: Martin Belanger <martin.belanger at dell.com>
> >
> > TP8010 introduces the Central Discovery Controller (CDC). To
> > differentiate between a CDC and a DDC (Direct DC), the identify
> > response now carries a DCTYPE (Discovery Controller Type) field.
> >
> > Signed-off-by: Martin Belanger <martin.belanger at dell.com>
> > ---
> >   drivers/nvme/host/core.c |  3 +++
> >   drivers/nvme/host/nvme.h |  2 ++
> >   include/linux/nvme.h     | 10 +++++++++-
> >   3 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index
> > 4fc794d9c2f4..cd34b92e8e9d 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -2978,6 +2978,7 @@ static int nvme_init_identify(struct nvme_ctrl
> *ctrl)
> >   	ctrl->kas = le16_to_cpu(id->kas);
> >   	ctrl->max_namespaces = le32_to_cpu(id->mnan);
> >   	ctrl->ctratt = le32_to_cpu(id->ctratt);
> > +	ctrl->dctype = id->dctype;
> >
> >   	if (id->rtd3e) {
> >   		/* us -> s */
> > @@ -3335,6 +3336,7 @@ nvme_show_int_function(numa_node);
> >   nvme_show_int_function(queue_count);
> >   nvme_show_int_function(sqsize);
> >   nvme_show_int_function(kato);
> > +nvme_show_int_function(dctype);
> >
> >   static ssize_t nvme_sysfs_delete(struct device *dev,
> >   				struct device_attribute *attr, const char *buf,
> @@ -3533,6
> > +3535,7 @@ static struct attribute *nvme_dev_attrs[] = {
> >   	&dev_attr_reconnect_delay.attr,
> >   	&dev_attr_fast_io_fail_tmo.attr,
> >   	&dev_attr_kato.attr,
> > +	&dev_attr_dctype.attr,
> >   	NULL
> >   };
> >
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index
> > fe224016418e..f8b60f7346fd 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -349,6 +349,8 @@ struct nvme_ctrl {
> >   	unsigned long discard_page_busy;
> >
> >   	struct nvme_fault_inject fault_inject;
> > +
> > +	enum nvme_dctype dctype;
> >   };
> >
> >   enum nvme_iopolicy {
> > diff --git a/include/linux/nvme.h b/include/linux/nvme.h index
> > 855dd9b3e84b..82567d493c51 100644
> > --- a/include/linux/nvme.h
> > +++ b/include/linux/nvme.h
> > @@ -240,6 +240,12 @@ enum nvme_ctrl_attr {
> >   	NVME_CTRL_ATTR_TBKAS		= (1 << 6),
> >   };
> >
> > +enum nvme_dctype {
> > +	NVME_DCTYPE_NOT_REPORTED = 0,
> > +	NVME_DCTYPE_DDC = 1,
> > +	NVME_DCTYPE_CDC = 2,
> > +};
> > +
> >   struct nvme_id_ctrl {
> >   	__le16			vid;
> >   	__le16			ssvid;
> > @@ -320,7 +326,9 @@ struct nvme_id_ctrl {
> >   	__le16			icdoff;
> >   	__u8			ctrattr;
> >   	__u8			msdbd;
> > -	__u8			rsvd1804[244];
> > +	__le16			ofcs;
> > +	__u8			dctype;
> > +	__u8			rsvd1807[241];
> >   	struct nvme_id_power_state	psd[32];
> >   	__u8			vs[1024];
> >   };
> 
> Please decode the fields for the sysfs attribute; 'cdc' is far easier to
> understand than '2'.

Ok, I will add a decoder, but just to explain why I did it this way....

I agree that 'cdc' is more human-readable than 2. On the other hand,
decoding to a string has the drawback that when (if) the enumeration 
changes, then we need to add more decode code in the kernel.

As you know it takes a while for kernel changes to make it into a distro.
By simply printing the decimal value we never have to make changes 
to the kernel again and any value added to the enum in the future will
just show up.

> 
> 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

Internal Use - Confidential