[PATCH v6 0/2] staging: axis-fifo: refactor and cleanup

[PATCH v6 0/2] staging: axis-fifo: refactor and cleanup

[Gustavo Piaz da Silva]

This series cleans up the axis-fifo driver probe function logic.

Patch 1 aligns the struct types with the OF API.
Patch 2 refactors the parsing logic to be simpler and less verbose.

Changes in v6:
 - Removed the helper function axis_fifo_get_u32() entirely.
 - Kept 'node' local variable in parse_dt to avoid checkpatch line length warnings.
 - Removed dev_err() calls in parse_dt as requested by Greg KH.
 - Fixed checkpatch style warning (missing blank line after declarations).
 - Ensured newline at the end of the file.

Changes in v5:
 - Added missing newline at the end of axis-fifo.c file in Patch 2.

Changes in v4:
 - Removed extra blank lines in commit messages.
 - Added "rx" and "tx" prefixes to error messages in Patch 2 (now removed in v6).
 - Explicitly listed changes in individual patches.

Changes in v3:
 - Split into two patches as requested.

Gustavo Piaz da Silva (2):
  staging: axis-fifo: align fifo depth types with OF API
  staging: axis-fifo: refactor device tree parsing

 drivers/staging/axis-fifo/axis-fifo.c | 82 +++++++++------------------
 1 file changed, 28 insertions(+), 54 deletions(-)

-- 
2.52.0

[PATCH v6 1/2] staging: axis-fifo: align fifo depth types with OF API

[Gustavo Piaz da Silva]

Change has_rx_fifo and has_tx_fifo types from int to u32 in struct
axis_fifo.

The of_property_read_u32() function expects a u32 pointer. While the
current code might work, aligning the types avoids potential mixed-type
issues and clarifies the data structure.

Signed-off-by: Gustavo Piaz da Silva <gustavopiazdasilva2102@gmail.com>
---
Changes in v6:
 - No changes.

Changes in v5:
 - No changes.

Changes in v4:
 - Removed extra blank lines in the commit message.

Changes in v3:
 - Split the original v2 patch into two separate patches.
 
 drivers/staging/axis-fifo/axis-fifo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 509d620d6ce7..7bc7bf191250 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -121,8 +121,8 @@ struct axis_fifo {
 
 	unsigned int rx_fifo_depth; /* max words in the receive fifo */
 	unsigned int tx_fifo_depth; /* max words in the transmit fifo */
-	int has_rx_fifo; /* whether the IP has the rx fifo enabled */
-	int has_tx_fifo; /* whether the IP has the tx fifo enabled */
+	u32 has_rx_fifo; /* whether the IP has the rx fifo enabled */
+	u32 has_tx_fifo; /* whether the IP has the tx fifo enabled */
 
 	wait_queue_head_t read_queue; /* wait queue for asynchronos read */
 	struct mutex read_lock; /* lock for reading */
-- 
2.52.0

[PATCH v6 2/2] staging: axis-fifo: refactor device tree parsing

[Gustavo Piaz da Silva]

Refactor the device tree parsing logic in axis_fifo_probe() to reduce
verbosity and simplify error handling.

Remove the verbose error logging and goto logic. Instead, check
of_property_read_u32() return values directly and propagate error codes
immediately. This aligns the driver with modern kernel standards by
removing unnecessary error messages during probe.

Signed-off-by: Gustavo Piaz da Silva <gustavopiazdasilva2102@gmail.com>
---
Changes in v6:
 - Removed axis_fifo_get_u32() helper function entirely.
 - Kept 'node' local variable to avoid checkpatch line length warnings.
 - Removed dev_err() calls in parse_dt as requested by Greg KH.
 - Fixed checkpatch style warning (missing blank line after declarations).
 - Ensured newline at end of file.

Changes in v5:
 - Added missing newline at the end of the file.

Changes in v4:
 - Removed extra blank lines in the commit message.
 - Added "rx" and "tx" prefixes to error messages (now removed in v6).

Changes in v3:
 - Split the original v2 patch into two separate patches.
 
 drivers/staging/axis-fifo/axis-fifo.c | 78 +++++++++------------------
 1 file changed, 26 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7bc7bf191250..c482611bbde7 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -484,66 +484,40 @@ static void axis_fifo_debugfs_init(struct axis_fifo *fifo)
 
 static int axis_fifo_parse_dt(struct axis_fifo *fifo)
 {
-	int ret;
-	unsigned int value;
 	struct device_node *node = fifo->dt_device->of_node;
+	int ret;
+	u32 width;
 
-	ret = of_property_read_u32(node, "xlnx,axi-str-rxd-tdata-width",
-				   &value);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,axi-str-rxd-tdata-width property\n");
-		goto end;
-	} else if (value != 32) {
-		dev_err(fifo->dt_device, "xlnx,axi-str-rxd-tdata-width only supports 32 bits\n");
-		ret = -EIO;
-		goto end;
-	}
+	ret = of_property_read_u32(node, "xlnx,axi-str-rxd-tdata-width", &width);
 
-	ret = of_property_read_u32(node, "xlnx,axi-str-txd-tdata-width",
-				   &value);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,axi-str-txd-tdata-width property\n");
-		goto end;
-	} else if (value != 32) {
-		dev_err(fifo->dt_device, "xlnx,axi-str-txd-tdata-width only supports 32 bits\n");
-		ret = -EIO;
-		goto end;
-	}
+	if (ret)
+		return ret;
+	if (width != 32)
+		return -EINVAL;
 
-	ret = of_property_read_u32(node, "xlnx,rx-fifo-depth",
-				   &fifo->rx_fifo_depth);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,rx-fifo-depth property\n");
-		ret = -EIO;
-		goto end;
-	}
+	ret = of_property_read_u32(node, "xlnx,axi-str-txd-tdata-width", &width);
+	if (ret)
+		return ret;
+	if (width != 32)
+		return -EINVAL;
 
-	ret = of_property_read_u32(node, "xlnx,tx-fifo-depth",
-				   &fifo->tx_fifo_depth);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,tx-fifo-depth property\n");
-		ret = -EIO;
-		goto end;
-	}
+	ret = of_property_read_u32(node, "xlnx,rx-fifo-depth", &fifo->rx_fifo_depth);
+	if (ret)
+		return ret;
 
-	ret = of_property_read_u32(node, "xlnx,use-rx-data",
-				   &fifo->has_rx_fifo);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,use-rx-data property\n");
-		ret = -EIO;
-		goto end;
-	}
+	ret = of_property_read_u32(node, "xlnx,tx-fifo-depth", &fifo->tx_fifo_depth);
+	if (ret)
+		return ret;
 
-	ret = of_property_read_u32(node, "xlnx,use-tx-data",
-				   &fifo->has_tx_fifo);
-	if (ret) {
-		dev_err(fifo->dt_device, "missing xlnx,use-tx-data property\n");
-		ret = -EIO;
-		goto end;
-	}
+	ret = of_property_read_u32(node, "xlnx,use-rx-data", &fifo->has_rx_fifo);
+	if (ret)
+		return ret;
 
-end:
-	return ret;
+	ret = of_property_read_u32(node, "xlnx,use-tx-data", &fifo->has_tx_fifo);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static int axis_fifo_probe(struct platform_device *pdev)
-- 
2.52.0

Re: [PATCH v6 2/2] staging: axis-fifo: refactor device tree parsing

[Dan Carpenter]

On Mon, Feb 02, 2026 at 11:20:47AM -0300, Gustavo Piaz da Silva wrote:
>  static int axis_fifo_parse_dt(struct axis_fifo *fifo)
>  {
> -	int ret;
> -	unsigned int value;
>  	struct device_node *node = fifo->dt_device->of_node;
> +	int ret;
> +	u32 width;
>  
> -	ret = of_property_read_u32(node, "xlnx,axi-str-rxd-tdata-width",
> -				   &value);

In the original commit then there were a bunch of changes and it kind of
was fine to rename "value" to "width" because you were also changing the
commit message and it was nice that the error message matched the code.
But in this patch you're just deleting error messages and changing the
goto end to a direct return.

At that point, I would prefer to see a really minimal diff.  No function
renames.  No changes to the line breaks.

> -	if (ret) {
> -		dev_err(fifo->dt_device, "missing xlnx,axi-str-rxd-tdata-width property\n");
> -		goto end;
> -	} else if (value != 32) {
> -		dev_err(fifo->dt_device, "xlnx,axi-str-rxd-tdata-width only supports 32 bits\n");
> -		ret = -EIO;
> -		goto end;
> -	}
> +	ret = of_property_read_u32(node, "xlnx,axi-str-rxd-tdata-width", &width);
>  
> + 	if (ret)

Delete the blank between the function call and the error checking.

[ snip ]

> -	ret = of_property_read_u32(node, "xlnx,rx-fifo-depth",
> -				   &fifo->rx_fifo_depth);
> +	ret = of_property_read_u32(node, "xlnx,rx-fifo-depth", &fifo->rx_fifo_depth);
> +	if (ret)
> +		return ret;

This is an example of changing the white space.  It's unrelated.
Whey you were changing to use a helper then it was related, but once
we delete the helper it's not.

regards,
dan carpenter

Re: [PATCH v6 0/2] staging: axis-fifo: refactor and cleanup

[Greg KH]

On Mon, Feb 02, 2026 at 11:20:45AM -0300, Gustavo Piaz da Silva wrote:
> This series cleans up the axis-fifo driver probe function logic.
> 
> Patch 1 aligns the struct types with the OF API.
> Patch 2 refactors the parsing logic to be simpler and less verbose.

Please slow down.  One patch series every few days at the most,
otherwise you are not allowing yourself to actually get review comments
from people, and you just move your changes further down the queue.

There's no rush here, relax, take a few days off, and think about what
you are wanting to do here before sending a new version.

thanks,

greg k-h

Re: [PATCH v6 1/2] staging: axis-fifo: align fifo depth types with OF API

[Greg KH]

On Mon, Feb 02, 2026 at 11:20:46AM -0300, Gustavo Piaz da Silva wrote:
> Change has_rx_fifo and has_tx_fifo types from int to u32 in struct
> axis_fifo.

I can not parse this sentance, sorry.

> The of_property_read_u32() function expects a u32 pointer. While the
> current code might work, aligning the types avoids potential mixed-type
> issues and clarifies the data structure.

Please note that the current code does work, so to imply it doesn't
might not be the best thing.

thanks,

greg k-h