From: Rob Herring <robh@kernel.org>
To: John Stultz <john.stultz@linaro.org>
Cc: "Andy Yan" <andy.yan@rock-chips.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Moritz Fischer" <moritz.fischer@ettus.com>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Kumar Gala" <galak@codeaurora.org>,
"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"Heiko Stübner" <heiko@sntech.de>,
"Sebastian Reichel" <sre@kernel.org>,
"Dmitry Eremin-Solenikov" <dbaryshkov@gmail.com>,
"Alexandre Belloni" <alexandre.belloni@free-electrons.com>,
"Jun Nie" <jun.nie@linaro.org>, "Paweł Moll" <pawel.moll@arm.com>,
"Will Deacon" <will.deacon@arm.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
"Caesar Wang" <wxt@rock-chips.com>,
devicetree@vger.kernel.org,
"Linux PM list" <linux-pm@vger.kernel.org>,
"Russell King - ARM Linux" <linux@arm.linux.org.uk>,
mbrugger@suse.com,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
lkml <linux-kernel@vger.kernel.org>,
"Richard Weinberger" <richard@nod.at>,
"David Woodhouse" <dwmw2@infradead.org>,
"Mark Rutland" <mark.rutland@arm.com>
Subject: Re: [PATCH v3 1/4] dt-bindings: power: reset: add document for reboot-mode driver
Date: Thu, 11 Feb 2016 11:04:57 -0600 [thread overview]
Message-ID: <20160211170457.GA19851@rob-hp-laptop> (raw)
In-Reply-To: <CALAqxLUnbFHjTuJXYXSruV3W0JJ09Fxz=KBe69tZDTd_3_+rxw@mail.gmail.com>
On Thu, Feb 04, 2016 at 09:03:44PM -0800, John Stultz wrote:
> On Thu, Feb 4, 2016 at 8:35 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Feb 04, 2016 at 03:46:15PM -0800, John Stultz wrote:
> >> On Thu, Feb 4, 2016 at 3:08 PM, Rob Herring <robh@kernel.org> wrote:
> >> > On Tue, Feb 02, 2016 at 05:59:11PM +0800, Andy Yan wrote:
> >> >> +Example:
> >> >> + reboot-mode {
> >> >> + mode-normal = <BOOT_NORMAL>;
> >> >> + mode-recovery = <BOOT_RECOVERY>;
> >> >> + mode-fastboot = <BOOT_FASTBOOT>;
> >> >
> >> > I tend to agree with John on calling this mode-bootloader.
> >> >
> >> > OTOH, fastboot is more specific about what the mode is. The name in DT
> >> > and the userspace name don't necessarily have to be the same.
> >>
> >> Wait. This is a bit confusing. The utility of adding a property name
> >> and using that name be the reboot command parsed for made sense
> >> (compared to earlier versions which had command strings) as it made
> >> the dts more terse. But it sounds here like you're suggesting we
> >> should have some logic in the driver that translates "reboot fastboot"
> >> to mode-bootloader or vice versa.
> >
> > I said early on the DT names and kernel-userspace names should not
> > necessarily be linked. They can be, but we shouldn't require that.
>
> Sigh. Ok. It seemed it was due to earlier comments (maybe from others,
> but I thought it was you), that we moved from specifying a command
> string, to using the label. But if you think the label name and the
> commands shouldn't be linked, it seems like we should re-introduce
> that. No?
>
> Unless your thinking we need some sort of static in-kernel mapping of
> commands to label names? But that just seems painfully indirect for
> little gain ("Its obvious! For that mode, you use this term here, and
> that different term over there!").
Tying it to a Linux ABI makes the binding Linux specific. I don't have a
problem that the strings happen to be the same, but we should have some
understanding that they may not be and allow for that.
> > My concern with mode-bootloader is what if you can boot into multiple
> > bootloader modes. Say USB mass storage is one option. "bootloader" is
> > not real specific.
>
> True. But as I think we agreed below, "bootloader" and "recovery" are
> basically defacto standards, and I think it would be a bad idea to try
> to declare all the existing android tooling and docs wrong just
> because the command is vague, technically.
Okay, as long as they are clearly documented what they mean.
> >> >> + mode-loader = <BOOT_LOADER>;
> >> >
> >> > This one needs a better name. Maybe it should be 'rockchip,mode-loader'
> >> > as it is vendor specific. Either way, loader is vague. Perhaps
> >> > rockchip,mode-bl-download?
> >>
> >> Hrm. So how what reboot command do you expect to trigger that?
> >
> > Whatever your OS has defined to map to that.
> >
> > We could just decide the kernel will strip <vendor> and 'mode-' and
> > match commands against what remains.
>
> That part sounds sane, although I do think having vendor prefixes are
> reasonable for actual commands as well.
Well, you could still have "rockchip,mode-rockchip-bl-download"...
We can bikeshed that when get there.
The other way random custom modes could be done is just allow the raw
value to be passed from userspace converting the string to a number.
Then we have no abstraction rather than half way abstracting it.
> >> I think one of the difficult things here is that there's no real
> >> standards for all bios/bootloader modes. So they are somewhat
> >> firmware/bootloader/device specific, and thus we need something that
> >> is flexible enough to allow lots of different modes to be easily
> >> specified. That said, this does expose a userspace interface (though
> >> one could argue kernel ABI doesn't cross reboots :) so we should try
> >> to have some consistency so the same userspace can work on various
> >> devices.
> >
> > There is: UEFI. Boot mode efivars are standard. But then they are pretty
> > much PC oriented though. It is more which device to boot off of, but
> > there is network boot or boot to bios setup.
>
> Well, there's a partial standard there. I'm told for android on x86,
> there is no UEFI standard way to communicate rebooting to fastboot or
> recovery. Every device does its own device specific driver.
So much for standards. However, while these specific modes have not been
standardized, there is a set of standard modes and these could have been
added to the existing mechanism. So there at least exists some model to
draw inspiration from.
Rob
next prev parent reply other threads:[~2016-02-11 17:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-02 9:56 [PATCH v3 0/4] add reboot mode driver Andy Yan
2016-02-02 9:59 ` [PATCH v3 1/4] dt-bindings: power: reset: add document for reboot-mode driver Andy Yan
2016-02-02 18:29 ` John Stultz
2016-02-04 23:08 ` Rob Herring
2016-02-04 23:46 ` John Stultz
2016-02-05 4:35 ` Rob Herring
2016-02-05 5:03 ` John Stultz
2016-02-11 17:04 ` Rob Herring [this message]
2016-02-15 9:43 ` Andy Yan
2016-02-02 10:02 ` [PATCH v3 2/4] power: reset: add reboot mode driver Andy Yan
2016-02-02 18:16 ` Moritz Fischer
2016-02-03 11:54 ` kbuild test robot
2016-02-02 10:10 ` [PATCH v3 3/4] ARM: dts: rockchip: add syscon-reboot-mode DT node Andy Yan
2016-02-02 10:13 ` [PATCH v3 4/4] ARM64: " Andy Yan
2016-02-03 2:07 ` Shawn Lin
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=20160211170457.GA19851@rob-hp-laptop \
--to=robh@kernel.org \
--cc=alexandre.belloni@free-electrons.com \
--cc=andy.yan@rock-chips.com \
--cc=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=dbaryshkov@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=dwmw2@infradead.org \
--cc=galak@codeaurora.org \
--cc=heiko@sntech.de \
--cc=ijc+devicetree@hellion.org.uk \
--cc=john.stultz@linaro.org \
--cc=jun.nie@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux@arm.linux.org.uk \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=matthias.bgg@gmail.com \
--cc=mbrugger@suse.com \
--cc=moritz.fischer@ettus.com \
--cc=pawel.moll@arm.com \
--cc=richard@nod.at \
--cc=sre@kernel.org \
--cc=will.deacon@arm.com \
--cc=wxt@rock-chips.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).