[PATCH] panic: add support to update panic_timeout via DT

[PATCH] panic: add support to update panic_timeout via DT

[Jeetesh Burman]

Add support to set 'panic_timeout' value via dtb to have
control from device tree. This makes sense when you have
multiple platforms support from same defconfig and
differentiated only through device tree blob.
The patch will check for panic time out value coming
from dtb if it exists, otherwise continues with
defconfig provided option.

Signed-off-by: Jeetesh Burman <jburman@nvidia.com>
---
 kernel/panic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/panic.c b/kernel/panic.c
index bdd18af..2541b96 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -27,6 +27,7 @@
 #include <linux/console.h>
 #include <linux/bug.h>
 #include <linux/ratelimit.h>
+#include <linux/of.h>
 
 #define PANIC_TIMER_STEP 100
 #define PANIC_BLINK_SPD 18
@@ -482,6 +483,12 @@ static u64 oops_id;
 
 static int init_oops_id(void)
 {
+	struct device_node *np;
+
+	np = of_find_node_by_name(NULL, "panic_timeout");
+	if (np)
+		of_property_read_u32(np, "panic-timeout-value", &panic_timeout);
+
 	if (!oops_id)
 		get_random_bytes(&oops_id, sizeof(oops_id));
 	else
-- 
2.1.4

Re: [PATCH] panic: add support to update panic_timeout via DT

[Sergey Senozhatsky]

On (10/27/17 09:27), Jeetesh Burman wrote:
> Add support to set 'panic_timeout' value via dtb to have
> control from device tree. This makes sense when you have
> multiple platforms support from same defconfig and
> differentiated only through device tree blob.
> The patch will check for panic time out value coming
> from dtb if it exists, otherwise continues with
> defconfig provided option.

*my personal opinion* - I'm not super happy to see more and more
of_find_node_by_name()'s in random places. but that's just IMHO.

>  static int init_oops_id(void)
>  {
> +	struct device_node *np;
> +
> +	np = of_find_node_by_name(NULL, "panic_timeout");
> +	if (np)
> +		of_property_read_u32(np, "panic-timeout-value", &panic_timeout);
> +
>  	if (!oops_id)
>  		get_random_bytes(&oops_id, sizeof(oops_id));
>  	else

why init_oops_id()? you are going to re-read it every time you call
print_oops_end_marker(), and the system can be configured not to
panic on oops. put it to oops_setup()? or, at least, move it to
`if (!oops_id)' branch, maybe?

	-ss

RE: [PATCH] panic: add support to update panic_timeout via DT

[Jeetesh Burman]

Hi Sergey,

Thanks for review, 'PATCH V2' uploaded

> why init_oops_id()? you are going to re-read it every time you call
> print_oops_end_marker(), and the system can be configured not to panic on
> oops. put it to oops_setup()? or, at least, move it to `if (!oops_id)' branch,
> maybe?
[Jeetesh] It looks to make changes in oops_setup(), need to pass "oops" parameter in kernel-command
line, so I made changes under 'if (!oops_id)', please review "PATCH V2"

Regards,
Jeetesh

> -----Original Message-----
> From: Sergey Senozhatsky [mailto:sergey.senozhatsky@gmail.com]
> Sent: Friday, October 27, 2017 1:30 PM
> To: Jeetesh Burman <jburman@nvidia.com>
> Cc: sergey.senozhatsky@gmail.com; keescook@chromium.org; linux-
> kernel@vger.kernel.org; Bibek Basu <bbasu@nvidia.com>
> Subject: Re: [PATCH] panic: add support to update panic_timeout via DT
> 
> On (10/27/17 09:27), Jeetesh Burman wrote:
> > Add support to set 'panic_timeout' value via dtb to have control from
> > device tree. This makes sense when you have multiple platforms support
> > from same defconfig and differentiated only through device tree blob.
> > The patch will check for panic time out value coming from dtb if it
> > exists, otherwise continues with defconfig provided option.
> 
> *my personal opinion* - I'm not super happy to see more and more
> of_find_node_by_name()'s in random places. but that's just IMHO.
> 
> >  static int init_oops_id(void)
> >  {
> > +	struct device_node *np;
> > +
> > +	np = of_find_node_by_name(NULL, "panic_timeout");
> > +	if (np)
> > +		of_property_read_u32(np, "panic-timeout-value",
> &panic_timeout);
> > +
> >  	if (!oops_id)
> >  		get_random_bytes(&oops_id, sizeof(oops_id));
> >  	else
> 
> why init_oops_id()? you are going to re-read it every time you call
> print_oops_end_marker(), and the system can be configured not to panic on
> oops. put it to oops_setup()? or, at least, move it to `if (!oops_id)' branch,
> maybe?
> 
> 	-ss