On Mon, Sep 25, 2023 at 03:01:25PM -0500, Ben Cheatham wrote:
> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie
> device) for CXL RCH root ports. The file will print the RCRB base
> MMIO address of the root port when read and will be used by
> users looking to inject CXL EINJ error types for RCH hosts.
I guess this is talking about a sysfs file? If so, maybe mention that
explicitly in the subject and commit log.
I don't know how you decided to capitalize CXL initialisms and not
"pcie", but I usually use "PCIe".
> +static struct cxl_port *cxl_root;
> +
> +void set_cxl_root(struct cxl_port *root_port)
> +{
> + cxl_root = root_port;
> +}
Is there always at most one cxl_root? Seems worth a one-line comment
at the static data item, since in the world of devices, data is
usually in a per-device struct, not in a single static item.
> @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> if (rc)
> return ERR_PTR(rc);
>
> + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group);
> + if (rc)
> + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n",
> + rc);
Is there any way to create this with an attribute group that the sysfs
infrastructure adds automatically? I'm not suggesting you have a race
condition here, but using the sysfs infrastructure avoids a lot of
potential problems.
Bjorn
