Received-SPF: pass (google.com: domain of linux-kernel+bounces-133537-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 12:50:06 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: fan <nifan.cxl@gmail.com>, <ira.weiny@intel.com>
CC: Dave Jiang <dave.jiang@intel.com>, Fan Ni <fan.ni@samsung.com>, "Jonathan
 Cameron" <Jonathan.Cameron@huawei.com>, Navneet Singh
	<navneet.singh@intel.com>, 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-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 06/26] cxl/port: Add Dynamic Capacity mode support to
 endpoint decoders
Message-ID: <661055ee8c5aa_e9f9f29481@iweiny-mobl.notmuch>
References: <20240324-dcd-type2-upstream-v1-0-b7b00d623625@intel.com>
 <20240324-dcd-type2-upstream-v1-6-b7b00d623625@intel.com>
 <ZgL5SdGCori3Dh3O@debian>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZgL5SdGCori3Dh3O@debian>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?VDJYY+aPmjg6S32OC4ACcMiZqs6n3va3HfgEV0gRIeSxoVivS+65cxahwOAj?=
 =?us-ascii?Q?c8v+/hjFIH7jwC7e10vkPi3LL4TIH+8GPztSy4bruTPS6DCNs3upvxrITphZ?=
 =?us-ascii?Q?Fe6UNbvRlWRYgvzvIqQ50Jq7wlDlHpw9RSm2Rd5YHyh6vgfYjPvx0Wvsu6XF?=
 =?us-ascii?Q?taEXhKqnTwKnjtp8wuQWh93dgVPACfZpIDhVeK2h1KNjv8UOnPuTAK2BqWEz?=
 =?us-ascii?Q?qe8ZXHqAlidBYczaabj7wdPth8HZp9cULMvseka+gMN6Bv+3NvYhrXFJtGzM?=
 =?us-ascii?Q?Stppq73UBmUTBRZfsE6NPhS1TQaPKPdTATxGGNyTCLt+ow6eToYiUtqoKq1+?=
 =?us-ascii?Q?vB0ka3WSRlosveKaT98Fapuank7Bo5Yv5p1bqKSbgy7pqR6NPTvY7Xmc7j4T?=
 =?us-ascii?Q?UTrO98i/gBkdsiI25IXwYjmAHFwJYLc9yXYpR5ueDKQB4ZDqsy6lauBjgNyM?=
 =?us-ascii?Q?1xqD/P0ykrzEjTx1d/nJCtuoHArG2GW5R+6wp763S444KbfXguNkJTn5Z4Ot?=
 =?us-ascii?Q?V+jBwGZQcowOzMfgzOKHRMsBi6ST+unEFOtGD/P7QcTjEL1ZTna+WycLWflN?=
 =?us-ascii?Q?T4GcG13t2H/wAUxU2ws32DMuEhQBmXcTYSnNk8YjVhxB1sOcvVccVHkaNQxc?=
 =?us-ascii?Q?EgTxW3bM8csJh2sIQt6SpfQ8zBvq3ZSQpMJppssp9snkge3CeGCkREr1gygB?=
 =?us-ascii?Q?lhQr/oUuyj1wJ4NxUNRrpActf0jvayj6+hZ10OW5N9r5LAU/T2uSyib02B2R?=
 =?us-ascii?Q?6nn900rfqf9FLizDGnAMl8S5yzQigSJkm8pR33gSMNw61OOYzBXUN8HxQjKa?=
 =?us-ascii?Q?TIZ3pYOL5qjQM2ywFLWz3dSeSV6f2eTaq5u5L75RrkET8+stAZA8F+GKhcCX?=
 =?us-ascii?Q?760WgmxXFTHcFCVAWv5ACNjXXZ0kdgUFJL49VxhAs5kCeZRmSZdvJ3XgMyJ5?=
 =?us-ascii?Q?hCGyV8Nz82PK5ltczjXA6LMtpgYdvFISsENa+6YRqfMB0jOZb31ChO/C4P5Q?=
 =?us-ascii?Q?w92Wq1N7JHFPvWKDFkesnYYB34W3/d1w1Qh2fqjAiwb/L0XNs1UNwDWFXQqX?=
 =?us-ascii?Q?fjqO89epzoGC6h4Xa6giu4/k+X5rRwJ3Rh6FeD65/5B+3XJinecltbGyXIb1?=
 =?us-ascii?Q?hcShqR4rqn7FzuuAUJdbevgMilfDcgOXdTdQlpP7o6+H4bdZf3YA9f+u94Kc?=
 =?us-ascii?Q?6PLudGN+PfaLxwS6vCJrUAldhn+wX5nwe9peqxLkVVX8pIOGnAylmmqOUK94?=
 =?us-ascii?Q?KkYCjQBL29b5Ygx5wB+HvFZnbCl8/REYkI0gnjpCnDVk8a4KlrElWlRUYgtJ?=
 =?us-ascii?Q?YXyeOeLpOHWscMVqWVSEplDDKPlkyhw5GsBQzDIeQDzLsewoVF3NioxmeTd7?=
 =?us-ascii?Q?6+vPnqtG+2xpYY7w5eD4UubKXqRrMpX9veYKWssqziv+zkE3HRGlS3c5BOEb?=
 =?us-ascii?Q?7XQpkEzCUkjOfsofPp1IsslGJnLgQEks3E/I0Q9klfJpMHjq/ZrXRsI1qKPM?=
 =?us-ascii?Q?zfOAmqyJftzPSkEbBhEyGiMi2mDToq4oRvxqehGe9lOch4BGpRjTtVd3lUdS?=
 =?us-ascii?Q?jDVFtoQdcxKx+FfqsMzrPK+swZiDJiUYLbgCYi/D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bb966056-1364-474c-55dc-08dc55a999ea
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2024 19:50:10.1375
 (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: hEqVOOSSHGVP2YDHhN1wdMyPx0KsuBK7rq8dvfDU330pRrGZ/fDXvJ2iXMTd4i1efJMpGoRFpFVfI8xhI9lfyg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8052
X-OriginatorOrg: intel.com

fan wrote:
> On Sun, Mar 24, 2024 at 04:18:09PM -0700, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 

[snip]

> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 66b8419fd0c3..e22b6f4f7145 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -255,6 +255,14 @@ static void devm_cxl_dpa_release(struct cxl_endpoint_decoder *cxled)
> >  	__cxl_dpa_release(cxled);
> >  }
> >  
> > +static int dc_mode_to_region_index(enum cxl_decoder_mode mode)
> > +{
> > +	if (mode < CXL_DECODER_DC0 || CXL_DECODER_DC7 < mode)
> 
> To me, it is more natural to have something like x<a || x>b other than
> x<a || b < x for out of range check.

Ok.


[snip]

> >  
> >  	guard(rwsem_write)(&cxl_dpa_rwsem);
> >  	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE)
> > @@ -433,6 +442,16 @@ int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
> >  			return -ENXIO;
> >  		}
> >  		break;
> > +	case CXL_DECODER_DC0 ... CXL_DECODER_DC7:
> > +		rc = dc_mode_to_region_index(mode);
> > +		if (rc < 0)
> > +			return rc;
> > +
> > +		if (resource_size(&cxlds->dc_res[rc]) == 0) {
> 
> The other similar checks above use "!resource_size(...)".
> 

Yea good catch.
Ira