Kobayashi,Daisuke wrote:
> > Dan Williams wrote:
> > > Kobayashi,Daisuke wrote:
> > > > +static struct attribute_group cxl_rcd_group = {
> > > > + .attrs = cxl_rcd_attrs,
> > > > + .is_visible = cxl_rcd_visible,
> > > > +};
> > > > +
> > > > +__ATTRIBUTE_GROUPS(cxl_rcd);
> > > > +
> > > > static int cxl_pci_probe(struct pci_dev *pdev, const struct
> > > > pci_device_id *id) {
> > > > struct pci_host_bridge *host_bridge =
> > > > pci_find_host_bridge(pdev->bus); @@ -806,6 +995,9 @@ static int
> > > cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > > > if (IS_ERR(mds))
> > > > return PTR_ERR(mds);
> > > > cxlds = &mds->cxlds;
> > > > + device_create_file(&pdev->dev, &dev_attr_rcd_link_cap);
> > > > + device_create_file(&pdev->dev, &dev_attr_rcd_link_ctrl);
> > > > + device_create_file(&pdev->dev, &dev_attr_rcd_link_status);
> > >
> > > No need to manually call device_create_file() when the attribute group is
> > > already registered below. I am surprised you did not get duplicate sysfs file
> > > warnings when registering these files twice.
> > >
> >
> > Thank you for pointing it out. Remove these calls.
> >
> If you are aware of the cause, I would appreciate your insight.
> In my environment, when I removed this device_create_file(),
> the file was not generated in sysfs. Therefore, I have not been
> able to remove this manual procedure at the moment. Is there a
> possibility that simply registering with
> struct pci_driver.driver.groups will not generate a sysfs file?
>
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.
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
device_remove_groups(&pdev->dev, cxl_rcd_groups) on remove.
I have attached the code below. Is my usage of probe/remove correct? If so,
I will update and resubmit the patch with the following code.
static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
@@ -812,6 +885,9 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (IS_ERR(mds))
return PTR_ERR(mds);
cxlds = &mds->cxlds;
+ rc = device_add_groups(&pdev->dev, cxl_rcd_groups);
+ if (rc)
+ dev_warn(&pdev->dev, "Couldn't make rcd_groups (%d)\n", rc);
pci_set_drvdata(pdev, cxlds);
cxlds->rcd = is_cxl_restricted(pdev);
@@ -964,10 +1040,18 @@ static const struct pci_error_handlers cxl_error_handlers = {
.cor_error_detected = cxl_cor_error_detected,
};
+static void cxl_pci_remove(struct pci_dev *pdev)
+{
+ if (is_cxl_restricted(pdev))
+ device_remove_groups(&pdev->dev, cxl_rcd_groups);
+}
+
static struct pci_driver cxl_pci_driver = {
.name = KBUILD_MODNAME,
.id_table = cxl_mem_pci_tbl,
.probe = cxl_pci_probe,
+ .remove = cxl_pci_remove,
.err_handler = &cxl_error_handlers,
.driver = {
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
--
