Received-SPF: pass (google.com: domain of linux-kernel+bounces-133451-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 5 Apr 2024 11:09:20 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: =?iso-8859-1?Q?J=F8rgen?= Hansen <Jorgen.Hansen@wdc.com>,
	"ira.weiny@intel.com" <ira.weiny@intel.com>, Dave Jiang
	<dave.jiang@intel.com>, Fan Ni <fan.ni@samsung.com>, Jonathan Cameron
	<Jonathan.Cameron@huawei.com>, Navneet Singh <navneet.singh@intel.com>
CC: Dan Williams <dan.j.williams@intel.com>, Davidlohr Bueso
	<dave@stgolabs.net>, Alison Schofield <alison.schofield@intel.com>, "Vishal
 Verma" <vishal.l.verma@intel.com>, "linux-btrfs@vger.kernel.org"
	<linux-btrfs@vger.kernel.org>, "linux-cxl@vger.kernel.org"
	<linux-cxl@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 03/26] cxl/mem: Read dynamic capacity configuration from
 the device
Message-ID: <66103e5045e1c_e9f9f2941@iweiny-mobl.notmuch>
References: <20240324-dcd-type2-upstream-v1-0-b7b00d623625@intel.com>
 <20240324-dcd-type2-upstream-v1-3-b7b00d623625@intel.com>
 <79ebeecc-1650-4fcc-a5c5-db64ac16316d@wdc.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <79ebeecc-1650-4fcc-a5c5-db64ac16316d@wdc.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?rnhZ8J1Uhd2xJJgBKXyBkkD9siLu29bfmBUymUq1CWb4yGcgk7Y4QpE7OO?=
 =?iso-8859-1?Q?olxDT7SChfAO5uRq7qECXBbhjMrteNS8ImdKd88kdGnO2PEI2nlHgkwZne?=
 =?iso-8859-1?Q?tZ3RENMx6eQ8siflYIEnieF5+jp7E1F+5r2O4bp0/NIkOy/tEfVyAKo71/?=
 =?iso-8859-1?Q?ZSPLT1NjUE+w/hYv0JuAuWc7vt3SIKZ1O9duPBfSA7W4edj6oF78NohZ7u?=
 =?iso-8859-1?Q?dYKq1XDrEB71GlMH11su0y1guM+JeFAaxWg1LONOujCb/LMh+yZKuvEmTj?=
 =?iso-8859-1?Q?ws4im7E2OEqKP59hWv602EYU05Q4IT0UO+9dVq3GgDVCtv0N2810lZsmkX?=
 =?iso-8859-1?Q?Ha3btWXi57AGigdjXIQ5TxOtyHJ2Vhr0Tfxq2t06ergjWnb5eN6fO2YMuv?=
 =?iso-8859-1?Q?Ocg9aJHno/E0i70w23xCGVe2Kw/A8IvKdg61g/rLgXcu29bvZqTDcuDQpu?=
 =?iso-8859-1?Q?ScM4LrsCtKK7ym5cgX/T4vIN2frxMKVi8Tir2zj5yvq5CCJ8Xv79ioJRfQ?=
 =?iso-8859-1?Q?5HEZW3Fofz/4t+dHOFQiUcIgG3lc84Zd6rIaGJzl2vQZzSr8v5URWWdP8R?=
 =?iso-8859-1?Q?1D9PQi5TO9s1lCLhbCvJweXIu9ZtQU7UxzAMofZrgGtTI9mNK/JET/jZDv?=
 =?iso-8859-1?Q?0xN3vS1nELE0Au8gnvZMbuHFa9GV+DTiBNzI5c2eHXaC0A3ectcs3zluJW?=
 =?iso-8859-1?Q?dRZLZsH60EsDTZk4y940qeeJ7gZ59Wwjr/cPDfp40Jfph5t1TJeSRJNPiD?=
 =?iso-8859-1?Q?OhY7n5vCByJMQ9YQQTxKhQtI4E8IBGe5JERhdnANGW4/AGIpFBMLVETXVn?=
 =?iso-8859-1?Q?yR056S0aKEm6k1g7V6X6FXH/t2LYfUp/BE6OUiuImq7qEUcHz2JZ7XYoo0?=
 =?iso-8859-1?Q?48SZehQUm94xcPWL7ocuknoSVQ5xpqjUNRUIPLaghVA+BclHKojBFDdsZ+?=
 =?iso-8859-1?Q?6Mo7rkl5jjQjRu6TdaXZvsbV37G1aSHUpgeuerrXe4B/yW9INIRIGVbmQE?=
 =?iso-8859-1?Q?6U82yuSk3iSJC2dMMZ7AV900mWA771FN4Z+77jkF0cJajW+WZtdLs3ngYW?=
 =?iso-8859-1?Q?06TrhzqM5NJnfTzS1VVAzgIc9/+H/5tCy8eKyJThyfdv6II/1983m2qK6H?=
 =?iso-8859-1?Q?RA/ZodP0Irffyu+VzBjK/yXLwtrgdARgfJtghfdyl23ATsO8tLHvIhyLUa?=
 =?iso-8859-1?Q?//oFeiuaP3l0+3+RqouABI/5QEyevxGVlpvaK94sQJc2vAeid8bf/vQSQ6?=
 =?iso-8859-1?Q?DKDJS8NisJCKcSm17HSFCja92QXQ4AM8fAvEbLKG309GNV/X0fbsFzX3+a?=
 =?iso-8859-1?Q?BnjQDtMqoyfocoY2xoA3cbcu0BB04/fwH+/oPzoRzPQrl4n19swu5qVRX2?=
 =?iso-8859-1?Q?hpq1ZNfRQQN70AVNpCCHLQGqy8PMr4TBiM1BT4pgH3vTf6OXN2DX7XKPv3?=
 =?iso-8859-1?Q?GULExCZh6OEVZSubtcznhwi+iRGDde49ZQu9OnAhUn6RMoqhbjffAVvFgK?=
 =?iso-8859-1?Q?cHTj7urD4HteWlCy5IjkuXaou+ff4AHEQTKNTTcRkNam0cVNNRcBUotCwW?=
 =?iso-8859-1?Q?oRMXxmHSA6S6Tvsr1QVxEhIg2qi7ImCtT8JtWjn6PEPIAv3I46zqSyp76u?=
 =?iso-8859-1?Q?KsvP75IGSmGs92WaTKaHsCwpEuOx0pGNu9?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e496445a-bb31-474e-4f5e-08dc559b892b
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2024 18:09:29.1536
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /FwFnN6PpqDql5p37Ea+2tD1/tNB9rKbJ3TyMaequSeePZ5F452pxSHgjjmfTa48+5RvsJbhuJ6Ykd4+vVmqug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB8537
X-OriginatorOrg: intel.com

J?rgen Hansen wrote:
> On 3/25/24 00:18, ira.weiny@intel.com wrote:
> 
> > From: Navneet Singh <navneet.singh@intel.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;