* [PATCH 0/2] nvme: some cleanup
@ 2019-04-29 17:35 Edmund Nadolski
2019-04-29 17:35 ` [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros Edmund Nadolski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Edmund Nadolski @ 2019-04-29 17:35 UTC (permalink / raw)
This series clarifies the code by adding some macros, comments,
and fixing a few miscellaneous typos.
Edmund Nadolski (2):
nvme: nvme_set_queue_count should use descriptive macros
nvme: add clarifying comments and fix some typos
drivers/nvme/host/core.c | 8 ++++++--
drivers/nvme/host/nvme.h | 32 ++++++++++++++++----------------
drivers/nvme/host/pci.c | 2 +-
3 files changed, 23 insertions(+), 19 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros
2019-04-29 17:35 [PATCH 0/2] nvme: some cleanup Edmund Nadolski
@ 2019-04-29 17:35 ` Edmund Nadolski
2019-04-29 23:21 ` Sagi Grimberg
2019-04-30 15:23 ` Christoph Hellwig
2019-04-29 17:35 ` [PATCH 2/2] nvme: add clarifying comments and fix some typos Edmund Nadolski
[not found] ` <CGME20190429173604epcas3p2e65d79c87ba36e384e6c42a1b1744eae@epcms2p4>
2 siblings, 2 replies; 9+ messages in thread
From: Edmund Nadolski @ 2019-04-29 17:35 UTC (permalink / raw)
Implement macros to set/get the number of submission and/or completion
queues requested by the Set Features command. This replaces the bit
masking/shifting code which is harder to read and more error prone to
maintain.
Signed-off-by: Edmund Nadolski <ednadols at linux.microsoft.com>
---
drivers/nvme/host/core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 3dd043aa6d1f..b3804dbdcc30 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1132,9 +1132,13 @@ static int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword
return ret;
}
+#define SET_NUMQ(nsqr, ncqr) (((nsqr) - 1) | (((ncqr) - 1) << 16))
+#define GET_NSQA(dw) (((dw) & 0xffff) + 1)
+#define GET_NCQA(dw) (((dw) >> 16) + 1)
+
int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
{
- u32 q_count = (*count - 1) | ((*count - 1) << 16);
+ u32 q_count = SET_NUMQ(*count, *count);
u32 result;
int status, nr_io_queues;
@@ -1152,7 +1156,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
dev_err(ctrl->device, "Could not set queue count (%d)\n", status);
*count = 0;
} else {
- nr_io_queues = min(result & 0xffff, result >> 16) + 1;
+ nr_io_queues = min(GET_NSQA(result), GET_NCQA(result));
*count = min(*count, nr_io_queues);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] nvme: add clarifying comments and fix some typos
2019-04-29 17:35 [PATCH 0/2] nvme: some cleanup Edmund Nadolski
2019-04-29 17:35 ` [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros Edmund Nadolski
@ 2019-04-29 17:35 ` Edmund Nadolski
2019-04-29 23:23 ` Sagi Grimberg
2019-04-30 3:22 ` Chaitanya Kulkarni
[not found] ` <CGME20190429173604epcas3p2e65d79c87ba36e384e6c42a1b1744eae@epcms2p4>
2 siblings, 2 replies; 9+ messages in thread
From: Edmund Nadolski @ 2019-04-29 17:35 UTC (permalink / raw)
Add comment descriptions for struct fields per NVMe spec.
Signed-off-by: Edmund Nadolski <ednadols at linux.microsoft.com>
---
drivers/nvme/host/nvme.h | 32 ++++++++++++++++----------------
drivers/nvme/host/pci.c | 2 +-
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 527d64545023..61736d1ef15e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -177,7 +177,7 @@ struct nvme_ctrl {
u16 cntlid;
u32 ctrl_config;
- u16 mtfa;
+ u16 mtfa; /* maximum time for firmware activation */
u32 queue_count;
u64 cap;
@@ -185,23 +185,23 @@ struct nvme_ctrl {
u32 max_hw_sectors;
u32 max_segments;
u16 crdt[3];
- u16 oncs;
- u16 oacs;
- u16 nssa;
+ u16 oncs; /* optional nvm command support */
+ u16 oacs; /* optional admin command support */
+ u16 nssa; /* nvme subsystem streams available */
u16 nr_streams;
u32 max_namespaces;
atomic_t abort_limit;
- u8 vwc;
- u32 vs;
- u32 sgls;
- u16 kas;
- u8 npss;
- u8 apsta;
- u32 oaes;
+ u8 vwc; /* volatile write cache */
+ u32 vs; /* version */
+ u32 sgls; /* scatter gather list support */
+ u16 kas; /* keep alive support */
+ u8 npss; /* number of power states supported */
+ u8 apsta; /* autonomous power state transition */
+ u32 oaes; /* optional asynchronous events supported */
u32 aen_result;
- u32 ctratt;
+ u32 ctratt; /* controller attribute */
unsigned int shutdown_timeout;
- unsigned int kato;
+ unsigned int kato; /* keep alive timeout */
bool subsystem;
unsigned long quirks;
struct nvme_id_power_state psd[32];
@@ -231,8 +231,8 @@ struct nvme_ctrl {
bool apst_enabled;
/* PCIe only: */
- u32 hmpre;
- u32 hmmin;
+ u32 hmpre; /* host memory buffer preferred size */
+ u32 hmmin; /* host memory buffer minimum size */
u32 hmminds;
u16 hmmaxd;
@@ -290,7 +290,7 @@ struct nvme_ns_ids {
/*
* Anchor structure for namespaces. There is one for each namespace in a
* NVMe subsystem that any of our controllers can see, and the namespace
- * structure for each controller is chained of it. For private namespaces
+ * structure for each controller is chained off it. For private namespaces
* there is a 1:1 relation to our namespace structures, that is ->list
* only ever has a single entry for private namespaces.
*/
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c1eecde6b853..368371c5cafc 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -227,7 +227,7 @@ struct nvme_iod {
};
/*
- * Check we didin't inadvertently grow the command struct
+ * Check we didn't inadvertently grow the command struct
*/
static inline void _nvme_check_size(void)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros
2019-04-29 17:35 ` [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros Edmund Nadolski
@ 2019-04-29 23:21 ` Sagi Grimberg
2019-04-30 15:23 ` Christoph Hellwig
1 sibling, 0 replies; 9+ messages in thread
From: Sagi Grimberg @ 2019-04-29 23:21 UTC (permalink / raw)
Looks fine,
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] nvme: add clarifying comments and fix some typos
2019-04-29 17:35 ` [PATCH 2/2] nvme: add clarifying comments and fix some typos Edmund Nadolski
@ 2019-04-29 23:23 ` Sagi Grimberg
2019-04-30 4:46 ` Edmund Nadolski
2019-04-30 3:22 ` Chaitanya Kulkarni
1 sibling, 1 reply; 9+ messages in thread
From: Sagi Grimberg @ 2019-04-29 23:23 UTC (permalink / raw)
> Add comment descriptions for struct fields per NVMe spec.
Lets split typos from this patch. Not sure that the comments are all
that useful, as we usually have a spec/tp at hand...
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] nvme: add clarifying comments and fix some typos
[not found] ` <CGME20190429173604epcas3p2e65d79c87ba36e384e6c42a1b1744eae@epcms2p4>
@ 2019-04-29 23:32 ` Minwoo Im
0 siblings, 0 replies; 9+ messages in thread
From: Minwoo Im @ 2019-04-29 23:32 UTC (permalink / raw)
> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On
> Behalf Of Edmund Nadolski
> Sent: Tuesday, April 30, 2019 2:36 AM
> To: linux-nvme at lists.infradead.org; ednadols at linux.microsoft.com
> Subject: [PATCH 2/2] nvme: add clarifying comments and fix some typos
>
> Add comment descriptions for struct fields per NVMe spec.
>
> Signed-off-by: Edmund Nadolski <ednadols at linux.microsoft.com>
IMHO, if you really want to provide description for structure attributes,
why don't we have all of them instead describing few fields only.
Also It would be better if typo-related commit is separted from it.
Thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] nvme: add clarifying comments and fix some typos
2019-04-29 17:35 ` [PATCH 2/2] nvme: add clarifying comments and fix some typos Edmund Nadolski
2019-04-29 23:23 ` Sagi Grimberg
@ 2019-04-30 3:22 ` Chaitanya Kulkarni
1 sibling, 0 replies; 9+ messages in thread
From: Chaitanya Kulkarni @ 2019-04-30 3:22 UTC (permalink / raw)
These fields are taken from the spec, so I don't see the need for
adding comments as most of the fields are combination of the bits and
for that you have to refer to the spec anyways.
On 4/29/19 10:36 AM, Edmund Nadolski wrote:
> Add comment descriptions for struct fields per NVMe spec.
>
> Signed-off-by: Edmund Nadolski <ednadols at linux.microsoft.com>
> ---
> drivers/nvme/host/nvme.h | 32 ++++++++++++++++----------------
> drivers/nvme/host/pci.c | 2 +-
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 527d64545023..61736d1ef15e 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -177,7 +177,7 @@ struct nvme_ctrl {
> u16 cntlid;
>
> u32 ctrl_config;
> - u16 mtfa;
> + u16 mtfa; /* maximum time for firmware activation */
> u32 queue_count;
>
> u64 cap;
> @@ -185,23 +185,23 @@ struct nvme_ctrl {
> u32 max_hw_sectors;
> u32 max_segments;
> u16 crdt[3];
> - u16 oncs;
> - u16 oacs;
> - u16 nssa;
> + u16 oncs; /* optional nvm command support */
> + u16 oacs; /* optional admin command support */
> + u16 nssa; /* nvme subsystem streams available */
> u16 nr_streams;
> u32 max_namespaces;
> atomic_t abort_limit;
> - u8 vwc;
> - u32 vs;
> - u32 sgls;
> - u16 kas;
> - u8 npss;
> - u8 apsta;
> - u32 oaes;
> + u8 vwc; /* volatile write cache */
> + u32 vs; /* version */
> + u32 sgls; /* scatter gather list support */
> + u16 kas; /* keep alive support */
> + u8 npss; /* number of power states supported */
> + u8 apsta; /* autonomous power state transition */
> + u32 oaes; /* optional asynchronous events supported */
> u32 aen_result;
> - u32 ctratt;
> + u32 ctratt; /* controller attribute */
> unsigned int shutdown_timeout;
> - unsigned int kato;
> + unsigned int kato; /* keep alive timeout */
> bool subsystem;
> unsigned long quirks;
> struct nvme_id_power_state psd[32];
> @@ -231,8 +231,8 @@ struct nvme_ctrl {
> bool apst_enabled;
>
> /* PCIe only: */
> - u32 hmpre;
> - u32 hmmin;
> + u32 hmpre; /* host memory buffer preferred size */
> + u32 hmmin; /* host memory buffer minimum size */
> u32 hmminds;
> u16 hmmaxd;
>
> @@ -290,7 +290,7 @@ struct nvme_ns_ids {
> /*
> * Anchor structure for namespaces. There is one for each namespace in a
> * NVMe subsystem that any of our controllers can see, and the namespace
> - * structure for each controller is chained of it. For private namespaces
> + * structure for each controller is chained off it. For private namespaces
> * there is a 1:1 relation to our namespace structures, that is ->list
> * only ever has a single entry for private namespaces.
> */
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index c1eecde6b853..368371c5cafc 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -227,7 +227,7 @@ struct nvme_iod {
> };
>
> /*
> - * Check we didin't inadvertently grow the command struct
> + * Check we didn't inadvertently grow the command struct
> */
> static inline void _nvme_check_size(void)
> {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] nvme: add clarifying comments and fix some typos
2019-04-29 23:23 ` Sagi Grimberg
@ 2019-04-30 4:46 ` Edmund Nadolski
0 siblings, 0 replies; 9+ messages in thread
From: Edmund Nadolski @ 2019-04-30 4:46 UTC (permalink / raw)
On 4/29/19 4:23 PM, Sagi Grimberg wrote:
>
>> Add comment descriptions for struct fields per NVMe spec.
>
> Lets split typos from this patch. Not sure that the comments are all
> that useful, as we usually have a spec/tp at hand...
They could be useful to save a lookup (or when a spec isn't handy). I had
also thought of doing the nvm_id_ctrl and nvme_id_ns structs, but I'll hold
off as it seems folks see this as extraneous.
I will split out the typos out and re-send.
Thanks,
Ed
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros
2019-04-29 17:35 ` [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros Edmund Nadolski
2019-04-29 23:21 ` Sagi Grimberg
@ 2019-04-30 15:23 ` Christoph Hellwig
1 sibling, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2019-04-30 15:23 UTC (permalink / raw)
On Mon, Apr 29, 2019@10:35:32AM -0700, Edmund Nadolski wrote:
> Implement macros to set/get the number of submission and/or completion
> queues requested by the Set Features command. This replaces the bit
> masking/shifting code which is harder to read and more error prone to
> maintain.
>
> Signed-off-by: Edmund Nadolski <ednadols at linux.microsoft.com>
I have to say I actually find this much harder to read than the old
version.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-30 15:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 17:35 [PATCH 0/2] nvme: some cleanup Edmund Nadolski
2019-04-29 17:35 ` [PATCH 1/2] nvme: nvme_set_queue_count should use descriptive macros Edmund Nadolski
2019-04-29 23:21 ` Sagi Grimberg
2019-04-30 15:23 ` Christoph Hellwig
2019-04-29 17:35 ` [PATCH 2/2] nvme: add clarifying comments and fix some typos Edmund Nadolski
2019-04-29 23:23 ` Sagi Grimberg
2019-04-30 4:46 ` Edmund Nadolski
2019-04-30 3:22 ` Chaitanya Kulkarni
[not found] ` <CGME20190429173604epcas3p2e65d79c87ba36e384e6c42a1b1744eae@epcms2p4>
2019-04-29 23:32 ` Minwoo Im
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.