On 24.11.22 10:35:38, Dan Williams wrote:
> In an RCH topology a CXL host-bridge as Root Complex Integrated Endpoint
> the represents the memory expander. Unlike a VH topology there is no
> CXL/PCIE Root Port that host the endpoint. The CXL subsystem maps this
> as the CXL root object (ACPI0017 on ACPI based systems) targeting the
> host-bridge as a dport, per usual, but then that dport directly hosts
> the endpoint port.
>
> Mock up that configuration with a 4th host-bridge that has a 'cxl_rcd'
> device instance as its immediate child.
>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> tools/testing/cxl/test/cxl.c | 151
> +++++++++++++++++++++++++++++++++++++++---
> tools/testing/cxl/test/mem.c | 37 ++++++++++
> 2 files changed, 176 insertions(+), 12 deletions(-)
One comment below.
> @@ -736,6 +779,87 @@ static void mock_companion(struct acpi_device *adev,
> struct device *dev)
> #define SZ_512G (SZ_64G * 8)
> #endif
>
> +static __init int cxl_rch_init(void)
> +{
> + int rc, i;
> +
> + for (i = 0; i < ARRAY_SIZE(cxl_rch); i++) {
> + int idx = NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + i;
> + struct acpi_device *adev = &host_bridge[idx];
> + struct platform_device *pdev;
> +
> + pdev = platform_device_alloc("cxl_host_bridge", idx);
> + if (!pdev)
> + goto err_bridge;
> +
> + mock_companion(adev, &pdev->dev);
> + rc = platform_device_add(pdev);
> + if (rc) {
> + platform_device_put(pdev);
> + goto err_bridge;
> + }
> +
> + cxl_rch[i] = pdev;
> + mock_pci_bus[idx].bridge = &pdev->dev;
> + rc = sysfs_create_link(&pdev->dev.kobj, &pdev->dev.kobj,
> + "firmware_node");
> + if (rc)
> + goto err_bridge;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(cxl_rcd); i++) {
> + int idx = NR_MEM_MULTI + NR_MEM_SINGLE + i;
> + struct platform_device *rch = cxl_rch[i];
> + struct platform_device *pdev;
> +
> + pdev = platform_device_alloc("cxl_rcd", idx);
> + if (!pdev)
> + goto err_mem;
> + pdev->dev.parent = &rch->dev;
> + set_dev_node(&pdev->dev, i % 2);
> +
> + rc = platform_device_add(pdev);
> + if (rc) {
> + platform_device_put(pdev);
> + goto err_mem;
> + }
> + cxl_rcd[i] = pdev;
> + }
> +
> + return 0;
> +
> +err_mem:
> + for (i = ARRAY_SIZE(cxl_rcd) - 1; i >= 0; i--)
> + platform_device_unregister(cxl_rcd[i]);
> +err_bridge:
platform_device_unregister() is safe to be used with NULL, so we can
have a single entry of this unregister code ...
> + for (i = ARRAY_SIZE(cxl_rch) - 1; i >= 0; i--) {
> + struct platform_device *pdev = cxl_rch[i];
> +
> + if (!pdev)
> + continue;
> + sysfs_remove_link(&pdev->dev.kobj, "firmware_node");
> + platform_device_unregister(cxl_rch[i]);
> + }
> +
> + return rc;
> +}
> +
> +static void cxl_rch_exit(void)
> +{
> + int i;
> +
> + for (i = ARRAY_SIZE(cxl_rcd) - 1; i >= 0; i--)
> + platform_device_unregister(cxl_rcd[i]);
> + for (i = ARRAY_SIZE(cxl_rch) - 1; i >= 0; i--) {
> + struct platform_device *pdev = cxl_rch[i];
> +
> + if (!pdev)
> + continue;
> + sysfs_remove_link(&pdev->dev.kobj, "firmware_node");
> + platform_device_unregister(cxl_rch[i]);
> + }
> +}
... and have a single function for both. This reduces code
duplication here.
-Robert
