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

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.

Logan