2020. december 27., vasárnap 23:27 keltezéssel, Roderick Colenbrander írta: > [...] > > > > - ret = sysfs_create_group(&hdev->dev.kobj, &ps_device_attribute_group); > > > > > > > > > > > > > > It's a minor thing, but I think `device_{add,remove}_group()` would be better > > > here in the sense that it expresses the fact that the group is added to a device, > > > not just any object better. > > > > Agreed, that's nicer I wasn't aware of it. I try to follow what other > > hid drivers do and they all used the kobj directly, which honestly > > felt nasty. Will change it to this. > > Actually devm_device_add_group seems to be even nicer. Surprisingly it > isn't widely used yet. > > Roderick Well, indeed, although I believe that shouldn't be used here. Consider what happens if the hid-playstation module is unloaded. The attributes of the HID device will not be unregistered, but the backing functions/etc. are unloaded, so reading/writing them will have undesirable effects - I imagine. So in either case, you'll need to use `[devm_]device_remove_group()`, and for that reason I think using the devm_* variant is less efficient. Please note, that I am not 100% sure this hypothesis is correct, but I'm pretty sure. Regards, Barnabás Pőcze