[10/15] nvdimm acpi: abstract the operations for root device and nvdimm devices
diff mbox

Message ID 1458203581-59143-11-git-send-email-guangrong.xiao@linux.intel.com
State New
Headers show

Commit Message

Xiao Guangrong March 17, 2016, 8:32 a.m. UTC
It separates the operations between root device and nvdimm devices in
order to introducing label functions support for nvdimm device

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 72 ++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 18 deletions(-)

Comments

Stefan Hajnoczi March 17, 2016, 10:35 a.m. UTC | #1
On Thu, Mar 17, 2016 at 04:32:56PM +0800, Xiao Guangrong wrote:
> +static void
> +nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
> +{
> +    NvdimmDsmFunc0Out func0 = {
> +        .len = cpu_to_le32(sizeof(func0)),
> +        .supported_func = cpu_to_le32(supported_func),
> +    };
> +    cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof(func0));
> +}
> +
> +static void
> +nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
> +{
> +    NvdimmDsmFuncNoPayloadOut out = {
> +        .len = cpu_to_le32(sizeof(out)),
> +        .func_ret_status = cpu_to_le32(func_ret_status),
> +    };
> +    cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
> +}
> +
> +static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
> +{
> +    /*
> +     * function 0 is called to inquire which functions are supported by
> +     * OSPM
> +     */
> +    if (!in->function) {
> +        return nvdimm_dsm_function0(0 /* No function supported other
> +                                         than function 0 */, dsm_mem_addr);

The return type is void so "return foo()" looks strange.  I went back
and double-checked function prototypes because I was surprised by this
line of code.  Please use the conventional "foo(); return;" for void
return instead.
Xiao Guangrong March 23, 2016, 3:43 a.m. UTC | #2
On 03/17/2016 06:35 PM, Stefan Hajnoczi wrote:
> On Thu, Mar 17, 2016 at 04:32:56PM +0800, Xiao Guangrong wrote:
>> +static void
>> +nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
>> +{
>> +    NvdimmDsmFunc0Out func0 = {
>> +        .len = cpu_to_le32(sizeof(func0)),
>> +        .supported_func = cpu_to_le32(supported_func),
>> +    };
>> +    cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof(func0));
>> +}
>> +
>> +static void
>> +nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
>> +{
>> +    NvdimmDsmFuncNoPayloadOut out = {
>> +        .len = cpu_to_le32(sizeof(out)),
>> +        .func_ret_status = cpu_to_le32(func_ret_status),
>> +    };
>> +    cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
>> +}
>> +
>> +static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
>> +{
>> +    /*
>> +     * function 0 is called to inquire which functions are supported by
>> +     * OSPM
>> +     */
>> +    if (!in->function) {
>> +        return nvdimm_dsm_function0(0 /* No function supported other
>> +                                         than function 0 */, dsm_mem_addr);
>
> The return type is void so "return foo()" looks strange.  I went back
> and double-checked function prototypes because I was surprised by this
> line of code.  Please use the conventional "foo(); return;" for void
> return instead.
>

I am so lazy to omit this single line. :)

Okay, will follow this style in the next version.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 4177227..071f66f 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -404,6 +404,53 @@  struct NvdimmDsmFuncNoPayloadOut {
 } QEMU_PACKED;
 typedef struct NvdimmDsmFuncNoPayloadOut NvdimmDsmFuncNoPayloadOut;
 
+static void
+nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFunc0Out func0 = {
+        .len = cpu_to_le32(sizeof(func0)),
+        .supported_func = cpu_to_le32(supported_func),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof(func0));
+}
+
+static void
+nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFuncNoPayloadOut out = {
+        .len = cpu_to_le32(sizeof(out)),
+        .func_ret_status = cpu_to_le32(func_ret_status),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+}
+
+static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /*
+     * function 0 is called to inquire which functions are supported by
+     * OSPM
+     */
+    if (!in->function) {
+        return nvdimm_dsm_function0(0 /* No function supported other
+                                         than function 0 */, dsm_mem_addr);
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
+static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /* See the comments in nvdimm_dsm_root(). */
+    if (!in->function) {
+        return nvdimm_dsm_function0(0 /* No function supported other
+                                         than function 0 */, dsm_mem_addr);
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
 static uint64_t
 nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
 {
@@ -434,26 +481,15 @@  nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
                  in->handle, in->function);
 
-    /*
-     * function 0 is called to inquire which functions are supported by
-     * OSPM
-     */
-    if (in->function == 0) {
-        NvdimmDsmFunc0Out func0 = {
-            .len = cpu_to_le32(sizeof(func0)),
-             /* No function supported other than function 0 */
-            .supported_func = cpu_to_le32(0),
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof func0);
-    } else {
-        /* No function except function 0 is supported yet. */
-        NvdimmDsmFuncNoPayloadOut out = {
-            .len = cpu_to_le32(sizeof(out)),
-            .func_ret_status = cpu_to_le32(1)  /* Not Supported */,
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+     /* Handle 0 is reserved for NVDIMM Root Device. */
+    if (!in->handle) {
+        nvdimm_dsm_root(in, dsm_mem_addr);
+        goto exit;
     }
 
+    nvdimm_dsm_device(in, dsm_mem_addr);
+
+exit:
     g_free(in);
 }