From: Heiko =?ISO-8859-1?Q?St=FCbner?= <heiko@sntech.de>
To: Andy Yan <andy.yan@rock-chips.com>
Cc: Rob Herring <robh@kernel.org>, john.stultz@linaro.org, arnd@arndb.de,
        pawel.moll@arm.com, mark.rutland@arm.com,
        ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        catalin.marinas@arm.com, olof@lixom.net,
        alexandre.belloni@free-electrons.com, dbaryshkov@gmail.com,
        sre@kernel.org, jun.nie@linaro.org, edubezval@gmail.com,
        f.fainelli@gmail.com, will.deacon@arm.com,
        linux-rockchip@lists.infradead.org, wxt@rock-chips.com,
        matthias.bgg@gmail.com, linux@arm.linux.org.uk,
        lorenzo.pieralisi@arm.com, moritz.fischer@ettus.com, richard@nod.at,
        dwmw2@infradead.org, jwerner@google.com
Subject: Re: [PATCH v5 1/4] dt-bindings: power: reset: add document for reboot-mode driver
Date: Fri, 18 Mar 2016 10:07:14 +0100
Message-ID: <3696428.Va8GqGlrHG@diego>
User-Agent: KMail/4.14.10 (Linux/4.2.0-1-amd64; KDE/4.14.14; x86_64; ; )
In-Reply-To: <56EBBFB1.6000808@rock-chips.com>
References: <1457439716-8084-1-git-send-email-andy.yan@rock-chips.com> <20160317155212.GA19171@rob-hp-laptop> <56EBBFB1.6000808@rock-chips.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8BIT
Content-Type: text/plain; charset="utf-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2877
Lines: 83

Hi Andy,

Am Freitag, 18. März 2016, 16:43:29 schrieb Andy Yan:
> On 2016年03月17日 23:52, Rob Herring wrote:
> > On Tue, Mar 08, 2016 at 08:23:05PM +0800, Andy Yan wrote:
> >> add device tree bindings document for reboot-mode driver
> >> 
> >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

[...]

> >> diff --git
> >> a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> >> b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> >> new file mode 100644
> >> index 0000000..d03cb52
> >> --- /dev/null
> >> +++
> >> b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> >> @@ -0,0 +1,34 @@
> >> +SYSCON reboot mode driver
> >> +
> >> +This driver get reboot mode magic value form reboot-mode driver
> >> +and stores it in a SYSCON mapped register. Then the bootloader
> >> +can read it and take different action according to the magic
> >> +value stored.
> >> +
> >> +This DT node should be represented as a sub-node of a "syscon",
> >> "simple-mfd" +node.
> >> +
> >> +Required properties:
> >> +- compatible: should be "syscon-reboot-mode"
> >> +- offset: offset in the register map for the storage register (in bytes)
> >> +
> >> +Optional properity:
> >> +- mask: the mask bits of the mode magic value, default set to 0xffffffff
> >> if missing.> 
> > I can't see how this is used. I would drop it for now.
> > 
> > With that change:
> > 
> > Acked-by: Rob Herring <robh@kernel.org>
> 
> The mask is suggested by Moritz Fischer in V2[0]. And I found Tegra did
> indeed only use some bits of one register to store the reboot mode[1].
> 
> @Heiko: It seems that this patch series can land now, but the dts parts
> belong to  two different branch(arm32 and arm64) . And these
> two parts both depends on one head file rockchip_boot-mode.h, So I hope
> some suggestions from you how to deal with it.

as this is a new driver with completely new binding, the split should be easy.

Sebastian (sre@kernel.org) should take the driver + binding-txt parts and I 
can then simply queue up the 32 and 64 bit devicetree additions in my 
respective branches for 4.7.


Thanks for pushing that whole thing through, as it looks really nice now.
Heiko


>     [0] https://lkml.org/lkml/2016/1/27/225
>     [1] drivers/soc/tegra/pmc.c  (tegra_pmc_restart_notify)
> 
> >> +
> >> +The rest of the properties should follow the generic reboot-mode
> >> discription +found in reboot-mode.txt
> >> +
> >> +Example:
> >> +	pmu: pmu@20004000 {
> >> +		compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
> >> +		reg = <0x20004000 0x100>;
> >> +
> >> +		reboot-mode {
> >> +			compatible = "syscon-reboot-mode";
> >> +			offset = <0x40>;
> >> +			mode-normal = <BOOT_NORMAL>;
> >> +			mode-recovery = <BOOT_RECOVERY>;
> >> +			mode-bootloader = <BOOT_FASTBOOT>;
> >> +			mode-loader = <BOOT_BL_DOWNLOAD>;
> >> +		};
> >> +	};