diff mbox series

[3/5] spi: rockchip: Don't check for failed get_fifo_len()

Message ID ce2e7f90e62b15adc2bed1f53122ad39c3a9b5ac.1727337732.git.dsimic@manjaro.org (mailing list archive)
State New
Headers show
Series Improve error handling in Rockchip SPI drivers | expand

Commit Message

Dragan Simic Sept. 26, 2024, 8:38 a.m. UTC
Since commit 13a96935e6f6 ("spi: rockchip: Support 64-location deep FIFOs"),
function get_fifo_len() can no longer return zero, so delete the redundant
check for zero in function rockchip_spi_probe().

Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 drivers/spi/spi-rockchip.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Heiko Stuebner Sept. 26, 2024, 8:55 a.m. UTC | #1
Am Donnerstag, 26. September 2024, 10:38:14 CEST schrieb Dragan Simic:
> Since commit 13a96935e6f6 ("spi: rockchip: Support 64-location deep FIFOs"),
> function get_fifo_len() can no longer return zero, so delete the redundant
> check for zero in function rockchip_spi_probe().
> 
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>

Didn't this topic come up in another recent patch too?

Anyway, having looked up the what the current get_fifo_len does,
the 0 case should never happen, as you describe, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Dragan Simic Sept. 26, 2024, 9:10 a.m. UTC | #2
Hello Heiko,

On 2024-09-26 10:55, Heiko Stuebner wrote:
> Am Donnerstag, 26. September 2024, 10:38:14 CEST schrieb Dragan Simic:
>> Since commit 13a96935e6f6 ("spi: rockchip: Support 64-location deep 
>> FIFOs"),
>> function get_fifo_len() can no longer return zero, so delete the 
>> redundant
>> check for zero in function rockchip_spi_probe().
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> 
> Didn't this topic come up in another recent patch too?

Hmm, perhaps, but I'm unaware of that patch?  Maybe I missed it.

> Anyway, having looked up the what the current get_fifo_len does,
> the 0 case should never happen, as you describe, so
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Thanks.
Mark Brown Sept. 26, 2024, 9:17 a.m. UTC | #3
On Thu, Sep 26, 2024 at 10:55:01AM +0200, Heiko Stuebner wrote:
> Am Donnerstag, 26. September 2024, 10:38:14 CEST schrieb Dragan Simic:
> > Since commit 13a96935e6f6 ("spi: rockchip: Support 64-location deep FIFOs"),
> > function get_fifo_len() can no longer return zero, so delete the redundant
> > check for zero in function rockchip_spi_probe().

> Didn't this topic come up in another recent patch too?

> Anyway, having looked up the what the current get_fifo_len does,
> the 0 case should never happen, as you describe, so

One of the people doing random cleanups posted the same patch which I
pushed back on since probe() isn't a hot path and it means if
get_fifo_len() changes again it could silently break things.
Dragan Simic Sept. 26, 2024, 10:14 a.m. UTC | #4
Hello Mark,

On 2024-09-26 11:17, Mark Brown wrote:
> On Thu, Sep 26, 2024 at 10:55:01AM +0200, Heiko Stuebner wrote:
>> Am Donnerstag, 26. September 2024, 10:38:14 CEST schrieb Dragan Simic:
>> > Since commit 13a96935e6f6 ("spi: rockchip: Support 64-location deep FIFOs"),
>> > function get_fifo_len() can no longer return zero, so delete the redundant
>> > check for zero in function rockchip_spi_probe().
> 
>> Didn't this topic come up in another recent patch too?
> 
>> Anyway, having looked up the what the current get_fifo_len does,
>> the 0 case should never happen, as you describe, so
> 
> One of the people doing random cleanups posted the same patch which I
> pushed back on since probe() isn't a hot path and it means if
> get_fifo_len() changes again it could silently break things.

Thanks for the clarification, it makes sense to keep the check for
future proofing.  I'll drop this patch in the v2 of this series.
diff mbox series

Patch

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 81f2a966c124..28879fed03f8 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -816,11 +816,6 @@  static int rockchip_spi_probe(struct platform_device *pdev)
 	}
 
 	rs->fifo_len = get_fifo_len(rs);
-	if (!rs->fifo_len) {
-		dev_err(&pdev->dev, "Failed to get fifo length\n");
-		ret = -EINVAL;
-		goto err_put_ctlr;
-	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, ROCKCHIP_AUTOSUSPEND_TIMEOUT);
 	pm_runtime_use_autosuspend(&pdev->dev);