diff mbox series

[v2] hw/block/nvme: align with existing style

Message ID 20210415120048.5484-1-anaidu.gollu@samsung.com
State New
Headers show
Series [v2] hw/block/nvme: align with existing style | expand

Commit Message

Gollu Appalanaidu April 15, 2021, noon UTC
Make uniform hexadecimal numbers format.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
---
-v2: Address review comments (Klaus)
use lower case hexa format for the code and in comments 
use the same format as used in Spec. ("FFFFFFFFh")
    

 hw/block/nvme-ns.c   |  2 +-
 hw/block/nvme.c      | 40 ++++++++++++++++++++--------------------
 include/block/nvme.h | 10 +++++-----
 3 files changed, 26 insertions(+), 26 deletions(-)

Comments

Philippe Mathieu-Daudé April 15, 2021, 1:13 p.m. UTC | #1
On 4/15/21 2:00 PM, Gollu Appalanaidu wrote:
> Make uniform hexadecimal numbers format.
> 
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> ---
> -v2: Address review comments (Klaus)
> use lower case hexa format for the code and in comments 
> use the same format as used in Spec. ("FFFFFFFFh")

^ This comment is relevant to the commit message.

Also it would be nice if the subsystem could describe somewhere
what is its style. Not sure where... The file header is probably
the simplest place.

Something like:

"While QEMU coding style prefers lowercase hexadecimal in constants,
the NVMe subsystem use the format from the NVMe specifications in
the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix."

>  hw/block/nvme-ns.c   |  2 +-
>  hw/block/nvme.c      | 40 ++++++++++++++++++++--------------------
>  include/block/nvme.h | 10 +++++-----
>  3 files changed, 26 insertions(+), 26 deletions(-)
Klaus Jensen April 15, 2021, 5:18 p.m. UTC | #2
On Apr 15 15:13, Philippe Mathieu-Daudé wrote:
>On 4/15/21 2:00 PM, Gollu Appalanaidu wrote:
>> Make uniform hexadecimal numbers format.
>>
>> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>> ---
>> -v2: Address review comments (Klaus)
>> use lower case hexa format for the code and in comments
>> use the same format as used in Spec. ("FFFFFFFFh")
>
>^ This comment is relevant to the commit message.
>
>Also it would be nice if the subsystem could describe somewhere
>what is its style. Not sure where... The file header is probably
>the simplest place.
>
>Something like:
>
>"While QEMU coding style prefers lowercase hexadecimal in constants,
>the NVMe subsystem use the format from the NVMe specifications in
>the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix."
>

+1 for that suggestion.

>>  hw/block/nvme-ns.c   |  2 +-
>>  hw/block/nvme.c      | 40 ++++++++++++++++++++--------------------
>>  include/block/nvme.h | 10 +++++-----
>>  3 files changed, 26 insertions(+), 26 deletions(-)
>
>
Gollu Appalanaidu April 15, 2021, 6:30 p.m. UTC | #3
On Thu, Apr 15, 2021 at 07:18:50PM +0200, Klaus Jensen wrote:
>On Apr 15 15:13, Philippe Mathieu-Daudé wrote:
>>On 4/15/21 2:00 PM, Gollu Appalanaidu wrote:
>>>Make uniform hexadecimal numbers format.
>>>
>>>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>>>---
>>>-v2: Address review comments (Klaus)
>>>use lower case hexa format for the code and in comments
>>>use the same format as used in Spec. ("FFFFFFFFh")
>>
>>^ This comment is relevant to the commit message.
>>
>>Also it would be nice if the subsystem could describe somewhere
>>what is its style. Not sure where... The file header is probably
>>the simplest place.
>>
>>Something like:
>>
>>"While QEMU coding style prefers lowercase hexadecimal in constants,
>>the NVMe subsystem use the format from the NVMe specifications in
>>the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix."
>>
>
>+1 for that suggestion.
>

Sure, will add it and send v3.

>>> hw/block/nvme-ns.c   |  2 +-
>>> hw/block/nvme.c      | 40 ++++++++++++++++++++--------------------
>>> include/block/nvme.h | 10 +++++-----
>>> 3 files changed, 26 insertions(+), 26 deletions(-)
>>
>>
diff mbox series

Patch

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 7bb618f182..a0895614d9 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -303,7 +303,7 @@  static void nvme_ns_init_zoned(NvmeNamespace *ns)
 
     id_ns_z = g_malloc0(sizeof(NvmeIdNsZoned));
 
-    /* MAR/MOR are zeroes-based, 0xffffffff means no limit */
+    /* MAR/MOR are zeroes-based, FFFFFFFFFh means no limit */
     id_ns_z->mar = cpu_to_le32(ns->params.max_active_zones - 1);
     id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);
     id_ns_z->zoc = 0;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d0..f50e25c501 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -3607,18 +3607,18 @@  static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req)
 
     /*
      * In the base NVM command set, Flush may apply to all namespaces
-     * (indicated by NSID being set to 0xFFFFFFFF). But if that feature is used
+     * (indicated by NSID being set to FFFFFFFFh). But if that feature is used
      * along with TP 4056 (Namespace Types), it may be pretty screwed up.
      *
-     * If NSID is indeed set to 0xFFFFFFFF, we simply cannot associate the
+     * If NSID is indeed set to FFFFFFFFh, we simply cannot associate the
      * opcode with a specific command since we cannot determine a unique I/O
      * command set. Opcode 0x0 could have any other meaning than something
      * equivalent to flushing and say it DOES have completely different
-     * semantics in some other command set - does an NSID of 0xFFFFFFFF then
+     * semantics in some other command set - does an NSID of FFFFFFFFh then
      * mean "for all namespaces, apply whatever command set specific command
      * that uses the 0x0 opcode?" Or does it mean "for all namespaces, apply
      * whatever command that uses the 0x0 opcode if, and only if, it allows
-     * NSID to be 0xFFFFFFFF"?
+     * NSID to be FFFFFFFFh"?
      *
      * Anyway (and luckily), for now, we do not care about this since the
      * device only supports namespace types that includes the NVM Flush command
@@ -3934,7 +3934,7 @@  static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,
             NVME_CHANGED_NSID_SIZE) {
         /*
          * If more than 1024 namespaces, the first entry in the log page should
-         * be set to 0xffffffff and the others to 0 as spec.
+         * be set to FFFFFFFFh and the others to 0 as spec.
          */
         if (i == ARRAY_SIZE(nslist)) {
             memset(nslist, 0x0, sizeof(nslist));
@@ -4332,7 +4332,7 @@  static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeRequest *req,
     trace_pci_nvme_identify_nslist(min_nsid);
 
     /*
-     * Both 0xffffffff (NVME_NSID_BROADCAST) and 0xfffffffe are invalid values
+     * Both FFFFFFFFh (NVME_NSID_BROADCAST) and FFFFFFFFEh are invalid values
      * since the Active Namespace ID List should return namespaces with ids
      * *higher* than the NSID specified in the command. This is also specified
      * in the spec (NVM Express v1.3d, Section 5.15.4).
@@ -4379,7 +4379,7 @@  static uint16_t nvme_identify_nslist_csi(NvmeCtrl *n, NvmeRequest *req,
     trace_pci_nvme_identify_nslist_csi(min_nsid, c->csi);
 
     /*
-     * Same as in nvme_identify_nslist(), 0xffffffff/0xfffffffe are invalid.
+     * Same as in nvme_identify_nslist(), FFFFFFFFh/FFFFFFFFEh are invalid.
      */
     if (min_nsid >= NVME_NSID_BROADCAST - 1) {
         return NVME_INVALID_NSID | NVME_DNR;
@@ -4595,7 +4595,7 @@  static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
             /*
              * The Reservation Notification Mask and Reservation Persistence
              * features require a status code of Invalid Field in Command when
-             * NSID is 0xFFFFFFFF. Since the device does not support those
+             * NSID is FFFFFFFFh. Since the device does not support those
              * features we can always return Invalid Namespace or Format as we
              * should do for all other features.
              */
@@ -4854,8 +4854,8 @@  static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
             return NVME_INVALID_FIELD | NVME_DNR;
         }
 
-        trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
-                                    ((dw11 >> 16) & 0xFFFF) + 1,
+        trace_pci_nvme_setfeat_numq((dw11 & 0xffff) + 1,
+                                    ((dw11 >> 16) & 0xffff) + 1,
                                     n->params.max_ioqpairs,
                                     n->params.max_ioqpairs);
         req->cqe.result = cpu_to_le32((n->params.max_ioqpairs - 1) |
@@ -5493,7 +5493,7 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
             n->bar.cc = data;
         }
         break;
-    case 0x1C:  /* CSTS */
+    case 0x1c:  /* CSTS */
         if (data & (1 << 4)) {
             NVME_GUEST_ERR(pci_nvme_ub_mmiowr_ssreset_w1c_unsupported,
                            "attempted to W1C CSTS.NSSRO"
@@ -5505,7 +5505,7 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
         break;
     case 0x20:  /* NSSR */
-        if (data == 0x4E564D65) {
+        if (data == 0x4e564d65) {
             trace_pci_nvme_ub_mmiowr_ssreset_unsupported();
         } else {
             /* The spec says that writes of other values have no effect */
@@ -5575,11 +5575,11 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         n->bar.cmbmsc = (n->bar.cmbmsc & 0xffffffff) | (data << 32);
         return;
 
-    case 0xE00: /* PMRCAP */
+    case 0xe00: /* PMRCAP */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrcap_readonly,
                        "invalid write to PMRCAP register, ignored");
         return;
-    case 0xE04: /* PMRCTL */
+    case 0xe04: /* PMRCTL */
         n->bar.pmrctl = data;
         if (NVME_PMRCTL_EN(data)) {
             memory_region_set_enabled(&n->pmr.dev->mr, true);
@@ -5590,19 +5590,19 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
             n->pmr.cmse = false;
         }
         return;
-    case 0xE08: /* PMRSTS */
+    case 0xe08: /* PMRSTS */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrsts_readonly,
                        "invalid write to PMRSTS register, ignored");
         return;
-    case 0xE0C: /* PMREBS */
+    case 0xe0C: /* PMREBS */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrebs_readonly,
                        "invalid write to PMREBS register, ignored");
         return;
-    case 0xE10: /* PMRSWTP */
+    case 0xe10: /* PMRSWTP */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrswtp_readonly,
                        "invalid write to PMRSWTP register, ignored");
         return;
-    case 0xE14: /* PMRMSCL */
+    case 0xe14: /* PMRMSCL */
         if (!NVME_CAP_PMRS(n->bar.cap)) {
             return;
         }
@@ -5622,7 +5622,7 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
 
         return;
-    case 0xE18: /* PMRMSCU */
+    case 0xe18: /* PMRMSCU */
         if (!NVME_CAP_PMRS(n->bar.cap)) {
             return;
         }
@@ -5664,7 +5664,7 @@  static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
          * from PMRSTS should ensure prior writes
          * made it to persistent media
          */
-        if (addr == 0xE08 &&
+        if (addr == 0xe08 &&
             (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
             memory_region_msync(&n->pmr.dev->mr, 0, n->pmr.dev->size);
         }
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 4ac926fbc6..0739e0d665 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -848,8 +848,8 @@  enum NvmeStatusCodes {
     NVME_FW_REQ_SUSYSTEM_RESET  = 0x0110,
     NVME_NS_ALREADY_ATTACHED    = 0x0118,
     NVME_NS_PRIVATE             = 0x0119,
-    NVME_NS_NOT_ATTACHED        = 0x011A,
-    NVME_NS_CTRL_LIST_INVALID   = 0x011C,
+    NVME_NS_NOT_ATTACHED        = 0x011a,
+    NVME_NS_CTRL_LIST_INVALID   = 0x011c,
     NVME_CONFLICTING_ATTRS      = 0x0180,
     NVME_INVALID_PROT_INFO      = 0x0181,
     NVME_WRITE_TO_RO            = 0x0182,
@@ -1409,9 +1409,9 @@  typedef enum NvmeZoneState {
     NVME_ZONE_STATE_IMPLICITLY_OPEN  = 0x02,
     NVME_ZONE_STATE_EXPLICITLY_OPEN  = 0x03,
     NVME_ZONE_STATE_CLOSED           = 0x04,
-    NVME_ZONE_STATE_READ_ONLY        = 0x0D,
-    NVME_ZONE_STATE_FULL             = 0x0E,
-    NVME_ZONE_STATE_OFFLINE          = 0x0F,
+    NVME_ZONE_STATE_READ_ONLY        = 0x0d,
+    NVME_ZONE_STATE_FULL             = 0x0e,
+    NVME_ZONE_STATE_OFFLINE          = 0x0f,
 } NvmeZoneState;
 
 static inline void _nvme_check_size(void)