[PATCH] staging: axis-fifo: Fix three real issues

[PATCH] staging: axis-fifo: Fix three real issues

[Breno Rodrigues Alves]

Fix three issues in the Xilinx AXI-Stream FIFO driver:

  1. axis_fifo_debugfs_init() did not check the return value of
     debugfs_create_dir().  When CONFIG_DEBUG_FS=n, the function
     returns ERR_PTR(-ENODEV); passing it as a parent to
     debugfs_create_file() triggers a WARN in the kernel debugfs
     core.  Bail out early if the directory was not created.

  2. axis_fifo_read() returned -EINVAL when the user buffer was
     smaller than the available data.  The user argument is not
     invalid - it is just too small - so return -ENOBUFS, which is
     the canonical errno for that situation (see recv(2)).

  3. axis_fifo_probe() filled in a 'struct resource *r_mem' pointer
     that is never read.  Pass NULL instead.

No functional change for the normal path.

Signed-off-by: Breno Rodrigues Alves <breno3011alves@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 3d358f919..c19cb68de 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -148,7 +148,7 @@ static ssize_t axis_fifo_read(struct file *f, char __user *buf,
 	words_available = bytes_available / sizeof(u32);
 
 	if (bytes_available > len) {
-		ret = -EINVAL;
+		ret = -ENOBUFS;
 		goto err_flush_rx;
 	}
 
@@ -378,6 +378,8 @@ DEFINE_SHOW_ATTRIBUTE(axis_fifo_debugfs_regs);
 static void axis_fifo_debugfs_init(struct axis_fifo *fifo)
 {
 	fifo->debugfs_dir = debugfs_create_dir(dev_name(fifo->dt_device), NULL);
+	if (IS_ERR_OR_NULL(fifo->debugfs_dir))
+		return;
 
 	debugfs_create_file("regs", 0444, fifo->debugfs_dir, fifo,
 			    &axis_fifo_debugfs_regs_fops);
@@ -428,7 +430,6 @@ static int axis_fifo_parse_dt(struct axis_fifo *fifo)
 
 static int axis_fifo_probe(struct platform_device *pdev)
 {
-	struct resource *r_mem;
 	struct device *dev = &pdev->dev;
 	struct axis_fifo *fifo = NULL;
 	int rc = 0; /* error return value */
@@ -447,7 +448,7 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	mutex_init(&fifo->read_lock);
 	mutex_init(&fifo->write_lock);
 
-	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, &r_mem);
+	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(fifo->base_addr))
 		return PTR_ERR(fifo->base_addr);
 
-- 
2.56.0

Re: [PATCH] staging: axis-fifo: Fix three real issues

[Greg Kroah-Hartman]

On Thu, Oct 22, 2026 at 06:53:49PM -0300, Breno Rodrigues Alves wrote:
> Fix three issues in the Xilinx AXI-Stream FIFO driver:
> 
>   1. axis_fifo_debugfs_init() did not check the return value of
>      debugfs_create_dir().  When CONFIG_DEBUG_FS=n, the function
>      returns ERR_PTR(-ENODEV); passing it as a parent to
>      debugfs_create_file() triggers a WARN in the kernel debugfs
>      core.  Bail out early if the directory was not created.

This is not a real issue, you should NEVER be checking the return value
of any debugfs file, and no WARN will ever fire (if so, please show me
where as that should not be happening.)

What tool told you this?  Please fix it.

thanks,

greg k-h

Re: [PATCH] staging: axis-fifo: Fix three real issues

[Breno Rodrigues Alves]

Hi Greg,

You are right, and I apologize for the noise.  No tool told me this -
it was my own reasoning.  I had the (outdated) impression that passing
an ERR_PTR parent from debugfs_create_dir() to debugfs_create_file()
would trigger a WARN in the debugfs core.  That was true years ago, but
modern debugfs handles ERR_PTR parents gracefully, and I should have
verified the claim on a CONFIG_DEBUG_FS=n kernel before sending the
patch.

The other two changes are independent of that assumption:

  - the -EINVAL -> -ENOBUFS change in axis_fifo_read(), which changes
    the errno returned when the user buffer is smaller than the
    available data;

  - the removal of the unused 'struct resource *r_mem' local.

If either of those is worth keeping, I can resend with just those two.
If the whole patch looks like churn, please let me know and I will drop
it.

Sorry again,
Breno

Re: [PATCH] staging: axis-fifo: Fix three real issues

[Dan Carpenter]

On Fri, Oct 23, 2026 at 05:52:00AM -0300, Breno Rodrigues Alves wrote:
> Hi Greg,
> 
> You are right, and I apologize for the noise.  No tool told me this -
> it was my own reasoning.  I had the (outdated) impression that passing
> an ERR_PTR parent from debugfs_create_dir() to debugfs_create_file()
> would trigger a WARN in the debugfs core.  That was true years ago, but
> modern debugfs handles ERR_PTR parents gracefully, and I should have
> verified the claim on a CONFIG_DEBUG_FS=n kernel before sending the
> patch.

It's been quite that way for quite a while.  Since Jan 2019.

> 
> The other two changes are independent of that assumption:
> 
>   - the -EINVAL -> -ENOBUFS change in axis_fifo_read(), which changes
>     the errno returned when the user buffer is smaller than the
>     available data;

I would argue that -EINVAL is the most common error code in this
situation.  I would accept other returns as well.  As a reviewer I
would probably leave this up to the author.

> 
>   - the removal of the unused 'struct resource *r_mem' local.

This is fine as a cleanup.

regards,
dan carpenter

[PATCH v2] staging: axis-fifo: remove unused r_mem local variable

[Breno Rodrigues Alves]

Remove 'struct resource *r_mem' from axis_fifo_probe().  It is filled
in by devm_platform_get_and_ioremap_resource() but never read; pass
NULL instead.

This is v2 of a three-change series.  The debugfs_create_dir() check
was dropped because debugfs has handled ERR_PTR parents gracefully
since 2019 (thanks Greg and Dan for the correction).  The
-EINVAL -> -ENOBUFS change was dropped as well, after Dan Carpenter
noted that -EINVAL is the common errno for this path and that he
would leave the choice to the author.

Signed-off-by: Breno Rodrigues Alves <breno3011alves@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 3d358f919..464368e04 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -428,7 +428,6 @@ static int axis_fifo_parse_dt(struct axis_fifo *fifo)
 
 static int axis_fifo_probe(struct platform_device *pdev)
 {
-	struct resource *r_mem;
 	struct device *dev = &pdev->dev;
 	struct axis_fifo *fifo = NULL;
 	int rc = 0; /* error return value */
@@ -447,7 +446,7 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	mutex_init(&fifo->read_lock);
 	mutex_init(&fifo->write_lock);
 
-	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, &r_mem);
+	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(fifo->base_addr))
 		return PTR_ERR(fifo->base_addr);
 
-- 
2.56.0

Re: [PATCH v2] staging: axis-fifo: remove unused r_mem local variable

[Dan Carpenter]

On Fri, Oct 23, 2026 at 07:39:43AM -0300, Breno Rodrigues Alves wrote:
> Remove 'struct resource *r_mem' from axis_fifo_probe().  It is filled
> in by devm_platform_get_and_ioremap_resource() but never read; pass
> NULL instead.
> 
> This is v2 of a three-change series.  The debugfs_create_dir() check
> was dropped because debugfs has handled ERR_PTR parents gracefully
> since 2019 (thanks Greg and Dan for the correction).  The
> -EINVAL -> -ENOBUFS change was dropped as well, after Dan Carpenter
> noted that -EINVAL is the common errno for this path and that he
> would leave the choice to the author.

This kind of meta commentary goes below the --- cut off line

> 
> Signed-off-by: Breno Rodrigues Alves <breno3011alves@gmail.com>
> ---
  ^^^
here.

https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

As always wait until tomorrow to send v3.

regards,
dan carpenter

Re: [PATCH v2] staging: axis-fifo: remove unused r_mem local variable

[Greg Kroah-Hartman]

On Fri, Oct 23, 2026 at 07:39:43AM -0300, Breno Rodrigues Alves wrote:
> Remove 'struct resource *r_mem' from axis_fifo_probe().  It is filled
> in by devm_platform_get_and_ioremap_resource() but never read; pass
> NULL instead.
> 
> This is v2 of a three-change series.  The debugfs_create_dir() check
> was dropped because debugfs has handled ERR_PTR parents gracefully
> since 2019 (thanks Greg and Dan for the correction).  The
> -EINVAL -> -ENOBUFS change was dropped as well, after Dan Carpenter
> noted that -EINVAL is the common errno for this path and that he
> would leave the choice to the author.
> 
> Signed-off-by: Breno Rodrigues Alves <breno3011alves@gmail.com>
> ---
>  drivers/staging/axis-fifo/axis-fifo.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index 3d358f919..464368e04 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -428,7 +428,6 @@ static int axis_fifo_parse_dt(struct axis_fifo *fifo)
>  
>  static int axis_fifo_probe(struct platform_device *pdev)
>  {
> -	struct resource *r_mem;
>  	struct device *dev = &pdev->dev;
>  	struct axis_fifo *fifo = NULL;
>  	int rc = 0; /* error return value */
> @@ -447,7 +446,7 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	mutex_init(&fifo->read_lock);
>  	mutex_init(&fifo->write_lock);
>  
> -	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, &r_mem);
> +	fifo->base_addr = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);

This doesn't make sense, if r_mem is really not needed, then call the
proper function instead (exercise left to the reader to determine what
that is...)

And how was this found?

How was this tested?

thanks,

greg k-h