Re: [PATCH v1 10/29] cxl/region: Add function to find a port's switch decoder by range

[Robert Richter <rrichter@amd.com>]

On Tue, Jan 07, 2025 at 01:38:33PM -0500, Gregory Price wrote:
> On Tue, Jan 07, 2025 at 03:09:56PM +0100, Robert Richter wrote:
> > Factor out code to find the switch decoder of a port for a specific
> > address range. Reuse the code to search a root decoder, create the
> > function cxl_port_find_switch_decoder() and rework
> > match_root_decoder_by_range() to be usable for switch decoders too.
> > 
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> >  drivers/cxl/core/region.c | 43 +++++++++++++++++++++++----------------
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> ... snip ...
> >  
> > -	cxlrd_dev = device_find_child(&iter->dev, hpa,
> > -				      match_root_decoder_by_range);
> > -	if (!cxlrd_dev) {
> > +	cxld = cxl_port_find_switch_decoder(iter, hpa);
> > +	if (!cxld) {
> 
> Are there scenarios where this would return a different decoder than
> previously? For example, is there an assumption that root decoders
> will be search first, as opposed to intermediate decoders?
> 
> The match function was changed to check is_switch_decoder from
> is_root_decoder, i'm just worried about the case where we might have
> multiple decoders in the path and the switch decoder is hit first -
> resulting in the wrong decoder returned.

Intermediate decoders never share the port with a root decoder, both
decoders always have different parents. So depending on the direction
of walking the tree, the same port is always found first. That is,
starting at the endpoint, an intermediate switch decoder would be
found first. Search is always deterministic.

Note the "root_decoder" is a subset of "switch_decoder" here.

-Robert

> 
> ~Gregory