Re: [PATCH v3 6/6] cxl/test: Add cxl_test accelerator driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Logic Error: Incorrect device ID field used for device serial number generation
- [Low] Build Failure: Undefined functions `cxl_set_capacity` and `devm_cxl_probe_mem`
--

commit 1221192460bb0724f4ff13dd01d1388c60a7fe54
Author: Dave Jiang <dave.jiang@intel.com>

cxl/test: Add cxl_test accelerator driver

This commit adds a type2 accelerator mock driver for the platform device to
simulate a CXL type2 device. It exercises the minimal API calls that a real
CXL type2 driver would utilize.

> diff --git a/tools/testing/cxl/test/accel.c b/tools/testing/cxl/test/accel.c
> new file mode 100644
> index 0000000000000..6e1dd6c815102
> --- /dev/null
> +++ b/tools/testing/cxl/test/accel.c
> @@ -0,0 +1,66 @@

[ ... ]

> +static int cxl_mock_accel_probe(struct platform_device *pdev)
> +{
> +	struct mock_cxl_accel *cxl_accel;
> +	struct device *dev = &pdev->dev;
> +	struct cxl_dev_state *cxlds;
> +	struct cxl_memdev *cxlmd;
> +	struct range mock_range;
> +	int rc;
> +
> +	cxl_accel = devm_cxl_dev_state_create(&pdev->dev, CXL_DEVTYPE_DEVMEM,
> +					      pdev->dev.id + 1, 0,

[Severity: Medium]
Is pdev->dev.id the correct field to use for the device serial number here?

For dynamically allocated platform devices, pdev->dev.id is usually 0, while
the actual instance identifier is stored in pdev->id. If pdev->dev.id always
evaluates to 0, will all simulated Type 2 accelerator instances end up
sharing the same serial number (1) and conflict during enumeration?

> +					      struct mock_cxl_accel, cxlds,
> +					      false);
> +	if (!cxl_accel)
> +		return -ENOMEM;
> +
> +	cxlds = &cxl_accel->cxlds;
> +	cxlds->media_ready = true;
> +	rc = cxl_set_capacity(cxlds, SZ_512M);

[Severity: Low]
Do cxl_set_capacity() and devm_cxl_probe_mem() below exist in the tree?

It looks like these functions are not defined or declared anywhere, which
might cause the module build to fail with implicit declaration and undefined
symbol errors. Other mock drivers in the tree appear to use
devm_cxl_add_memdev(). Could these function calls be leftovers from an
earlier design?

> +	if (rc)
> +		return rc;
> +
> +	cxlmd = devm_cxl_probe_mem(cxlds, &mock_range);
> +	if (IS_ERR(cxlmd))
> +		return PTR_ERR(cxlmd);
> +	cxl_accel->cxlmd = cxlmd;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610222328.2272427-1-dave.jiang@intel.com?part=6