From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frank Rowand Subject: Re: [PATCH] pstore/ram: Improve backward compatibility with older Chromebooks Date: Tue, 7 May 2019 15:23:38 -0700 Message-ID: <90974ece-ab3a-7f5a-7d71-bd8a0d1d5aec@gmail.com> References: <20190503174730.245762-1-dianders@chromium.org> <30361ae7-36a6-0858-77ec-40493ef44b98@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Doug Anderson Cc: Kees Cook , Rob Herring , Anton Vorontsov , "open list:ARM/Rockchip SoC..." , Julius Werner , Guenter Roeck , Matthias Kaehlcke , Brian Norris , Colin Cross , Tony Luck , LKML List-Id: linux-rockchip.vger.kernel.org Hi Doug, On 5/7/19 3:19 PM, Doug Anderson wrote: > Hi, > > On Tue, May 7, 2019 at 3:17 PM Frank Rowand wrote: >> >> On 5/6/19 4:58 PM, Doug Anderson wrote: >>> Hi, >>> >>> On Mon, May 6, 2019 at 2:10 PM Kees Cook wrote: >>>> >>>> From: Douglas Anderson >>>> Date: Fri, May 3, 2019 at 10:48 AM >>>> To: Kees Cook, Anton Vorontsov >>>> Cc: , , >>>> , , , >>>> Douglas Anderson, Colin Cross, Tony Luck, >>>> >>>> >>>>> When you try to run an upstream kernel on an old ARM-based Chromebook >>>>> you'll find that console-ramoops doesn't work. >>>>> >>>>> Old ARM-based Chromebooks, before >>>>> ("ramoops: support upstream {console,pmsg,ftrace}-size properties") >>>>> used to create a "ramoops" node at the top level that looked like: >>>>> >>>>> / { >>>>> ramoops { >>>>> compatible = "ramoops"; >>>>> reg = <...>; >>>>> record-size = <...>; >>>>> dump-oops; >>>>> }; >>>>> }; >>>>> >>>>> ...and these Chromebooks assumed that the downstream kernel would make >>>>> console_size / pmsg_size match the record size. The above ramoops >>>>> node was added by the firmware so it's not easy to make any changes. >>>>> >>>>> Let's match the expected behavior, but only for those using the old >>>>> backward-compatible way of working where ramoops is right under the >>>>> root node. >>>>> >>>>> NOTE: if there are some out-of-tree devices that had ramoops at the >>>>> top level, left everything but the record size as 0, and somehow >>>>> doesn't want this behavior, we can try to add more conditions here. >>>>> >>>>> Signed-off-by: Douglas Anderson >>>> >>>> I like this; thanks! Rob is this okay by you? I just want to >>>> double-check since it's part of the DT parsing logic. >>>> >>>> I'll pick it up and add a Cc: stable. >>> >>> Hold off a second--I may need to send out a v2 but out of time for the >>> day. I think I need a #include file to fix errors on x86: >>> >>>> implicit declaration of function 'of_node_is_root' [-Werror,-Wimplicit-function-declaration >> >> Instead of checking "of_node_is_root(parent_node)" the patch could check >> for parent_node not "/reserved-memory". Then the x86 error would not >> occur. >> >> The check I am suggesting is not as precise, but it should be good enough >> for this case, correct? > > Sure, there are a million different ways to slice it. If you prefer > that instead of adding a dummy of_node_is_root() I'm happy to do that. Yes, I would prefer to avoid adding a dummy of_node_is_root() if the alternative is reasonable (and if I understand, you are saying the alternative is reasonable). Thanks, Frank