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@xxxxxxx>
> ---
> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> Cc: Adham Abozaeid <adham.abozaeid@xxxxxxxxxxxxx>
> Cc: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx>
> Cc: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>
> Cc: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Kalle Valo <kvalo@xxxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>
> Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx
> ---
> .../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)
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
