[PATCH 2/2] nvme: Expose cntrltype and dctype through sysfs.
Belanger, Martin
Martin.Belanger at dell.com
Thu Feb 3 08:04:26 PST 2022
> 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 ...
Hi Chaitanya,
Ok, I will keep the return on a separate line. BTW, is that specific to the
NVMe driver. I see that in many places in the kernel they put the
return on the same line as the case statement, as shown here:
$ git grep -E "case .*: *return" | wc -l
1676
>
> > + }
> > +}
>
> 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];
> > };
> >
Internal Use - Confidential
More information about the Linux-nvme
mailing list