lkml.org 
[lkml]   [2019]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] iio: core: print error message on debugfs register failure
Date
On Tue, 2019-12-17 at 16:19 +0100, Greg KH wrote:
> [External]
>
> On Wed, Dec 11, 2019 at 05:16:36PM +0200, Alexandru Ardelean wrote:
> > From: Michael Hennerich <michael.hennerich@analog.com>
> >
> > If there's a failure when registering a debugfs entry for a device,
> > don't
> > silently ignore the failure. Instead, print an error message and an
> > error
> > code signaling the failure.
>
> No, never do that.
>
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/industrialio-core.c | 34 +++++++++++++++++++++++++++------
> > 1 file changed, 28 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-core.c
> > b/drivers/iio/industrialio-core.c
> > index dab67cb69fe6..662dabf8b08c 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -364,23 +364,45 @@ static const struct file_operations
> > iio_debugfs_reg_fops = {
> > static void iio_device_unregister_debugfs(struct iio_dev *indio_dev)
> > {
> > debugfs_remove_recursive(indio_dev->debugfs_dentry);
> > + indio_dev->debugfs_dentry = NULL;
> > }
> >
> > static void iio_device_register_debugfs(struct iio_dev *indio_dev)
> > {
> > + struct dentry *d;
> > + int ret;
> > +
> > if (indio_dev->info->debugfs_reg_access == NULL)
> > return;
> >
> > if (!iio_debugfs_dentry)
> > return;
> >
> > - indio_dev->debugfs_dentry =
> > - debugfs_create_dir(dev_name(&indio_dev->dev),
> > - iio_debugfs_dentry);
> > + d = debugfs_create_dir(dev_name(&indio_dev->dev),
> > iio_debugfs_dentry);
> > + if (IS_ERR_OR_NULL(d))
> > + goto error;
>
> No, don't do that, I spent a lot of time removing all of these pointless
> checks.
>
> No kernel code shoudl ever care if debugfs is workign or not, just make
> the call and move on. You can always pass the result of a debugfs call
> into another one with no problems.
>
> > +
> > + indio_dev->debugfs_dentry = d;
> > +
> > + d = debugfs_create_file("direct_reg_access", 0644,
> > + indio_dev->debugfs_dentry, indio_dev,
> > + &iio_debugfs_reg_fops);
> > +
> > + if (IS_ERR_OR_NULL(d))
> > + goto error;
>
> This check isn't even correct :)
>
> So this isn't going to work no matter what, sorry.
>
> just don't do this.
>
> The code is just fine as-is.

I'm fine with this answer.
I'll sync our IIO core code with upstream.
Mostly I just care that the diff between the 2 files is empty.

Thanks
Alex

>
> thanks,
>
> greg k-h
\
 
 \ /
  Last update: 2019-12-17 16:24    [W:0.040 / U:6.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site