From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Julius Werner <jwerner@chromium.org>
Cc: linux-kernel@vger.kernel.org,
Thierry Escande <thierry.escande@collabora.com>,
Dmitry Torokhov <dtor@chromium.org>,
Aaron Durbin <adurbin@chromium.org>
Subject: Re: [PATCH] firmware: google: memconsole: Adapt to new coreboot ring buffer format
Date: Fri, 28 Apr 2017 09:14:43 +0200 [thread overview]
Message-ID: <20170428071443.GC27859@kroah.com> (raw)
In-Reply-To: <20170427205917.18392-1-jwerner@chromium.org>
On Thu, Apr 27, 2017 at 01:59:17PM -0700, Julius Werner wrote:
> This patch needs to be applied on top of the other memconsole patches
> currently queued up in char-misc-next (ending in 049a59db34e).
>
> -------------------------8<--------------------------------------------
That needs to go below the --- line, not above it.
>
> The upstream coreboot implementation of memconsole was enhanced from a
> single-boot console to a persistent ring buffer
> (https://review.coreboot.org/#/c/18301). This patch changes the kernel
> memconsole driver to be able to read the new format.
>
> In addition, it is desirable for the driver to always return the current
> state of the console, so that it may be used by runtime firmware to log
> further messages after the kernel is already initialized. The existing
> implementation would already re-read the buffer on every access, but
> only measured the total size of the console once during initialization.
> Thus, the generic console interface was redesigned to allow the
> implementation full control over how to process an access, and the
> coreboot implementation will re-read the console size every time.
>
> Finally, add a new /sys/firmware/rawlog node and change the existing
> /sys/firmware/log to sanitize non-printable characters from the output.
> With the new persistent console it becomes more likely that bytes might
> get slightly corrupted (due to DRAM degradation during a reboot), and
> it's usually undesirable to get stray control characters in the console
> dump because a bit in a letter flipped.
When you say things like "in addition" and "finally", that implies that
the code needs to be broken up into multiple patches. Rememeber, each
patch only should ever do 1 thing.
Please do that here and make a patch series.
thanks,
greg k-h
next prev parent reply other threads:[~2017-04-28 7:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-27 20:59 [PATCH] firmware: google: memconsole: Adapt to new coreboot ring buffer format Julius Werner
2017-04-28 7:14 ` Greg Kroah-Hartman [this message]
[not found] ` <CAODwPW9bJ=qX7L1uDBpnpV3RKHOxAurhF1=te2g54GGSJTAtBQ@mail.gmail.com>
2017-04-29 5:33 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170428071443.GC27859@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=adurbin@chromium.org \
--cc=dtor@chromium.org \
--cc=jwerner@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=thierry.escande@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox