diff mbox series

[1/8] Spi: sprd-adi: Getting panic reason before reboot

Message ID 20230815023426.15076-1-jiansheng.wu@unisoc.com
State New
Headers show
Series [1/8] Spi: sprd-adi: Getting panic reason before reboot | expand

Commit Message

Jiansheng Wu Aug. 15, 2023, 2:34 a.m. UTC
Registered adi_panic_event to panic_notifier_list, that is used to
get panic reason and judge restart causes before system reboot.
It's can improve reboot reasons from panic.

Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
---
 drivers/spi/spi-sprd-adi.c | 46 ++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

Comments

Thomas Weißschuh Aug. 15, 2023, 8:22 a.m. UTC | #1
On 2023-08-15 10:34:19+0800, Jiansheng Wu wrote:
> Registered adi_panic_event to panic_notifier_list, that is used to
> get panic reason and judge restart causes before system reboot.
> It's can improve reboot reasons from panic.
> 
> Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
> ---
>  drivers/spi/spi-sprd-adi.c | 46 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
> index 22e39c4c12c4..dd00d63a3cd0 100644
> --- a/drivers/spi/spi-sprd-adi.c
> +++ b/drivers/spi/spi-sprd-adi.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
>  #include <linux/spi/spi.h>
> @@ -128,6 +129,8 @@
>  #define WDG_LOAD_MASK			GENMASK(15, 0)
>  #define WDG_UNLOCK_KEY			0xe551
>  
> +#define PANIC_REASON_LEN_MAX		20
> +
>  struct sprd_adi_wdg {
>  	u32 base;
>  	u32 rst_sts;
> @@ -155,6 +158,31 @@ struct sprd_adi {
>  	const struct sprd_adi_data *data;
>  };
>  
> +static char *panic_reason;
> +static int adi_panic_event(struct notifier_block *self, unsigned long val, void *reason)
> +{
> +	if (reason == NULL)
> +		return 0;
> +
> +	if (strlen(reason) < PANIC_REASON_LEN_MAX)
> +		memcpy(panic_reason, reason, strlen(reason));
> +	else
> +		memcpy(panic_reason, reason, PANIC_REASON_LEN_MAX);

This will truncate the trailing '\0'.
The string logic below will be invalid on a non-null-terminated byte
array.
strscpy() would avoid the issue.

> +
> +	return 0;
> +}
> +
> +static struct notifier_block adi_panic_event_nb = {
> +	.notifier_call  = adi_panic_event,
> +	.priority       = INT_MAX,
> +};
> +
> +static int adi_get_panic_reason_init(void)
> +{
> +	atomic_notifier_chain_register(&panic_notifier_list, &adi_panic_event_nb);
> +	return 0;
> +}
> +
>  static int sprd_adi_check_addr(struct sprd_adi *sadi, u32 reg)
>  {
>  	if (reg >= sadi->data->slave_addr_size) {
> @@ -378,9 +406,15 @@ static int sprd_adi_restart(struct notifier_block *this, unsigned long mode,
>  					     restart_handler);
>  	u32 val, reboot_mode = 0;
>  
> -	if (!cmd)
> -		reboot_mode = HWRST_STATUS_NORMAL;
> -	else if (!strncmp(cmd, "recovery", 8))
> +	if (!cmd) {
> +		if (strlen(panic_reason)) {
> +			reboot_mode = HWRST_STATUS_PANIC;
> +			if (strstr(panic_reason, "tospanic"))
> +				reboot_mode = HWRST_STATUS_SECURITY;
> +		} else {
> +			reboot_mode = HWRST_STATUS_NORMAL;
> +		}
> +	} else if (!strncmp(cmd, "recovery", 8))
>  		reboot_mode = HWRST_STATUS_RECOVERY;
>  	else if (!strncmp(cmd, "alarm", 5))
>  		reboot_mode = HWRST_STATUS_ALARM;
> @@ -520,6 +554,10 @@ static int sprd_adi_probe(struct platform_device *pdev)
>  	u16 num_chipselect;
>  	int ret;
>  
> +	panic_reason = devm_kzalloc(&pdev->dev, (sizeof(char))*PANIC_REASON_LEN_MAX, GFP_KERNEL);

sizeof(char) is guaranteed to be 1 by the C standard.

It seems weird to devm_alloc() something that will be stored in a static
buffer. If multiple devices are bound to the driver the results will be
weird.
Also if the device is unbound the notifier won't be unregistered and
panic_mode will point to freed memory.

> +	if (!panic_reason)
> +		return -ENOMEM;
> +
>  	if (!np) {
>  		dev_err(&pdev->dev, "can not find the adi bus node\n");
>  		return -ENODEV;
> @@ -573,7 +611,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
>  	}
>  
>  	sprd_adi_hw_init(sadi);
> -
> +	adi_get_panic_reason_init();

Why drop the empty line?

>  	if (sadi->data->wdg_rst)
>  		sadi->data->wdg_rst(sadi);
>  
> -- 
> 2.17.1
>
Mark Brown Aug. 15, 2023, 1:21 p.m. UTC | #2
On Tue, Aug 15, 2023 at 10:34:19AM +0800, Jiansheng Wu wrote:

> +	if (strlen(reason) < PANIC_REASON_LEN_MAX)
> +		memcpy(panic_reason, reason, strlen(reason));
> +	else
> +		memcpy(panic_reason, reason, PANIC_REASON_LEN_MAX);

There's length limited strcpy() variants for this.
diff mbox series

Patch

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 22e39c4c12c4..dd00d63a3cd0 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -12,6 +12,7 @@ 
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/panic_notifier.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/spi/spi.h>
@@ -128,6 +129,8 @@ 
 #define WDG_LOAD_MASK			GENMASK(15, 0)
 #define WDG_UNLOCK_KEY			0xe551
 
+#define PANIC_REASON_LEN_MAX		20
+
 struct sprd_adi_wdg {
 	u32 base;
 	u32 rst_sts;
@@ -155,6 +158,31 @@  struct sprd_adi {
 	const struct sprd_adi_data *data;
 };
 
+static char *panic_reason;
+static int adi_panic_event(struct notifier_block *self, unsigned long val, void *reason)
+{
+	if (reason == NULL)
+		return 0;
+
+	if (strlen(reason) < PANIC_REASON_LEN_MAX)
+		memcpy(panic_reason, reason, strlen(reason));
+	else
+		memcpy(panic_reason, reason, PANIC_REASON_LEN_MAX);
+
+	return 0;
+}
+
+static struct notifier_block adi_panic_event_nb = {
+	.notifier_call  = adi_panic_event,
+	.priority       = INT_MAX,
+};
+
+static int adi_get_panic_reason_init(void)
+{
+	atomic_notifier_chain_register(&panic_notifier_list, &adi_panic_event_nb);
+	return 0;
+}
+
 static int sprd_adi_check_addr(struct sprd_adi *sadi, u32 reg)
 {
 	if (reg >= sadi->data->slave_addr_size) {
@@ -378,9 +406,15 @@  static int sprd_adi_restart(struct notifier_block *this, unsigned long mode,
 					     restart_handler);
 	u32 val, reboot_mode = 0;
 
-	if (!cmd)
-		reboot_mode = HWRST_STATUS_NORMAL;
-	else if (!strncmp(cmd, "recovery", 8))
+	if (!cmd) {
+		if (strlen(panic_reason)) {
+			reboot_mode = HWRST_STATUS_PANIC;
+			if (strstr(panic_reason, "tospanic"))
+				reboot_mode = HWRST_STATUS_SECURITY;
+		} else {
+			reboot_mode = HWRST_STATUS_NORMAL;
+		}
+	} else if (!strncmp(cmd, "recovery", 8))
 		reboot_mode = HWRST_STATUS_RECOVERY;
 	else if (!strncmp(cmd, "alarm", 5))
 		reboot_mode = HWRST_STATUS_ALARM;
@@ -520,6 +554,10 @@  static int sprd_adi_probe(struct platform_device *pdev)
 	u16 num_chipselect;
 	int ret;
 
+	panic_reason = devm_kzalloc(&pdev->dev, (sizeof(char))*PANIC_REASON_LEN_MAX, GFP_KERNEL);
+	if (!panic_reason)
+		return -ENOMEM;
+
 	if (!np) {
 		dev_err(&pdev->dev, "can not find the adi bus node\n");
 		return -ENODEV;
@@ -573,7 +611,7 @@  static int sprd_adi_probe(struct platform_device *pdev)
 	}
 
 	sprd_adi_hw_init(sadi);
-
+	adi_get_panic_reason_init();
 	if (sadi->data->wdg_rst)
 		sadi->data->wdg_rst(sadi);