Messages in this thread | | | From | Jeetesh Burman <> | Subject | RE: [PATCH] panic: add support to update panic_timeout via DT | Date | Fri, 27 Oct 2017 09:39:12 +0000 |
| |
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
| |