Re: [PATCH v2 1/4] dmaengine: add per-channel sysfs attribute groups via chan_groups

On Tue, Aug 18, 2026 at 04:44:30PM -0600, Logan Gunthorpe wrote:
>
>
> On 2026-08-18 14:56, Frank Li wrote:
> > On Mon, Jul 27, 2026 at 12:48:41PM -0600, Logan Gunthorpe wrote:
> >> @@ -1098,6 +1124,7 @@ static int __dma_async_device_channel_register(struct dma_device *device,
> >>
> >>  	chan->dev->device.class = &dma_devclass;
> >>  	chan->dev->device.parent = device->dev;
> >> +	chan->dev->device.groups = device->chan_groups;
> >
> > why not call device_add_groups() with const groups, so needn't save
> > "device->chan_groups" at all.
> That would mean each driver looping over dma->channels calling
> device_add_groups() itself, which is what I was pushing back against in
> my previous response, only a bit worse:
>
> Letting the core set dev->groups is also better than adding the groups
> after registration: device_add() applies dev->groups before signalling
> to userspace that the group has been created. If the driver is
> responsible for calling dma_add_groups() after registration, then
> userspace can be signalled before all the groups are added and ready,
> causing subtle issues with udev rules, etc. I'm not sure it can all be
> done correctly without introducing complicated race issues.
>
> I really think setting device.groups before calling device_add() is by
> far the better solution here.
>
> I had to go digging for this, but I was reminded of a (older than I had
> thought) blog post that GKH wrote more than a decade ago to describe
> this exact issue:
>
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
>
> The recommendation: just set the groups field in the device object.

Thanks, let me think to how to handle it better.  This field just use once
when register.

Frank

>
> Logan