diff mbox series

wifi: wilc1000: Add proper error handling for remaining CMD52

Message ID 20241018194244.280322-1-marex@denx.de
State New
Headers show
Series wifi: wilc1000: Add proper error handling for remaining CMD52 | expand

Commit Message

Marek Vasut Oct. 18, 2024, 7:41 p.m. UTC
A few of the CMD52 calls did not have any error handling, add it.
This prevents odd errors like "Unexpected interrupt (1) int=nnn"
when the CMD52 fails just above in the IRQ handler and the CMD52
error code is ignored by the driver. Fill the error handling in.
Sort the variables in those affected functions while at it. Note
that the error code itself is already printed in wilc_sdio_cmd52().

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
Cc: Ajay Singh <ajay.kathat@microchip.com>
Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 .../net/wireless/microchip/wilc1000/sdio.c    | 27 ++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

Comments

Alexis Lothoré (eBPF Foundation) Oct. 22, 2024, 9:11 a.m. UTC | #1
Hi Marek,

On 10/18/24 21:41, Marek Vasut wrote:
> A few of the CMD52 calls did not have any error handling, add it.
> This prevents odd errors like "Unexpected interrupt (1) int=nnn"
> when the CMD52 fails just above in the IRQ handler and the CMD52
> error code is ignored by the driver. Fill the error handling in.
> Sort the variables in those affected functions while at it. Note
> that the error code itself is already printed in wilc_sdio_cmd52().
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
> Cc: Ajay Singh <ajay.kathat@microchip.com>
> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
>  .../net/wireless/microchip/wilc1000/sdio.c    | 27 ++++++++++++++-----
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
> index 5262c8846c13d..170470d1c2092 100644
> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> @@ -769,8 +769,10 @@ static int wilc_sdio_init(struct wilc *wilc, bool resume)
>  
>  static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
>  {
> -	u32 tmp;
> +	struct sdio_func *func = dev_to_sdio_func(wilc->dev);
>  	struct sdio_cmd52 cmd;
> +	u32 tmp;
> +	int ret;
>  
>  	/**
>  	 *      Read DMA count in words
> @@ -780,12 +782,20 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
>  	cmd.raw = 0;
>  	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG;
>  	cmd.data = 0;
> -	wilc_sdio_cmd52(wilc, &cmd);
> +	ret = wilc_sdio_cmd52(wilc, &cmd);
> +	if (ret) {
> +		dev_err(&func->dev, "Fail cmd 52, set DATA_SZ[0] register...\n");

I don't get the log message, why "set" DATA_SZ[0] ? This helper is rather trying
to read it. Same for the other logs added below

> +		return ret;
> +	}
>  	tmp = cmd.data;
>  
>  	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1;
>  	cmd.data = 0;
> -	wilc_sdio_cmd52(wilc, &cmd);
> +	ret = wilc_sdio_cmd52(wilc, &cmd);
> +	if (ret) {
> +		dev_err(&func->dev, "Fail cmd 52, set DATA_SZ[1] register...\n");
> +		return ret;
> +	}
>  	tmp |= (cmd.data << 8);
>  
>  	*size = tmp;
> @@ -796,9 +806,10 @@ static int wilc_sdio_read_int(struct wilc *wilc, u32 *int_status)
>  {
>  	struct sdio_func *func = dev_to_sdio_func(wilc->dev);
>  	struct wilc_sdio *sdio_priv = wilc->bus_data;
> -	u32 tmp;
> -	u8 irq_flags;
>  	struct sdio_cmd52 cmd;
> +	u8 irq_flags;
> +	u32 tmp;
> +	int ret;
>  
>  	wilc_sdio_read_size(wilc, &tmp);
>  
> @@ -817,7 +828,11 @@ static int wilc_sdio_read_int(struct wilc *wilc, u32 *int_status)
>  	cmd.raw = 0;
>  	cmd.read_write = 0;
>  	cmd.data = 0;
> -	wilc_sdio_cmd52(wilc, &cmd);
> +	ret = wilc_sdio_cmd52(wilc, &cmd);
> +	if (ret) {
> +		dev_err(&func->dev, "Fail cmd 52, set IRQ_FLAG register...\n");
> +		return ret;
> +	}
>  	irq_flags = cmd.data;
>  
>  	if (sdio_priv->irq_gpio)
diff mbox series

Patch

diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 5262c8846c13d..170470d1c2092 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -769,8 +769,10 @@  static int wilc_sdio_init(struct wilc *wilc, bool resume)
 
 static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
 {
-	u32 tmp;
+	struct sdio_func *func = dev_to_sdio_func(wilc->dev);
 	struct sdio_cmd52 cmd;
+	u32 tmp;
+	int ret;
 
 	/**
 	 *      Read DMA count in words
@@ -780,12 +782,20 @@  static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
 	cmd.raw = 0;
 	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG;
 	cmd.data = 0;
-	wilc_sdio_cmd52(wilc, &cmd);
+	ret = wilc_sdio_cmd52(wilc, &cmd);
+	if (ret) {
+		dev_err(&func->dev, "Fail cmd 52, set DATA_SZ[0] register...\n");
+		return ret;
+	}
 	tmp = cmd.data;
 
 	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1;
 	cmd.data = 0;
-	wilc_sdio_cmd52(wilc, &cmd);
+	ret = wilc_sdio_cmd52(wilc, &cmd);
+	if (ret) {
+		dev_err(&func->dev, "Fail cmd 52, set DATA_SZ[1] register...\n");
+		return ret;
+	}
 	tmp |= (cmd.data << 8);
 
 	*size = tmp;
@@ -796,9 +806,10 @@  static int wilc_sdio_read_int(struct wilc *wilc, u32 *int_status)
 {
 	struct sdio_func *func = dev_to_sdio_func(wilc->dev);
 	struct wilc_sdio *sdio_priv = wilc->bus_data;
-	u32 tmp;
-	u8 irq_flags;
 	struct sdio_cmd52 cmd;
+	u8 irq_flags;
+	u32 tmp;
+	int ret;
 
 	wilc_sdio_read_size(wilc, &tmp);
 
@@ -817,7 +828,11 @@  static int wilc_sdio_read_int(struct wilc *wilc, u32 *int_status)
 	cmd.raw = 0;
 	cmd.read_write = 0;
 	cmd.data = 0;
-	wilc_sdio_cmd52(wilc, &cmd);
+	ret = wilc_sdio_cmd52(wilc, &cmd);
+	if (ret) {
+		dev_err(&func->dev, "Fail cmd 52, set IRQ_FLAG register...\n");
+		return ret;
+	}
 	irq_flags = cmd.data;
 
 	if (sdio_priv->irq_gpio)