[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66103e5045e1c_e9f9f2941@iweiny-mobl.notmuch>
Date: Fri, 5 Apr 2024 11:09:20 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Jørgen Hansen <Jorgen.Hansen@....com>,
"ira.weiny@...el.com" <ira.weiny@...el.com>, Dave Jiang
<dave.jiang@...el.com>, Fan Ni <fan.ni@...sung.com>, Jonathan Cameron
<Jonathan.Cameron@...wei.com>, Navneet Singh <navneet.singh@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>, Davidlohr Bueso
<dave@...olabs.net>, Alison Schofield <alison.schofield@...el.com>, "Vishal
Verma" <vishal.l.verma@...el.com>, "linux-btrfs@...r.kernel.org"
<linux-btrfs@...r.kernel.org>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 03/26] cxl/mem: Read dynamic capacity configuration from
the device
Jørgen Hansen wrote:
> On 3/25/24 00:18, ira.weiny@...el.com wrote:
>
> > From: Navneet Singh <navneet.singh@...el.com>
> >
[snip]
> > /**
> > * struct cxl_memdev_state - Generic Type-3 Memory Device Class driver data
> > *
> > @@ -467,6 +482,8 @@ struct cxl_dev_state {
> > * @enabled_cmds: Hardware commands found enabled in CEL.
> > * @exclusive_cmds: Commands that are kernel-internal only
> > * @total_bytes: sum of all possible capacities
> > + * @static_cap: Sum of static RAM and PMEM capacities
> > + * @dynamic_cap: Complete DPA range occupied by DC regions
>
> How about naming these total_range, static_cap and dynamic_range to make
> it clear that the DPA range occupied by DC regions isn't necessarily
> usable capacity (as opposed to the static_cap where the spec defines it
> as usable capacity).
I thought this was a good idea but on second thought these are not range
variables at all. They really represent the various lengths of the
resources.
For total_bytes the documentation already says 'sum of all __possible__
capacities'.
I think you have a point for the new fields though. They should all be
named in some consistent manner and documented as such.
So I propose:
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 94531af018f8..9c18b229f69a 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -481,9 +481,9 @@ struct cxl_dc_region_info {
* @dcd_cmds: List of DCD commands implemented by memory device
* @enabled_cmds: Hardware commands found enabled in CEL.
* @exclusive_cmds: Commands that are kernel-internal only
- * @total_bytes: sum of all possible capacities
- * @static_cap: Sum of static RAM and PMEM capacities
- * @dynamic_cap: Complete DPA range occupied by DC regions
+ * @total_bytes: length of all possible capacities
+ * @static_bytes: length of possible static RAM and PMEM partitions
+ * @dynamic_bytes: length of possible DC partitions (DC Regions)
* @volatile_only_bytes: hard volatile capacity
* @persistent_only_bytes: hard persistent capacity
* @partition_align_bytes: alignment size for partition-able capacity
@@ -515,8 +515,8 @@ struct cxl_memdev_state {
DECLARE_BITMAP(exclusive_cmds, CXL_MEM_COMMAND_ID_MAX);
u64 total_bytes;
- u64 static_cap;
- u64 dynamic_cap;
+ u64 static_bytes;
+ u64 dynamic_bytes;
u64 volatile_only_bytes;
u64 persistent_only_bytes;
u64 partition_align_bytes;
Powered by blists - more mailing lists