Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function

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

On Tue, 23 Apr 2024 19:33:20 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Apr 23, 2024 at 06:20:14PM +0200, Marek Behún wrote:
> > On Tue, 23 Apr 2024 19:02:21 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:  
> 
> ...
> 
> > > > +static inline struct dentry *
> > > > +devm_debugfs_create_dir(struct device *dev, const char *name,
> > > > +			struct dentry *parent)
> > > > +{
> > > > +	struct dentry *dentry;
> > > > +	int err;    
> > >   
> > > > +	dentry = debugfs_create_dir(name, parent);
> > > > +	if (IS_ERR(dentry))
> > > > +		return dentry;    
> > > 
> > > Why do we care?
> > >   
> > > > +	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
> > > > +				       dentry);
> > > > +	if (err < 0)
> > > > +		return ERR_PTR(err);
> > > > +
> > > > +	return dentry;
> > > > +}    
> > > 
> > > static inline struct dentry *
> > > devm_debugfs_create_dir(struct device *dev, const char *name, struct dentry *parent)
> > > {
> > > 	struct dentry *dentry;
> > > 	int err;
> > > 
> > > 	dentry = debugfs_create_dir(name, parent);
> > > 
> > > 	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, dentry);
> > > 	if (err)
> > > 		return ERR_PTR(err);  
> > 
> > But if the dir creation fails, there is no need to allocate devm
> > destructor...  
> 
> Yes, but why do we care about that?
> debugfs is designed the way that we should ignore errors from it.
> 
> This discussion let me think, that probably the best solution is to add a field
> to struct device_driver to handle this.
> 
> debugfs will appear only when device is completely instantiated, removed before
> anything, and a lot of other benefits (see the story behind dev_groups as a
> reference).
> 

Okay, this seems way beyond the scope of what I am trying to do here.

Since Matti is also not entirely content with these additions to the
devm-helpers.h header and you suggested to use gpiod_to_irq(), I shall
drop these changes from the next version and just use
devm_add_action_or_reset() in the driver directly.

Marek