diff mbox series

[V2,01/16] firmware: raspberrypi: Improve timeout warning

Message ID 20240728114200.75559-2-wahrenst@gmx.net
State New
Headers show
Series ARM: bcm2835: Implement initial S2Idle for Raspberry Pi | expand

Commit Message

Stefan Wahren July 28, 2024, 11:41 a.m. UTC 
Recent work on raspberry-power driver showed that even the
stacktrace on firmware property timeout doesn't provide
enough information. So add the first tag name to the warning
to be in line with a status error.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/firmware/raspberrypi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.34.1

Comments

Stefan Wahren Aug. 12, 2024, 8:51 p.m. UTC | #1 
Hi Florian,

Am 28.07.24 um 13:41 schrieb Stefan Wahren:
> Recent work on raspberry-power driver showed that even the
> stacktrace on firmware property timeout doesn't provide
> enough information. So add the first tag name to the warning
> to be in line with a status error.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Are there any concerns, because i assumed this patch would go via your tree?

Best regards
> ---
>   drivers/firmware/raspberrypi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index ac34876a97f8..18cc34987108 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -62,7 +62,6 @@ rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
>   			ret = 0;
>   		} else {
>   			ret = -ETIMEDOUT;
> -			WARN_ONCE(1, "Firmware transaction timeout");
>   		}
>   	} else {
>   		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> @@ -125,6 +124,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
>   		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
>   			buf[2], buf[1]);
>   		ret = -EINVAL;
> +	} else if (ret == -ETIMEDOUT) {
> +		WARN_ONCE(1, "Firmware transaction 0x%08x timeout", buf[2]);
>   	}
>
>   	dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);
> --
> 2.34.1
>
Florian Fainelli Aug. 13, 2024, 8:21 p.m. UTC | #2 
From: Florian Fainelli <f.fainelli@gmail.com>

On Sun, 28 Jul 2024 13:41:45 +0200, Stefan Wahren <wahrenst@gmx.net> wrote:
> Recent work on raspberry-power driver showed that even the
> stacktrace on firmware property timeout doesn't provide
> enough information. So add the first tag name to the warning
> to be in line with a status error.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/drivers/next, thanks!
--
Florian
diff mbox series

Patch

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ac34876a97f8..18cc34987108 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -62,7 +62,6 @@  rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
 			ret = 0;
 		} else {
 			ret = -ETIMEDOUT;
-			WARN_ONCE(1, "Firmware transaction timeout");
 		}
 	} else {
 		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
@@ -125,6 +124,8 @@  int rpi_firmware_property_list(struct rpi_firmware *fw,
 		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
 			buf[2], buf[1]);
 		ret = -EINVAL;
+	} else if (ret == -ETIMEDOUT) {
+		WARN_ONCE(1, "Firmware transaction 0x%08x timeout", buf[2]);
 	}

 	dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);