Re: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code

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

On Thu, 13 Jun 2024 16:37:23 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Thu, 13 Jun 2024, Marek Behún wrote:
> 
> > On Thu, 13 Jun 2024 13:19:47 +0300 (EEST)
> > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> >   
> > > On Thu, 13 Jun 2024, Marek Behún wrote:
> > >   
> > > > On Thu, 13 Jun 2024 11:31:43 +0300 (EEST)
> > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > >     
> > > > > Is empty .release necessary at all? I found some kobj_type structs without 
> > > > > .release so I'd expect it to be unnecessary.    
> > > > 
> > > > lib/kobject.c function kobject_cleanup() has the following:
> > > > 
> > > >   if (t && !t->release)
> > > >     pr_debug("'%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",    
> > > 
> > > Hmm, the plot thickens... that documentation file says:
> > > 
> > > 'Do not try to get rid of this warning by providing an "empty" release 
> > > function.'
> > > 
> > > ?  
> > 
> > This whole thing stinks. I will rewrite it so that the attributes will
> > be under the device's kobject, as they should be. This way I can get
> > rid of the whole own kobject type.  
> 
> Yeah, I didn't really understand why they weren't there in the first 
> place.
> 
> > Then I will add a symlink from /sys/firmware/turris-mox-rwtm to the
> > device, for sysfs ABI compatibility.  
> 
> 
> BTW (unrelated to any of your patches unless I missed something)...
> You might want to check one unlock_mutex: label that seemed to be 100% 
> duplicate the tail of the return path.

Do you mean this?

          mutex_unlock(&rwtm->busy);
          return len;
  unlock_mutex:
          mutex_unlock(&rwtm->busy);
          return ret;

It's not 100% duplicate, but yes, I could do ret = len.

The thing is that this whole debugfs code is going away. I wanted to
clean the driver up a little before converting this debugfs code to
keyctl (which is the correct API for the thing that is now exposed to
userspace via debugfs).

Marek