Re: [PATCH] iio: core: print error message on debugfs register failure

[Greg KH]

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.

thanks,

greg k-h