Re: [PATCH v2] cxl/region: Add cxl_decoder_is_passthrough() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `cxl_decoder_is_passthrough()` incorrectly identifies single-target root decoders as passthrough decoders, contradicting the commit message.
--

--- Patch [1]: [PATCH v2] cxl/region: Add cxl_decoder_is_passthrough() helper ---
commit 9b921fd6e2f183466d76ed78181b72b15a45105b
Author: Richard Cheng <icheng@nvidia.com>
cxl/region: Add cxl_decoder_is_passthrough() helper

This commit abstracts the passthrough decoder check into a standalone
helper function. The commit message explicitly notes this is necessary
because checking only for a NULL ->commit() would incorrectly identify
root decoders and DVSEC-emulated endpoint decoders as passthrough.

Link: https://lore.kernel.org/linux-cxl/20260727103743.63343-1-icheng@nvidia.com/

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
[ ... ]
> +/*
> + * A single-dport host-bridge need not publish an HDM decoder capability
> + * when passthrough decode can be assumed. The resulting decoder is a
> + * software-only construct with no registers to program, so it carries no
> + * ->commit() operation, see devm_cxl_add_passthrough_decoder().
> + *
> + * A NULL ->commit() alone does not identify one, it is also NULL for
> + * DVSEC-emulated endpoint decoders. Test the decoder type and target
> + * count as well.
> + */
> +static bool cxl_decoder_is_passthrough(struct cxl_decoder *cxld)
> +{
> +	if (cxld->commit)
> +		return false;
> +
> +	if (!is_switch_decoder(&cxld->dev))
> +		return false;
> +
> +	return to_cxl_switch_decoder(&cxld->dev)->nr_targets <= 1;

[Severity: Medium]
Does this helper correctly exclude root decoders as intended?

The commit message states that root decoders leave ->commit() NULL and
that a future caller testing ->commit() alone would silently include them.
However, looking at is_switch_decoder() in drivers/cxl/core/port.c, it
returns true for root decoders:

bool is_switch_decoder(struct device *dev)
{
        return is_root_decoder(dev) || dev->type == &cxl_decoder_switch_type;
}

Since is_switch_decoder() evaluates to true for root decoders, won't this
function incorrectly return true for single-target root decoders? This
appears to create a misleading abstraction that contradicts the commit
message's claim about ruling out root decoders.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805065935.27837-1-icheng@nvidia.com?part=1