Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 28 Jul 2022 11:53:33 -0500 | Subject | Re: [PATCH v2 1/3] drivers: misc: intel_sysid: Add sysid from arch to drivers | From | Pierre-Louis Bossart <> |
| |
On 7/28/22 10:59, Greg KH wrote: > On Thu, Jul 28, 2022 at 10:37:37AM -0500, Pierre-Louis Bossart wrote: >> Thanks for the review Greg, >> >>>> +static int intel_sysid_probe(struct platform_device *pdev) >>>> +{ >>>> + struct intel_sysid *sysid; >>>> + struct resource *regs; >>>> + >>>> + sysid = devm_kzalloc(&pdev->dev, sizeof(struct intel_sysid), >>>> + GFP_KERNEL); >>>> + if (!sysid) >>>> + return -ENOMEM; >>>> + >>>> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + if (!regs) >>>> + return -ENXIO; >>>> + >>>> + sysid->regs = devm_ioremap_resource(&pdev->dev, regs); >>>> + if (IS_ERR(sysid->regs)) >>>> + return PTR_ERR(sysid->regs); >>>> + >>>> + platform_set_drvdata(pdev, sysid); >>>> + >>>> + return devm_device_add_group(&pdev->dev, &intel_sysid_attr_group); >>> >>> You just raced with userspace and lost. Please use the default group >>> for the platform device. >>> >>> I need to go remove this function, it should not be used at all as it is >>> broken. >> >> Can you elaborate on the issue and suggested replacement? >> >> We used this function for the SoundWire sysfs based on your review >> comments (2 years ago?) that we should not muck with kobj, and that >> function devm_device_add_group() is also used in a probe function. > > Use the default_groups pointer in the driver structure.
did you mean dev_groups?
I am not following the idea, for SoundWire all the attributes are really device-specific or described by ACPI and cannot be hard-coded in the driver structure.
| ![\](/images/icornerr.gif) |