Re: [RFC 3/5] cxl: Separate coherence from target type

[Huang, Ying]

"Huang, Ying" <ying.huang@xxxxxxxxx> writes:

> Gregory Price <gourry@xxxxxxxxxx> writes:
>
>> On Wed, Sep 25, 2024 at 10:46:45AM +0800, Huang Ying wrote:

[snip]

>
>>> @@ -1925,6 +1933,29 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>>>  		return -ENXIO;
>>>  	}
>>>  
>>> +	/* Set the coherence of region to that of the first endpoint */
>>> +	if (cxlr->coherence == CXL_DECODER_INVALIDCOH) {
>>> +		unsigned long flags = cxlrd->cxlsd.cxld.flags;
>>> +		enum cxl_decoder_coherence coherence = cxled->cxld.coherence;
>>> +
>>> +		cxlr->coherence = coherence;
>>> +		if ((coherence == CXL_DECODER_HOSTONLYCOH &&
>>> +		     !(flags & CXL_DECODER_F_HOSTONLYCOH)) ||
>>> +		    (coherence == CXL_DECODER_DEVCOH &&
>>> +		     !(flags & CXL_DECODER_F_DEVCOH))) {
>>
>> silly nit but my gut tells me we can make this less ugly.
>> Not a blocker though.
>
> Yes.  This looks urgly.  Will think about how to improve it.

Reviewed the code again.  However, I don't have any idea to improve
this.  Do you have some suggestion?

>>> +			dev_dbg(&cxlr->dev,
>>> +"%s:%s endpoint coherence: %d isn't supported by root decoder: %#lx\n",
>>> +				dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
>>> +				coherence, flags);
>>> +			return -ENXIO;
>>> +		}
>

--
Best Regards,
Huang, Ying