On Wed, 3 Apr 2024 09:40:27 +0000
"Daisuke Kobayashi (Fujitsu)" <kobayashi.da-06@xxxxxxxxxxx> 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?
>
> > > > pci_set_drvdata(pdev, cxlds);
> > > >
> > > > cxlds->rcd = is_cxl_restricted(pdev); @@ -967,6 +1159,7 @@ static
> > > > struct pci_driver cxl_pci_driver = {
> > > > .err_handler = &cxl_error_handlers,
> > > > .driver = {
> > > > .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > > > + .dev_groups = cxl_rcd_groups,
Odd though it may seem, try setting
.dev_groups in the outer structure not the inner one.
.err_handler = &....
.dev_groups = cxl_rcd_groups,
.driver = {
...
},
Similar to:
https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/sp-pci.c#L592
For reasons I don't follow, __pci_register_driver() overrides the internal one.
Some ancient bit of code migration that never finished?
https://elixir.bootlin.com/linux/latest/source/drivers/pci/pci-driver.c#L1447
I did some digging.
https://lore.kernel.org/all/20190731124349.4474-2-gregkh@xxxxxxxxxxxxxxxxxxx/
So this got added to the driver core fairly recently (only 4 years ago ;)
The the dev_groups was added to pci in
https://lore.kernel.org/all/20210512142648.666476-8-andrey.grodzovsky@xxxxxxx/
I'm not sure why the bounce via pci_driver is needed though.
Greg, looks like this came from usb originally, can you recall the reasoning?
> > > > },
> > > > };
> > > >
> > > > --
> > > > 2.43.0
> > > >
> > > >
> > >
> >
>
>
