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

[Chaitanya Kulkarni]

On 2/2/22 13: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.
> 

above can be re aligned to 72 char per line :-

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");
> +	case NVME_CTRL_ADMIN:	return sysfs_emit(buf, "admin\n");
> +	default:		return sysfs_emit(buf, "reserved\n");

please keep the return on new line ...

> +	}
> +}

with newline added to the switch for each new case we will be adding new 
case statement and a sysfs_emit() call, following style avoids current 
and future repetations of case ... sysfs_emit() calls and brings down to 
only two calls including any future ctrl type value (2 lines for each 
new value pf ctrltype vs 1 line in the array of string :-

static ssize_t nvme_ctrl_cntrltype_show(struct device *dev,
                                      struct device_attribute *attr, 
char *buf)
{
         const char *type[] = {
             [NVME_CTRL_IO], "io\n",
             [NVME_CTRL_DISC], "disc\n",
             [NVME_CTRL_ADMIN], "admin\n",
         };

         if (ctrl->cntrltype > NVME_CTRL_ADMIN || !type[ctrl->ctrltype])
             return sysfs_emit(buf, "reserved\n");

         return sysfs_emit(buf, type[ctrl->ctrltype]);
}




> +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");
> +	case NVME_DCTYPE_DDC:		return sysfs_emit(buf, "ddc\n");
> +	case NVME_DCTYPE_CDC:		return sysfs_emit(buf, "cdc\n");
> +	default:			return sysfs_emit(buf, "reserved\n");
> +	}
> +}

same comment as above to use array of string unless there is any reason
we should repeat the sysfs_emit() calls.

> +static DEVICE_ATTR(dctype, S_IRUGO, nvme_ctrl_dctype_show, NULL);
> +
>   static struct attribute *nvme_dev_attrs[] = {
>   	&dev_attr_reset_controller.attr,
>   	&dev_attr_rescan_controller.attr,
> @@ -3546,6 +3577,8 @@ static struct attribute *nvme_dev_attrs[] = {
>   	&dev_attr_reconnect_delay.attr,
>   	&dev_attr_fast_io_fail_tmo.attr,
>   	&dev_attr_kato.attr,
> +	&dev_attr_cntrltype.attr,
> +	&dev_attr_dctype.attr,
>   	NULL
>   };
>   
> @@ -3569,6 +3602,8 @@ static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj,
>   		return 0;
>   	if (a == &dev_attr_fast_io_fail_tmo.attr && !ctrl->opts)
>   		return 0;
> +	if (a == &dev_attr_dctype.attr && ctrl->cntrltype == NVME_CTRL_DISC)
> +		return 0;
>   
>   	return a->mode;
>   }
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index a162f6c6da6e..e30626c00791 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -349,6 +349,9 @@ struct nvme_ctrl {
>   	unsigned long discard_page_busy;
>   
>   	struct nvme_fault_inject fault_inject;
> +
> +	enum nvme_ctrl_type cntrltype;
> +	enum nvme_dctype dctype;
>   };
>   
>   enum nvme_iopolicy {
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index 855dd9b3e84b..af1494d83a52 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -43,6 +43,12 @@ enum name_ctrl_type {
>   	NVME_CTRL_ADMIN	= 3,		/* Administrative controller */
>   };
>   
> +enum nvme_dctype {
> +	NVME_DCTYPE_NOT_REPORTED = 0,
> +	NVME_DCTYPE_DDC = 1,		/* Direct Discovery Controller */
> +	NVME_DCTYPE_CDC = 2,		/* Central Discovery Controller */
> +};
> +

consider follwing with aligned initial values that also matches what
we have in the include/linux/nvme.h for enums initialization :-
enum nvme_dctype {
         NVME_DCTYPE_NOT_REPORTED        = 0,
         NVME_DCTYPE_DDC                 = 1, /* Direct Discovery 
Controller */
         NVME_DCTYPE_CDC                 = 2, /* Central Discovery 
Controller */
};

>   /* Address Family codes for Discovery Log Page entry ADRFAM field */
>   enum {
>   	NVMF_ADDR_FAMILY_PCI	= 0,	/* PCIe */
> @@ -320,7 +326,9 @@ struct nvme_id_ctrl {
>   	__le16			icdoff;
>   	__u8			ctrattr;
>   	__u8			msdbd;
> -	__u8			rsvd1804[244];
> +	__u8			rsvd1804[2];
> +	__u8			dctype;
> +	__u8			rsvd1807[241];
>   	struct nvme_id_power_state	psd[32];
>   	__u8			vs[1024];
>   };
>