Re: [PATCH 13/13] HID: playstation: report DualSense hardware and firmware version.

["Barnabás Pőcze" <pobrn@protonmail.com>]

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