Re: [PATCH v2 08/17] firmware: turris-mox-rwtm: Don't create own kobject type

["Marek Behún" <kabel@kernel.org>]

On Thu, 13 Jun 2024 22:32:28 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jun 13, 2024 at 6:11 PM Marek Behún <kabel@kernel.org> wrote:
> >
> > In order to create attribute files in /sys/firmware/turris-mox-rwtm,
> > this driver creates it's own kobject type.  
> 
> its
> 
> > Simplify this by dropping this own kobject creation, and instead
> > creating standard device attribute files.
> >
> > For backwards compatibility with sysfs ABI, create a symlink
> > /sys/firmware/turris-mox-rwtm, pointing to this device's sysfs
> > directory.  
> 
> ...
> 
> 
> > +static void rwtm_firmware_symlink_drop(void *)  
> 
> Interesting, can we actually name the parameter, like "unused"?
> 
> > +{
> > +       sysfs_remove_link(firmware_kobj, DRIVER_NAME);  
> 
> But why not provide a fimware_kobj pointer as a parameter?
> 
> > +}  
> 
> ...
> 
> > +       /*
> > +        * For sysfs ABI compatibility, create symlink
> > +        * /sys/firmware/turris-mox-rwtm to this device's sysfs directory.
> > +        */
> > +       ret = sysfs_create_link(firmware_kobj, &dev->kobj, DRIVER_NAME);
> > +       if (!ret)
> > +               devm_add_action_or_reset(dev, rwtm_firmware_symlink_drop, NULL);  
> 
> This means that it will remove the link in case devm_add_action()
> fails. Is it okay?

IMO rather to drop the symlink (which is created for backwards
compatibility anyway) in the improbable case that
devm_add_action() fails, then than to leave it dangling there on driver
unbind.