Re: [PATCH] cxl/hdm: allow zero sized committed decoders

[Dave Jiang <dave.jiang@intel.com>]

On 10/1/25 3:03 PM, Vishal Aslot wrote:
> Thanks for the quick review, Dave. Yes, forgot to put the commit message. :)
> About the copyright message. Should I not be updating the year to 2025 in it?

I wouldn't bother unless you are doing massive changes and in that case it would be adding NVidia copyright anyhow for you. Also, typically linux mailing lists prefer responding inline and not top posting.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> ________________________________________
> From: Dave Jiang <dave.jiang@intel.com>
> Sent: Wednesday, October 1, 2025 4:28 PM
> To: Vishal Aslot; Davidlohr Bueso; Jonathan Cameron; Alison Schofield; Vishal Verma; Ira Weiny; Dan Williams; Li Ming; Peter Zijlstra; Dan Carpenter; Zijun Hu; linux-cxl@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] cxl/hdm: allow zero sized committed decoders
> 
> External email: Use caution opening links or attachments
> 
> 
> On 10/1/25 1:37 PM, Vishal Aslot wrote:
>> Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
> 
> Missing commit log?
> 
>> ---
>>  drivers/cxl/core/hdm.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index e9e1d555cec6..97a5f27f5b72 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -1,5 +1,5 @@
>>  // SPDX-License-Identifier: GPL-2.0-only
>> -/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
>> +/* Copyright(c) 2022-2025 Intel Corporation. All rights reserved. */
> 
> Stray change?
> 
>>  #include <linux/seq_file.h>
>>  #include <linux/device.h>
>>  #include <linux/delay.h>
>> @@ -1050,7 +1050,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>>                         dev_warn(&port->dev,
>>                                  "decoder%d.%d: Committed with zero size\n",
>>                                  port->id, cxld->id);
> 
> I wonder if we should make this dev_dbg() now that it's a valid case.
> 
>> -                       return -ENXIO;
>> +                       return -ENOSPC;
>>                 }
>>                 port->commit_end = cxld->id;
>>         } else {
>> @@ -1210,6 +1210,11 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>>                 rc = init_hdm_decoder(port, cxld, target_map, hdm, i,
>>                                       &dpa_base, info);
>>                 if (rc) {
>> +                       if (rc == -ENOSPC) {
>> +                               put_device(&cxld->dev);
> 
> Why put_device()? Should we enumerate this decoder instead of pretend it doesn't exist? essentially only a dev_set_name() and device_add()?
> 
>> +                               rc = 0;
> 
> Don't think this is needed since the continue will put it at start of the loop where rc gets written by init_hdm_decoder() return value.
> 
>> +                               continue;
>> +                       }
>>                         dev_warn(&port->dev,
>>                                  "Failed to initialize decoder%d.%d\n",
>>                                  port->id, i);
>> --
>> 2.34.1
>