Re: [PATCH v8 07/10] cxl/pci: Split BI capability probe from setup

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  9 Sep 2026 10:02:59 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Decouple the topology read-only capability verification phase from
> cxl_bi_setup() into cxl_bi_probe_capable(), recording the result in
> cxlds->bi_capable.
> 
> This allows further dealing with auto-committed BI hdm decoders;
> having such knowledge upon decoder enumeration time.
> 
> No functional change intended.
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
One question about whether you can reduce the diff by shuffling things
a little.  Either way
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

> ---
>  drivers/cxl/core/pci.c | 87 +++++++++++++++++++++++++-----------------
>  drivers/cxl/cxl.h      |  1 +
>  drivers/cxl/port.c     |  1 +
>  include/cxl/cxl.h      |  2 +
>  4 files changed, 56 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 8c3c00137849..95064be6faad 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -1297,59 +1297,37 @@ static int cxl_bi_enable_path(struct cxl_dev_state *cxlds,

> +/*
> + * An SBR wipes the device's BI Enable; an FLR leaves it alone.
> + * The check is against the hardware, not decoder state: BI is
> + * enabled at probe, so it can be wiped with no decoder ever
> + * committed. A wipe invalidates the software state; BI is never
> + * re-enabled here.
> + */
> +void cxl_bi_reset_detected(struct cxl_port *endpoint)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(endpoint->uport_dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	void __iomem *bi = endpoint->regs.bi_decoder;
> +
> +	if (!cxlds->bi)
> +		return;
> +
> +	if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE,
> +		      readl(bi + CXL_BI_DECODER_CTRL_OFFSET)))
> +		return;
> +
> +	dev_dbg(cxlds->dev, "BI disabled by reset\n");
> +	cxlds->bi = false;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_bi_reset_detected, "CXL");

Why did this need to move?  Just to have a nicer code arrangement?
Maybe not worth it given it makes it look like this changed
and it didn't.  I'd leave it where diff thinks it was originally.



> +
> +int cxl_bi_setup(struct cxl_port *endpoint)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(endpoint->uport_dev);
> +	struct cxl_dport *dport = endpoint->parent_dport;
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	int rc;
> +
> +	if (!cxlds->bi_capable)
> +		return 0;
> +