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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> > ---
> > 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
