Re: [RFC PATCH 08/18] cxl/port: Update switch_port_probe() for CXL cache devices

["Cheatham, Benjamin" <benjamin.cheatham@amd.com>]

On 8/19/2025 7:03 AM, Jonathan Cameron wrote:
> On Tue, 12 Aug 2025 16:29:11 -0500
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> 
>> CXL switch port probe currently assumes there are only CXL.mem-capable
>> devices below the switch with preconfigured HDM decoders. Now that
>> CXL.cache devices can be added below a switch port, it's possible that
>> they either have no HDM decoders (type 1) or the CXL.mem capabilities of
>> the endpoint haven't been set up yet (type 2).
>>
>> The HDM decoders being disabled (if present) is no longer a gauranteed
>> failure condition when only cache devices are present below the port.
>> Knowing what kind of devices are under the switch port isn't possible
>> until the endpoints are probed, so delay HDM setup and validation until
>> endpoint port probe.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---
>>  drivers/cxl/core/hdm.c | 31 +++++++++++++++++++++++++++++++
>>  drivers/cxl/cxl.h      |  1 +
>>  drivers/cxl/port.c     | 23 +++++++++++++----------
>>  3 files changed, 45 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index 865a71bce251..6e04f1f4c166 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -205,6 +205,37 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>>  }
>>  EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_hdm, "CXL");
>>  
>> +int cxl_validate_endpoint_hdm_setup(struct cxl_port *endpoint)
>> +{
>> +	struct cxl_port *sp = parent_port_of(endpoint);
>> +	struct cxl_hdm *cxlhdm;
>> +	int rc;
>> +
>> +	for (; sp && !is_cxl_root(sp); sp = parent_port_of(sp)) {
> 
> Might as well set sp as loop init rather than earlier.
> 

Yep, will do.

>> +		cxlhdm = dev_get_drvdata(&sp->dev);
>> +		if (cxlhdm && cxlhdm->decoder_count > 0)
>> +			continue;
>> +
>> +		if (!sp->reg_map.component_map.hdm_decoder.valid || !cxlhdm) {
>> +			dev_err(&sp->dev, "Missing HDM decoder capability\n");
>> +			return -ENXIO;
>> +		}
>> +
>> +		if (sp->total_dports == 1) {
>> +			dev_dbg(&sp->dev, "Fallback to passthrough decoder\n");
>> +			rc = devm_cxl_add_passthrough_decoder(sp);
>> +			if (rc)
>> +				return rc;
>> +		}
>> +	}
>> +
>> +	if (is_cxl_root(sp))
>> +		return 0;
> 
> Could move this test into the loop, alowing the loop condition to be simplified
> to just sp;
> 

Sure!

>> +
>> +	return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_validate_endpoint_hdm_setup, "CXL");
>> +
>