RE: [PATCH v3 1/3] Add sysfs attribute for CXL 1.1 device link status

[Dan Williams <dan.j.williams@intel.com>]

Daisuke Kobayashi (Fujitsu) wrote:
[..]
> I would like to report on some additional findings.
> The process of registering cxl_rcd_groups to struct pci_driver.driver.dev_groups seems
> to not generate a file in sysfs when looking at the contents of the module_pci_driver() macro.

Oh, apologies, and thanks for taking a deeper look. The failure is
because my suggested example led you astray. I suggested this:

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 2ff361e756d6..eec04f103aa8 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -971,6 +971,7 @@ static struct pci_driver cxl_pci_driver = {
        .err_handler            = &cxl_error_handlers,
        .driver = {
                .probe_type     = PROBE_PREFER_ASYNCHRONOUS,
+               .dev_groups     = cxl_rcd_groups,
        },
 }; 


...the correct place to put it is here:

@@ -969,6 +969,7 @@ static struct pci_driver cxl_pci_driver = {
        .id_table               = cxl_mem_pci_tbl,
        .probe                  = cxl_pci_probe,
        .err_handler            = &cxl_error_handlers,
+       .dev_groups             = cxl_rcd_groups,
        .driver = {
                .probe_type     = PROBE_PREFER_ASYNCHRONOUS,
        },


...otherwise __pci_register_driver() will overwrite it. This is a subtle
bug given probe_type is directly initialized in .driver.

> For this feature, I think it would be best to output the values to a directory
> of /sys/bus/pci/devices/<pci-addr>/. To output to this directory, the attribute would
> need to be registered to pci_dev.dev.
> My current understanding is that the best way to do this would be to register the attribute
> with device_add_groups(&pdev->dev, cxl_rcd_groups) on probe and remove the files with

No, the dynamic sysfs registration APIs should be avoided when possible.
The above fix is what you need.