[PATCH v3 0/5] Add reboot notifier driver for rockchip platform

[PATCH v3 0/5] Add reboot notifier driver for rockchip platform

[Andy Yan]

rockchip platform have a protocol to pass the kernel reboot
mode to bootloader by some special registers when system reboot.
By this way the bootloader can take different action according
to the different kernel reboot mode, for example, command
"reboot loader" will reboot the board to rockusb mode, this is
a very convenient way to get the board enter download mode.
And Android system also use this protocol to pass "recovery"、
“fastboot” reboot mode to bootloader. In upstream land, We found
tegra platform also use this mechanism.

Before this version, I have sent two Series, which can be found
at [0] [1]
[0] https://patchwork.kernel.org/patch/7140751/
[1] https://patchwork.kernel.org/patch/7153531/

Changes in v3:
 - rename a pinctrl node in rk3288-veyron, the original name will be
   used in the incoming reboot notifier driver
 - add dt binding
 - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
 - use dts pass the related register
 - add DT node

Changes in v2:
  - check cpu dt node
  - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
  - fix a align issue
  - use reboot_notifier instead of restart_handler

Andy Yan (5):
  ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset
  dt-bindings: soc: add document for rockchip reboot notifier driver
  soc: rockchip: add reboot notifier driver
  ARM: dts: rockchip: add reboot node
  ARM64: dts: rockchip: add reboot node

 .../bindings/soc/rockchip/rockchip-reboot.txt      | 18 ++++
 arch/arm/boot/dts/rk3288-veyron.dtsi               |  2 +-
 arch/arm/boot/dts/rk3288.dtsi                      |  6 ++
 arch/arm/boot/dts/rk3xxx.dtsi                      |  6 ++
 arch/arm64/boot/dts/rockchip/rk3368.dtsi           |  6 ++
 drivers/soc/rockchip/Kconfig                       |  7 ++
 drivers/soc/rockchip/Makefile                      |  1 +
 drivers/soc/rockchip/loader.h                      | 22 +++++
 drivers/soc/rockchip/reboot.c                      | 98 ++++++++++++++++++++++
 9 files changed, 165 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
 create mode 100644 drivers/soc/rockchip/loader.h
 create mode 100644 drivers/soc/rockchip/reboot.c

-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/5] ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset

[Andy Yan]

rename pinctrl node reboot to reset to match it's lable name

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v3:
 - rename a pinctrl node in rk3288-veyron, the original name will be
   used in the incoming reboot notifier driver

Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 2fa7a0d..292aaaf 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -495,7 +495,7 @@
 		};
 	};
 
-	reboot {
+	reset {
 		ap_warm_reset_h: ap-warm-reset-h {
 			rockchip,pins = <RK_GPIO0 13 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
-- 
1.9.1

Re: [PATCH v3 1/5] ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset

[Sergei Shtylyov]

Hello.

On 11/18/2015 12:50 PM, Andy Yan wrote:

> rename pinctrl node reboot to reset to match it's lable name

    Label? BTW, I'm not seeing it on this node...

> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>
> ---
>
> Changes in v3:
>   - rename a pinctrl node in rk3288-veyron, the original name will be
>     used in the incoming reboot notifier driver
>
> Changes in v2: None
>
>   arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
> index 2fa7a0d..292aaaf 100644
> --- a/arch/arm/boot/dts/rk3288-veyron.dtsi
> +++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
> @@ -495,7 +495,7 @@
>   		};
>   	};
>
> -	reboot {
> +	reset {
>   		ap_warm_reset_h: ap-warm-reset-h {
>   			rockchip,pins = <RK_GPIO0 13 RK_FUNC_GPIO &pcfg_pull_none>;
>   		};

MBR, Sergei

Re: [PATCH v3 1/5] ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset

[Andy Yan]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

Hi Sergei:


On 2015年11月18日 22:18, Sergei Shtylyov wrote:
> Hello.
>
> On 11/18/2015 12:50 PM, Andy Yan wrote:
>
>> rename pinctrl node reboot to reset to match it's lable name
>
>    Label? BTW, I'm not seeing it on this node...
      Sorry, I mean "ap_warm_reset_h: ap-warm-reset-h" here
      Maybe it should be called subnode
>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>>   - rename a pinctrl node in rk3288-veyron, the original name will be
>>     used in the incoming reboot notifier driver
>>
>> Changes in v2: None
>>
>>   arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi 
>> b/arch/arm/boot/dts/rk3288-veyron.dtsi
>> index 2fa7a0d..292aaaf 100644
>> --- a/arch/arm/boot/dts/rk3288-veyron.dtsi
>> +++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
>> @@ -495,7 +495,7 @@
>>           };
>>       };
>>
>> -    reboot {
>> +    reset {
>>           ap_warm_reset_h: ap-warm-reset-h {
>>               rockchip,pins = <RK_GPIO0 13 RK_FUNC_GPIO 
>> &pcfg_pull_none>;
>>           };
>
> MBR, Sergei
>
>
>
>


[-- Attachment #2: Type: text/html, Size: 2486 bytes --]

[PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

Add devicetree binding document for rockchip reboot nofifier driver

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v3:
 - add dt binding

Changes in v2: None

 .../bindings/soc/rockchip/rockchip-reboot.txt          | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt

diff --git a/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
new file mode 100644
index 0000000..6f69c8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
@@ -0,0 +1,18 @@
+Rockchip reboot notifier driver
+
+This driver get reboot mode arguments from userspace
+and stores it in special register. Then the bootloader
+will read it and take different action according the
+argument stored.
+
+Required properties:
+- compatible: should be "rockchip,reboot"
+- regmap: this is phandle to the register map node
+- offset: offset in the register map for the storage register (in bytes)
+
+Examples:
+	reboot {
+	   compatible = "rockchip,reboot";
+	   regmap = <&pmu>;
+	   offset = <0x94>;
+	};
-- 
1.9.1

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Rob Herring]

On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> Add devicetree binding document for rockchip reboot nofifier driver

Just reading the subject this is way too specific to the Linux driver 
needs rather than a h/w description. Please don't create fake DT nodes 
just to bind to drivers. Whatever &pmu is is probably what should have 
the DT node. Let the driver for it create child devices if you need 
that.

Rob
> 
> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
> Changes in v3:
>  - add dt binding
> 
> Changes in v2: None
> 
>  .../bindings/soc/rockchip/rockchip-reboot.txt          | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
> new file mode 100644
> index 0000000..6f69c8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
> @@ -0,0 +1,18 @@
> +Rockchip reboot notifier driver
> +
> +This driver get reboot mode arguments from userspace
> +and stores it in special register. Then the bootloader
> +will read it and take different action according the
> +argument stored.
> +
> +Required properties:
> +- compatible: should be "rockchip,reboot"
> +- regmap: this is phandle to the register map node
> +- offset: offset in the register map for the storage register (in bytes)
> +
> +Examples:
> +	reboot {
> +	   compatible = "rockchip,reboot";
> +	   regmap = <&pmu>;
> +	   offset = <0x94>;
> +	};
> -- 
> 1.9.1
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

Hi Rob:

On 2015年11月19日 06:59, Rob Herring wrote:
> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
>> Add devicetree binding document for rockchip reboot nofifier driver
> Just reading the subject this is way too specific to the Linux driver
> needs rather than a h/w description. Please don't create fake DT nodes
> just to bind to drivers. Whatever &pmu is is probably what should have
> the DT node. Let the driver for it create child devices if you need
> that.

     This is note a fake DT nodes, we really need it to tell the driver
      which register to use to store the reboot mode. Because rockchip
      use different register file to store the reboot mode on different
      platform, on rk3066,rk3188, rk3288,it use  one of the PMU 
register, on
      the incoming RK3036, it use one of the GRF register, and it use 
one  of
      the PMUGRF register for arm64 platform rk3368. On the other hand, the
      PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
      by other DT nodes by phandle. So maybe let it as a separate DT 
node here
      is better.

> Rob
>> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> ---
>>
>> Changes in v3:
>>   - add dt binding
>>
>> Changes in v2: None
>>
>>   .../bindings/soc/rockchip/rockchip-reboot.txt          | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
>> new file mode 100644
>> index 0000000..6f69c8d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt
>> @@ -0,0 +1,18 @@
>> +Rockchip reboot notifier driver
>> +
>> +This driver get reboot mode arguments from userspace
>> +and stores it in special register. Then the bootloader
>> +will read it and take different action according the
>> +argument stored.
>> +
>> +Required properties:
>> +- compatible: should be "rockchip,reboot"
>> +- regmap: this is phandle to the register map node
>> +- offset: offset in the register map for the storage register (in bytes)
>> +
>> +Examples:
>> +	reboot {
>> +	   compatible = "rockchip,reboot";
>> +	   regmap = <&pmu>;
>> +	   offset = <0x94>;
>> +	};
>> -- 
>> 1.9.1
>>
>>
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Heiko Stuebner]

Hi Andy,

Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
> Hi Rob:
> 
> On 2015年11月19日 06:59, Rob Herring wrote:
> > On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> >> Add devicetree binding document for rockchip reboot nofifier driver
> > Just reading the subject this is way too specific to the Linux driver
> > needs rather than a h/w description. Please don't create fake DT nodes
> > just to bind to drivers. Whatever &pmu is is probably what should have
> > the DT node. Let the driver for it create child devices if you need
> > that.
> 
>      This is note a fake DT nodes, we really need it to tell the driver
>       which register to use to store the reboot mode. Because rockchip
>       use different register file to store the reboot mode on different
>       platform, on rk3066,rk3188, rk3288,it use  one of the PMU 
> register, on
>       the incoming RK3036, it use one of the GRF register, and it use 
> one  of
>       the PMUGRF register for arm64 platform rk3368. On the other hand, the
>       PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
>       by other DT nodes by phandle. So maybe let it as a separate DT 
> node here
>       is better.

or alternatively we could do something similar to what the bl-switcher 
cupfreq-driver does. Take a look at

drivers/cpufreq/arm_big_little.c
drivers/clk/clk-mb86s7x.c

We already have the core restart-handler code in the clock-tree, so could 
maybe simply do the
	platform_device_register_simple("rockchip-reboot", -1, NULL, 0);
in that common code?

Though I'm not yet sure how to get the platform-data. I guess one option would 
be to do things like the 3288 suspend code does (arch/arm/mach-rockchip/pm.c 
at the bottom), by having the per-soc-data in the driver and then matching 
against the pmu. Because the pmu is not part of the clock controller binding 
(and probably also shouldn't be).


Heiko


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

Hi Heiko:

On 2015年11月19日 12:35, Heiko Stuebner wrote:
> Hi Andy,
>
> Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
>> Hi Rob:
>>
>> On 2015年11月19日 06:59, Rob Herring wrote:
>>> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
>>>> Add devicetree binding document for rockchip reboot nofifier driver
>>> Just reading the subject this is way too specific to the Linux driver
>>> needs rather than a h/w description. Please don't create fake DT nodes
>>> just to bind to drivers. Whatever &pmu is is probably what should have
>>> the DT node. Let the driver for it create child devices if you need
>>> that.
>>       This is note a fake DT nodes, we really need it to tell the driver
>>        which register to use to store the reboot mode. Because rockchip
>>        use different register file to store the reboot mode on different
>>        platform, on rk3066,rk3188, rk3288,it use  one of the PMU
>> register, on
>>        the incoming RK3036, it use one of the GRF register, and it use
>> one  of
>>        the PMUGRF register for arm64 platform rk3368. On the other hand, the
>>        PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
>>        by other DT nodes by phandle. So maybe let it as a separate DT
>> node here
>>        is better.
> or alternatively we could do something similar to what the bl-switcher
> cupfreq-driver does. Take a look at
>
> drivers/cpufreq/arm_big_little.c
> drivers/clk/clk-mb86s7x.c
>
> We already have the core restart-handler code in the clock-tree, so could
> maybe simply do the
> 	platform_device_register_simple("rockchip-reboot", -1, NULL, 0);
> in that common code?
>
> Though I'm not yet sure how to get the platform-data. I guess one option would
> be to do things like the 3288 suspend code does (arch/arm/mach-rockchip/pm.c
> at the bottom), by having the per-soc-data in the driver and then matching
> against the pmu. Because the pmu is not part of the clock controller binding
> (and probably also shouldn't be).
>
>
> Heiko
>
>
>
>
>
    Thanks for your suggestion.
     I have read the code you list above, if we implement the reboot 
notifier
     driver like this, the driver need to add much more code to find the 
platform
     data(like arch/arm/mach-rockhcip/pm.c), what's more, if we have a 
new soc
     in the future and the soc use a different register here, we need 
modify the
     driver to add a new platform data again, this will bring additional 
work.

     Use the DT node pass the register will make the driver code simple 
and clear.
     Is there any hurt to put this information in the DT?

     Andy

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Rob Herring]

On Thu, Nov 19, 2015 at 7:16 PM, Andy Yan <andy.yan@rock-chips.com> wrote:
> On 2015年11月19日 12:35, Heiko Stuebner wrote:
>> Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
>>> On 2015年11月19日 06:59, Rob Herring wrote:
>>>> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
>>>>>
>>>>> Add devicetree binding document for rockchip reboot nofifier driver
>>>>
>>>> Just reading the subject this is way too specific to the Linux driver
>>>> needs rather than a h/w description. Please don't create fake DT nodes
>>>> just to bind to drivers. Whatever &pmu is is probably what should have
>>>> the DT node. Let the driver for it create child devices if you need
>>>> that.
>>>
>>>       This is note a fake DT nodes, we really need it to tell the driver
>>>        which register to use to store the reboot mode. Because rockchip
>>>        use different register file to store the reboot mode on different
>>>        platform, on rk3066,rk3188, rk3288,it use  one of the PMU
>>> register, on
>>>        the incoming RK3036, it use one of the GRF register, and it use
>>> one  of
>>>        the PMUGRF register for arm64 platform rk3368. On the other hand,
>>> the
>>>        PMU/GRF/PMUGRF register file are mapped as "syscon", then
>>> referenced
>>>        by other DT nodes by phandle. So maybe let it as a separate DT
>>> node here
>>>        is better.
>>
>> or alternatively we could do something similar to what the bl-switcher
>> cupfreq-driver does. Take a look at
>>
>> drivers/cpufreq/arm_big_little.c
>> drivers/clk/clk-mb86s7x.c
>>
>> We already have the core restart-handler code in the clock-tree, so could
>> maybe simply do the
>>         platform_device_register_simple("rockchip-reboot", -1, NULL, 0);
>> in that common code?
>>
>> Though I'm not yet sure how to get the platform-data. I guess one option
>> would
>> be to do things like the 3288 suspend code does
>> (arch/arm/mach-rockchip/pm.c
>> at the bottom), by having the per-soc-data in the driver and then matching
>> against the pmu. Because the pmu is not part of the clock controller
>> binding
>> (and probably also shouldn't be).
>>
>    Thanks for your suggestion.
>     I have read the code you list above, if we implement the reboot notifier
>     driver like this, the driver need to add much more code to find the
> platform
>     data(like arch/arm/mach-rockhcip/pm.c), what's more, if we have a new
> soc
>     in the future and the soc use a different register here, we need modify
> the
>     driver to add a new platform data again, this will bring additional
> work.
>
>     Use the DT node pass the register will make the driver code simple and
> clear.
>     Is there any hurt to put this information in the DT?

Add the data you need to the PMU node. Then the PMU driver can get it
and pass to the child driver.

Rob

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

[-- Attachment #1: Type: text/plain, Size: 4997 bytes --]

Hi Rob：

2015-11-20 9:58 GMT+08:00 Rob Herring <robh@kernel.org>:

> On Thu, Nov 19, 2015 at 7:16 PM, Andy Yan <andy.yan@rock-chips.com> wrote:
> > On 2015年11月19日 12:35, Heiko Stuebner wrote:
> >> Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
> >>> On 2015年11月19日 06:59, Rob Herring wrote:
> >>>> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> >>>>>
> >>>>> Add devicetree binding document for rockchip reboot nofifier driver
> >>>>
> >>>> Just reading the subject this is way too specific to the Linux driver
> >>>> needs rather than a h/w description. Please don't create fake DT nodes
> >>>> just to bind to drivers. Whatever &pmu is is probably what should have
> >>>> the DT node. Let the driver for it create child devices if you need
> >>>> that.
> >>>
> >>>       This is note a fake DT nodes, we really need it to tell the
> driver
> >>>        which register to use to store the reboot mode. Because rockchip
> >>>        use different register file to store the reboot mode on
> different
> >>>        platform, on rk3066,rk3188, rk3288,it use  one of the PMU
> >>> register, on
> >>>        the incoming RK3036, it use one of the GRF register, and it use
> >>> one  of
> >>>        the PMUGRF register for arm64 platform rk3368. On the other
> hand,
> >>> the
> >>>        PMU/GRF/PMUGRF register file are mapped as "syscon", then
> >>> referenced
> >>>        by other DT nodes by phandle. So maybe let it as a separate DT
> >>> node here
> >>>        is better.
> >>
> >> or alternatively we could do something similar to what the bl-switcher
> >> cupfreq-driver does. Take a look at
> >>
> >> drivers/cpufreq/arm_big_little.c
> >> drivers/clk/clk-mb86s7x.c
> >>
> >> We already have the core restart-handler code in the clock-tree, so
> could
> >> maybe simply do the
> >>         platform_device_register_simple("rockchip-reboot", -1, NULL, 0);
> >> in that common code?
> >>
> >> Though I'm not yet sure how to get the platform-data. I guess one option
> >> would
> >> be to do things like the 3288 suspend code does
> >> (arch/arm/mach-rockchip/pm.c
> >> at the bottom), by having the per-soc-data in the driver and then
> matching
> >> against the pmu. Because the pmu is not part of the clock controller
> >> binding
> >> (and probably also shouldn't be).
> >>
> >    Thanks for your suggestion.
> >     I have read the code you list above, if we implement the reboot
> notifier
> >     driver like this, the driver need to add much more code to find the
> > platform
> >     data(like arch/arm/mach-rockhcip/pm.c), what's more, if we have a new
> > soc
> >     in the future and the soc use a different register here, we need
> modify
> > the
> >     driver to add a new platform data again, this will bring additional
> > work.
> >
> >     Use the DT node pass the register will make the driver code simple
> and
> > clear.
> >     Is there any hurt to put this information in the DT?
>
> Add the data you need to the PMU node. Then the PMU driver can get it
> and pass to the child driver.
>
> Rob
> --
>
>
    Do you mean I should implement the DT node like this？

   diff --git a/arch/arm/boot/dts/rk3xxx.dtsi
b/arch/arm/boot/dts/rk3xxx.dtsi
index 7b14d7a..1735d09 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -103,12 +103,6 @@
                };
        };

-       reboot {
-               compatible = "rockchip,reboot";
-               rockchip,regmap = <&pmu>;
-               offset = <0x40>;
-       };
-
        xin24m: oscillator {
                compatible = "fixed-clock";
                clock-frequency = <24000000>;
@@ -249,7 +243,11 @@

        pmu: pmu@20004000 {
                compatible = "rockchip,rk3066-pmu", "syscon";
-               reg = <0x20004000 0x100>;
+              reg = <0x20004000 0x100>;
+              reboot {
+                       compatible = "rockchip,reboot";
+                       offset = <0x40>;
+               };
        };

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index cd02229..8a9837a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -202,12 +202,6 @@
                method = "smc";
        };

-       reboot {
-               compatible = "rockchip,reboot";
-               rockchip,regmap = <&pmugrf>;
-               offset = <0x200>;
-       };
-
        timer {
                compatible = "arm,armv8-timer";
                interrupts = <GIC_PPI 13
@@ -493,6 +487,10 @@
        pmugrf: syscon@ff738000 {
                compatible = "rockchip,rk3368-pmugrf", "syscon";
                reg = <0x0 0xff738000 0x0 0x1000>;
+              reboot {
+                       compatible = "rockchip,reboot";
+                       offset = <0x200>;
+               };
        };

[-- Attachment #2: Type: text/html, Size: 7162 bytes --]

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

[-- Attachment #1: Type: text/plain, Size: 5442 bytes --]

Hi Rob、Heiko：

2015-11-23 21:15 GMT+08:00 Andy Yan <andyshrk@gmail.com>:

> Hi Rob：
>
> 2015-11-20 9:58 GMT+08:00 Rob Herring <robh@kernel.org>:
>
>> On Thu, Nov 19, 2015 at 7:16 PM, Andy Yan <andy.yan@rock-chips.com>
>> wrote:
>> > On 2015年11月19日 12:35, Heiko Stuebner wrote:
>> >> Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
>> >>> On 2015年11月19日 06:59, Rob Herring wrote:
>> >>>> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
>> >>>>>
>> >>>>> Add devicetree binding document for rockchip reboot nofifier driver
>> >>>>
>> >>>> Just reading the subject this is way too specific to the Linux driver
>> >>>> needs rather than a h/w description. Please don't create fake DT
>> nodes
>> >>>> just to bind to drivers. Whatever &pmu is is probably what should
>> have
>> >>>> the DT node. Let the driver for it create child devices if you need
>> >>>> that.
>> >>>
>> >>>       This is note a fake DT nodes, we really need it to tell the
>> driver
>> >>>        which register to use to store the reboot mode. Because
>> rockchip
>> >>>        use different register file to store the reboot mode on
>> different
>> >>>        platform, on rk3066,rk3188, rk3288,it use  one of the PMU
>> >>> register, on
>> >>>        the incoming RK3036, it use one of the GRF register, and it use
>> >>> one  of
>> >>>        the PMUGRF register for arm64 platform rk3368. On the other
>> hand,
>> >>> the
>> >>>        PMU/GRF/PMUGRF register file are mapped as "syscon", then
>> >>> referenced
>> >>>        by other DT nodes by phandle. So maybe let it as a separate DT
>> >>> node here
>> >>>        is better.
>> >>
>> >> or alternatively we could do something similar to what the bl-switcher
>> >> cupfreq-driver does. Take a look at
>> >>
>> >> drivers/cpufreq/arm_big_little.c
>> >> drivers/clk/clk-mb86s7x.c
>> >>
>> >> We already have the core restart-handler code in the clock-tree, so
>> could
>> >> maybe simply do the
>> >>         platform_device_register_simple("rockchip-reboot", -1, NULL,
>> 0);
>> >> in that common code?
>> >>
>> >> Though I'm not yet sure how to get the platform-data. I guess one
>> option
>> >> would
>> >> be to do things like the 3288 suspend code does
>> >> (arch/arm/mach-rockchip/pm.c
>> >> at the bottom), by having the per-soc-data in the driver and then
>> matching
>> >> against the pmu. Because the pmu is not part of the clock controller
>> >> binding
>> >> (and probably also shouldn't be).
>> >>
>> >    Thanks for your suggestion.
>> >     I have read the code you list above, if we implement the reboot
>> notifier
>> >     driver like this, the driver need to add much more code to find the
>> > platform
>> >     data(like arch/arm/mach-rockhcip/pm.c), what's more, if we have a
>> new
>> > soc
>> >     in the future and the soc use a different register here, we need
>> modify
>> > the
>> >     driver to add a new platform data again, this will bring additional
>> > work.
>> >
>> >     Use the DT node pass the register will make the driver code simple
>> and
>> > clear.
>> >     Is there any hurt to put this information in the DT?
>>
>> Add the data you need to the PMU node. Then the PMU driver can get it
>> and pass to the child driver.
>>
>> Rob
>> --
>>
>>
>     Do you mean I should implement the DT node like this？
>
>    diff --git a/arch/arm/boot/dts/rk3xxx.dtsi
> b/arch/arm/boot/dts/rk3xxx.dtsi
> index 7b14d7a..1735d09 100644
> --- a/arch/arm/boot/dts/rk3xxx.dtsi
> +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> @@ -103,12 +103,6 @@
>                 };
>         };
>
> -       reboot {
> -               compatible = "rockchip,reboot";
> -               rockchip,regmap = <&pmu>;
> -               offset = <0x40>;
> -       };
> -
>         xin24m: oscillator {
>                 compatible = "fixed-clock";
>                 clock-frequency = <24000000>;
> @@ -249,7 +243,11 @@
>
>         pmu: pmu@20004000 {
>                 compatible = "rockchip,rk3066-pmu", "syscon";
> -               reg = <0x20004000 0x100>;
> +              reg = <0x20004000 0x100>;
> +              reboot {
> +                       compatible = "rockchip,reboot";
> +                       offset = <0x40>;
> +               };
>         };
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> index cd02229..8a9837a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> @@ -202,12 +202,6 @@
>                 method = "smc";
>         };
>
> -       reboot {
> -               compatible = "rockchip,reboot";
> -               rockchip,regmap = <&pmugrf>;
> -               offset = <0x200>;
> -       };
> -
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 13
> @@ -493,6 +487,10 @@
>         pmugrf: syscon@ff738000 {
>                 compatible = "rockchip,rk3368-pmugrf", "syscon";
>                 reg = <0x0 0xff738000 0x0 0x1000>;
> +              reboot {
> +                       compatible = "rockchip,reboot";
> +                       offset = <0x200>;
> +               };
>         };
>
>

  Is there any further suggestion for this? If not, I will send the V4 with
the DT node as a subnode in PMU or PMUGRF.

Andy

[-- Attachment #2: Type: text/html, Size: 7864 bytes --]

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Heiko Stübner]

Hi Andy,

Am Dienstag, 1. Dezember 2015, 23:10:15 schrieb Andy Yan:
> 2015-11-23 21:15 GMT+08:00 Andy Yan <andyshrk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> > 2015-11-20 9:58 GMT+08:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> >> On Thu, Nov 19, 2015 at 7:16 PM, Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >> 
> >> wrote:
> >> > On 2015年11月19日 12:35, Heiko Stuebner wrote:
> >> >> Am Donnerstag, 19. November 2015, 09:17:37 schrieb Andy Yan:
> >> >>> On 2015年11月19日 06:59, Rob Herring wrote:
> >> >>>> On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> >> >>>>> Add devicetree binding document for rockchip reboot nofifier driver
> >> >>>> 
> >> >>>> Just reading the subject this is way too specific to the Linux
> >> >>>> driver
> >> >>>> needs rather than a h/w description. Please don't create fake DT
> >> 
> >> nodes
> >> 
> >> >>>> just to bind to drivers. Whatever &pmu is is probably what should
> >> 
> >> have
> >> 
> >> >>>> the DT node. Let the driver for it create child devices if you need
> >> >>>> that.
> >> >>>> 
> >> >>>       This is note a fake DT nodes, we really need it to tell the
> >> 
> >> driver
> >> 
> >> >>>        which register to use to store the reboot mode. Because
> >> 
> >> rockchip
> >> 
> >> >>>        use different register file to store the reboot mode on
> >> 
> >> different
> >> 
> >> >>>        platform, on rk3066,rk3188, rk3288,it use  one of the PMU
> >> >>> 
> >> >>> register, on
> >> >>> 
> >> >>>        the incoming RK3036, it use one of the GRF register, and it
> >> >>>        use
> >> >>> 
> >> >>> one  of
> >> >>> 
> >> >>>        the PMUGRF register for arm64 platform rk3368. On the other
> >> 
> >> hand,
> >> 
> >> >>> the
> >> >>> 
> >> >>>        PMU/GRF/PMUGRF register file are mapped as "syscon", then
> >> >>> 
> >> >>> referenced
> >> >>> 
> >> >>>        by other DT nodes by phandle. So maybe let it as a separate DT
> >> >>> 
> >> >>> node here
> >> >>> 
> >> >>>        is better.
> >> >> 
> >> >> or alternatively we could do something similar to what the bl-switcher
> >> >> cupfreq-driver does. Take a look at
> >> >> 
> >> >> drivers/cpufreq/arm_big_little.c
> >> >> drivers/clk/clk-mb86s7x.c
> >> >> 
> >> >> We already have the core restart-handler code in the clock-tree, so
> >> 
> >> could
> >> 
> >> >> maybe simply do the
> >> >> 
> >> >>         platform_device_register_simple("rockchip-reboot", -1, NULL,
> >> 
> >> 0);
> >> 
> >> >> in that common code?
> >> >> 
> >> >> Though I'm not yet sure how to get the platform-data. I guess one
> >> 
> >> option
> >> 
> >> >> would
> >> >> be to do things like the 3288 suspend code does
> >> >> (arch/arm/mach-rockchip/pm.c
> >> >> at the bottom), by having the per-soc-data in the driver and then
> >> 
> >> matching
> >> 
> >> >> against the pmu. Because the pmu is not part of the clock controller
> >> >> binding
> >> >> (and probably also shouldn't be).
> >> >> 
> >> >    Thanks for your suggestion.
> >> >    
> >> >     I have read the code you list above, if we implement the reboot
> >> 
> >> notifier
> >> 
> >> >     driver like this, the driver need to add much more code to find the
> >> > 
> >> > platform
> >> > 
> >> >     data(like arch/arm/mach-rockhcip/pm.c), what's more, if we have a
> >> 
> >> new
> >> 
> >> > soc
> >> > 
> >> >     in the future and the soc use a different register here, we need
> >> 
> >> modify
> >> 
> >> > the
> >> > 
> >> >     driver to add a new platform data again, this will bring additional
> >> > 
> >> > work.
> >> > 
> >> >     Use the DT node pass the register will make the driver code simple
> >> 
> >> and
> >> 
> >> > clear.
> >> > 
> >> >     Is there any hurt to put this information in the DT?
> >> 
> >> Add the data you need to the PMU node. Then the PMU driver can get it
> >> and pass to the child driver.
> >> 
> >> Rob
> >> --
> >> 
> >     Do you mean I should implement the DT node like this？
> >    
> >    diff --git a/arch/arm/boot/dts/rk3xxx.dtsi
> > 
> > b/arch/arm/boot/dts/rk3xxx.dtsi
> > index 7b14d7a..1735d09 100644
> > --- a/arch/arm/boot/dts/rk3xxx.dtsi
> > +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> > @@ -103,12 +103,6 @@
> > 
> >                 };
> >         
> >         };
> > 
> > -       reboot {
> > -               compatible = "rockchip,reboot";
> > -               rockchip,regmap = <&pmu>;
> > -               offset = <0x40>;
> > -       };
> > -
> > 
> >         xin24m: oscillator {
> >         
> >                 compatible = "fixed-clock";
> >                 clock-frequency = <24000000>;
> > 
> > @@ -249,7 +243,11 @@
> > 
> >         pmu: pmu@20004000 {
> >         
> >                 compatible = "rockchip,rk3066-pmu", "syscon";
> > 
> > -               reg = <0x20004000 0x100>;
> > +              reg = <0x20004000 0x100>;
> > +              reboot {
> > +                       compatible = "rockchip,reboot";
> > +                       offset = <0x40>;
> > +               };
> > 
> >         };
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> > index cd02229..8a9837a 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> > @@ -202,12 +202,6 @@
> > 
> >                 method = "smc";
> >         
> >         };
> > 
> > -       reboot {
> > -               compatible = "rockchip,reboot";
> > -               rockchip,regmap = <&pmugrf>;
> > -               offset = <0x200>;
> > -       };
> > -
> > 
> >         timer {
> >         
> >                 compatible = "arm,armv8-timer";
> >                 interrupts = <GIC_PPI 13
> > 
> > @@ -493,6 +487,10 @@
> > 
> >         pmugrf: syscon@ff738000 {
> >         
> >                 compatible = "rockchip,rk3368-pmugrf", "syscon";

    compatible = "rockchip,rk3368-pmugrf", "syscon", "simple-mfd";


> >                 reg = <0x0 0xff738000 0x0 0x1000>;
> > 
> > +              reboot {
> > +                       compatible = "rockchip,reboot";
> > +                       offset = <0x200>;
> > +               };
> > 
> >         };
> 
>   Is there any further suggestion for this? If not, I will send the V4 with
> the DT node as a subnode in PMU or PMUGRF.

I guess Rob is the authority on this, but I'm not sure on the "devicetree 
describes hardware" level.

On the one hand it is not really a hardware-device, but on the other hand it 
is a firmware-interface (like psci etc, that's already in the devicetree 
elsewhere), so I'd guess it should be ok.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]

On Thu, Nov 19, 2015 at 09:17:37AM +0800, Andy Yan wrote:
> Hi Rob:
> 
> On 2015年11月19日 06:59, Rob Herring wrote:
> >On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> >>Add devicetree binding document for rockchip reboot nofifier driver
> >Just reading the subject this is way too specific to the Linux driver
> >needs rather than a h/w description. Please don't create fake DT nodes
> >just to bind to drivers. Whatever &pmu is is probably what should have
> >the DT node. Let the driver for it create child devices if you need
> >that.
> 
>     This is note a fake DT nodes, we really need it to tell the driver
>      which register to use to store the reboot mode. Because rockchip
>      use different register file to store the reboot mode on different
>      platform, on rk3066,rk3188, rk3288,it use  one of the PMU register, on
>      the incoming RK3036, it use one of the GRF register, and it use one  of
>      the PMUGRF register for arm64 platform rk3368. On the other hand, the
>      PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
>      by other DT nodes by phandle. So maybe let it as a separate DT node
> here
>      is better.

In that case you should probably implement a reboot notifier in each of
the drivers you list and depending on the generation of the SoC. You can
easily parameterize this by matching on the compatible string.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Andy Yan]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

Hi Thierry:

2015-11-19 20:56 GMT+08:00 Thierry Reding <treding@nvidia.com>:

> On Thu, Nov 19, 2015 at 09:17:37AM +0800, Andy Yan wrote:
> > Hi Rob:
> >
> > On 2015年11月19日 06:59, Rob Herring wrote:
> > >On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> > >>Add devicetree binding document for rockchip reboot nofifier driver
> > >Just reading the subject this is way too specific to the Linux driver
> > >needs rather than a h/w description. Please don't create fake DT nodes
> > >just to bind to drivers. Whatever &pmu is is probably what should have
> > >the DT node. Let the driver for it create child devices if you need
> > >that.
> >
> >     This is note a fake DT nodes, we really need it to tell the driver
> >      which register to use to store the reboot mode. Because rockchip
> >      use different register file to store the reboot mode on different
> >      platform, on rk3066,rk3188, rk3288,it use  one of the PMU register,
> on
> >      the incoming RK3036, it use one of the GRF register, and it use
> one  of
> >      the PMUGRF register for arm64 platform rk3368. On the other hand,
> the
> >      PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
> >      by other DT nodes by phandle. So maybe let it as a separate DT node
> > here
> >      is better.
>
> In that case you should probably implement a reboot notifier in each of
> the drivers you list and depending on the generation of the SoC. You can
> easily parameterize this by matching on the compatible string.
>
> Thierry
>

 There is no rockchip specific driver for PMU/GRF/PMUGRF register file I
list above， they
 use the generic driver “syscon”

Andy

[-- Attachment #2: Type: text/html, Size: 2404 bytes --]

Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

On Thu, Nov 19, 2015 at 09:39:02PM +0800, Andy Yan wrote:
> Hi Thierry:
> 
> 2015-11-19 20:56 GMT+08:00 Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>:
> 
> > On Thu, Nov 19, 2015 at 09:17:37AM +0800, Andy Yan wrote:
> > > Hi Rob:
> > >
> > > On 2015年11月19日 06:59, Rob Herring wrote:
> > > >On Wed, Nov 18, 2015 at 05:53:30PM +0800, Andy Yan wrote:
> > > >>Add devicetree binding document for rockchip reboot nofifier driver
> > > >Just reading the subject this is way too specific to the Linux driver
> > > >needs rather than a h/w description. Please don't create fake DT nodes
> > > >just to bind to drivers. Whatever &pmu is is probably what should have
> > > >the DT node. Let the driver for it create child devices if you need
> > > >that.
> > >
> > >     This is note a fake DT nodes, we really need it to tell the driver
> > >      which register to use to store the reboot mode. Because rockchip
> > >      use different register file to store the reboot mode on different
> > >      platform, on rk3066,rk3188, rk3288,it use  one of the PMU register,
> > on
> > >      the incoming RK3036, it use one of the GRF register, and it use
> > one  of
> > >      the PMUGRF register for arm64 platform rk3368. On the other hand,
> > the
> > >      PMU/GRF/PMUGRF register file are mapped as "syscon", then referenced
> > >      by other DT nodes by phandle. So maybe let it as a separate DT node
> > > here
> > >      is better.
> >
> > In that case you should probably implement a reboot notifier in each of
> > the drivers you list and depending on the generation of the SoC. You can
> > easily parameterize this by matching on the compatible string.
> >
> > Thierry
> >
> 
>  There is no rockchip specific driver for PMU/GRF/PMUGRF register file I
> list above， they
>  use the generic driver “syscon”

Well, just go and write specific drivers, then.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Andy Yan]

rockchip platform have a protocol to pass the kernel reboot
mode to bootloader by some special registers when system reboot.
By this way the bootloader can take different action according
to the different kernel reboot mode, for example, command
"reboot loader" will reboot the board to rockusb mode, this is
a very convenient way to get the board enter download mode.

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v3:
 - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
 - use dts pass the related register

Changes in v2:
  - check cpu dt node
  - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
  - fix a align issue
  - use reboot_notifier instead of restart_handler

 drivers/soc/rockchip/Kconfig  |  7 ++++
 drivers/soc/rockchip/Makefile |  1 +
 drivers/soc/rockchip/loader.h | 22 ++++++++++
 drivers/soc/rockchip/reboot.c | 98 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 drivers/soc/rockchip/loader.h
 create mode 100644 drivers/soc/rockchip/reboot.c

diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
index 7140ff8..4edbc44 100644
--- a/drivers/soc/rockchip/Kconfig
+++ b/drivers/soc/rockchip/Kconfig
@@ -15,4 +15,11 @@ config ROCKCHIP_PM_DOMAINS
 
           If unsure, say N.
 
+config ROCKCHIP_REBOOT
+	bool "Rockchip reboot notifier driver"
+	help
+	  Say y here will enable reboot notifier support.
+	  This will get reboot mode arguments from userspace and
+	  store it in special register.
+
 endif
diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile
index 3d73d06..9817496 100644
--- a/drivers/soc/rockchip/Makefile
+++ b/drivers/soc/rockchip/Makefile
@@ -2,3 +2,4 @@
 # Rockchip Soc drivers
 #
 obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o
+obj-$(CONFIG_ROCKCHIP_REBOOT) += reboot.o
diff --git a/drivers/soc/rockchip/loader.h b/drivers/soc/rockchip/loader.h
new file mode 100644
index 0000000..bf51baa
--- /dev/null
+++ b/drivers/soc/rockchip/loader.h
@@ -0,0 +1,22 @@
+#ifndef __MACH_ROCKCHIP_LOADER_H
+#define __MACH_ROCKCHIP_LOADER_H
+
+/*high 24 bits is tag, low 8 bits is type*/
+#define SYS_LOADER_REBOOT_FLAG   0x5242C300
+
+enum {
+	BOOT_NORMAL = 0, /* normal boot */
+	BOOT_LOADER,     /* enter loader rockusb mode */
+	BOOT_MASKROM,    /* enter maskrom rockusb mode (not support now) */
+	BOOT_RECOVER,    /* enter recover */
+	BOOT_NORECOVER,  /* do not enter recover */
+	BOOT_SECONDOS,   /* boot second OS (not support now)*/
+	BOOT_WIPEDATA,   /* enter recover and wipe data. */
+	BOOT_WIPEALL,    /* enter recover and wipe all data. */
+	BOOT_CHECKIMG,   /* check firmware img with backup part*/
+	BOOT_FASTBOOT,   /* enter fast boot mode */
+	BOOT_SECUREBOOT_DISABLE,
+	BOOT_CHARGING,   /* enter charge mode */
+	BOOT_MAX         /* MAX VALID BOOT TYPE.*/
+};
+#endif
diff --git a/drivers/soc/rockchip/reboot.c b/drivers/soc/rockchip/reboot.c
new file mode 100644
index 0000000..048aeb0b
--- /dev/null
+++ b/drivers/soc/rockchip/reboot.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include "loader.h"
+
+struct rockchip_reboot {
+	struct device *dev;
+	struct regmap *map;
+	u32 offset;
+	struct notifier_block reboot_notifier;
+};
+
+static void rockchip_get_reboot_flag(const char *cmd, u32 *flag)
+{
+	*flag = SYS_LOADER_REBOOT_FLAG + BOOT_NORMAL;
+
+	if (cmd) {
+		if (!strcmp(cmd, "loader") || !strcmp(cmd, "bootloader"))
+			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_LOADER;
+		else if (!strcmp(cmd, "recovery"))
+			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_RECOVER;
+		else if (!strcmp(cmd, "charge"))
+			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_CHARGING;
+		else if (!strcmp(cmd, "fastboot"))
+			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_FASTBOOT;
+	}
+}
+
+static int rockchip_reboot_notify(struct notifier_block *this,
+				  unsigned long mode, void *cmd)
+{
+	struct rockchip_reboot *reboot;
+	u32 flag;
+
+	reboot = container_of(this, struct rockchip_reboot, reboot_notifier);
+	rockchip_get_reboot_flag(cmd, &flag);
+	regmap_write(reboot->map, reboot->offset, flag);
+
+	return NOTIFY_DONE;
+}
+
+static int __init rockchip_reboot_probe(struct platform_device *pdev)
+{
+	struct rockchip_reboot *reboot;
+	int ret;
+
+	reboot = devm_kzalloc(&pdev->dev, sizeof(*reboot), GFP_KERNEL);
+	if (!reboot)
+		return -ENOMEM;
+
+	reboot->dev = &pdev->dev;
+	reboot->map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+						      "rockchip,regmap");
+	if (IS_ERR(reboot->map))
+		return PTR_ERR(reboot->map);
+
+	if (of_property_read_u32(pdev->dev.of_node, "offset", &reboot->offset))
+		return -EINVAL;
+
+	reboot->reboot_notifier.notifier_call = rockchip_reboot_notify;
+	ret = register_reboot_notifier(&reboot->reboot_notifier);
+	if (ret)
+		dev_err(reboot->dev, "can't register reboot notifier\n");
+
+	return ret;
+}
+
+static const struct of_device_id rockchip_reboot_of_match[] = {
+	{ .compatible = "rockchip,reboot" },
+	{}
+};
+
+static struct platform_driver rockchip_reboot_driver = {
+	.probe = rockchip_reboot_probe,
+	.driver = {
+		.name = "rockchip-reboot",
+		.of_match_table = rockchip_reboot_of_match,
+	},
+};
+module_platform_driver(rockchip_reboot_driver);
+
+MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com");
+MODULE_DESCRIPTION("Rockchip platform reboot notifier driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

Hi Andy,

[auto build test WARNING on rockchip/for-next]
[also build test WARNING on v4.4-rc1 next-20151118]

url:    https://github.com/0day-ci/linux/commits/Andy-Yan/Add-reboot-notifier-driver-for-rockchip-platform/20151118-181000
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: arm64-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

>> WARNING: drivers/soc/built-in.o(.data+0x718): Section mismatch in reference from the variable rockchip_reboot_driver to the function .init.text:rockchip_reboot_probe()
   The variable rockchip_reboot_driver references
   the function __init rockchip_reboot_probe()
   If the reference is valid then annotate the
   variable with or __refdata (see linux/init.h) or name the variable:
   

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45486 bytes --]

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Arnd Bergmann]

On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> rockchip platform have a protocol to pass the kernel reboot
> mode to bootloader by some special registers when system reboot.
> By this way the bootloader can take different action according
> to the different kernel reboot mode, for example, command
> "reboot loader" will reboot the board to rockusb mode, this is
> a very convenient way to get the board enter download mode.
> 
> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Adding John Stultz to Cc

I just saw this thread pop up again, and had to think of John's recent
patch to unify this across platforms.

John, can you have a look at this driver too, and see how it fits in?
I think this is yet another variant, using an MMIO register rather than
RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
it conceptually fits in with what you had.

The driver goes through an existing syscon regmap as far as I can
tell, rather than a memory area or its own device.

	Arnd

> ---
> 
> Changes in v3:
>  - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
>  - use dts pass the related register
> 
> Changes in v2:
>   - check cpu dt node
>   - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
>   - fix a align issue
>   - use reboot_notifier instead of restart_handler
> 
>  drivers/soc/rockchip/Kconfig  |  7 ++++
>  drivers/soc/rockchip/Makefile |  1 +
>  drivers/soc/rockchip/loader.h | 22 ++++++++++
>  drivers/soc/rockchip/reboot.c | 98 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 128 insertions(+)
>  create mode 100644 drivers/soc/rockchip/loader.h
>  create mode 100644 drivers/soc/rockchip/reboot.c
> 
> diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
> index 7140ff8..4edbc44 100644
> --- a/drivers/soc/rockchip/Kconfig
> +++ b/drivers/soc/rockchip/Kconfig
> @@ -15,4 +15,11 @@ config ROCKCHIP_PM_DOMAINS
>  
>            If unsure, say N.
>  
> +config ROCKCHIP_REBOOT
> +	bool "Rockchip reboot notifier driver"
> +	help
> +	  Say y here will enable reboot notifier support.
> +	  This will get reboot mode arguments from userspace and
> +	  store it in special register.
> +
>  endif
> diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile
> index 3d73d06..9817496 100644
> --- a/drivers/soc/rockchip/Makefile
> +++ b/drivers/soc/rockchip/Makefile
> @@ -2,3 +2,4 @@
>  # Rockchip Soc drivers
>  #
>  obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_ROCKCHIP_REBOOT) += reboot.o
> diff --git a/drivers/soc/rockchip/loader.h b/drivers/soc/rockchip/loader.h
> new file mode 100644
> index 0000000..bf51baa
> --- /dev/null
> +++ b/drivers/soc/rockchip/loader.h
> @@ -0,0 +1,22 @@
> +#ifndef __MACH_ROCKCHIP_LOADER_H
> +#define __MACH_ROCKCHIP_LOADER_H
> +
> +/*high 24 bits is tag, low 8 bits is type*/
> +#define SYS_LOADER_REBOOT_FLAG   0x5242C300
> +
> +enum {
> +	BOOT_NORMAL = 0, /* normal boot */
> +	BOOT_LOADER,     /* enter loader rockusb mode */
> +	BOOT_MASKROM,    /* enter maskrom rockusb mode (not support now) */
> +	BOOT_RECOVER,    /* enter recover */
> +	BOOT_NORECOVER,  /* do not enter recover */
> +	BOOT_SECONDOS,   /* boot second OS (not support now)*/
> +	BOOT_WIPEDATA,   /* enter recover and wipe data. */
> +	BOOT_WIPEALL,    /* enter recover and wipe all data. */
> +	BOOT_CHECKIMG,   /* check firmware img with backup part*/
> +	BOOT_FASTBOOT,   /* enter fast boot mode */
> +	BOOT_SECUREBOOT_DISABLE,
> +	BOOT_CHARGING,   /* enter charge mode */
> +	BOOT_MAX         /* MAX VALID BOOT TYPE.*/
> +};
> +#endif
> diff --git a/drivers/soc/rockchip/reboot.c b/drivers/soc/rockchip/reboot.c
> new file mode 100644
> index 0000000..048aeb0b
> --- /dev/null
> +++ b/drivers/soc/rockchip/reboot.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include "loader.h"
> +
> +struct rockchip_reboot {
> +	struct device *dev;
> +	struct regmap *map;
> +	u32 offset;
> +	struct notifier_block reboot_notifier;
> +};
> +
> +static void rockchip_get_reboot_flag(const char *cmd, u32 *flag)
> +{
> +	*flag = SYS_LOADER_REBOOT_FLAG + BOOT_NORMAL;
> +
> +	if (cmd) {
> +		if (!strcmp(cmd, "loader") || !strcmp(cmd, "bootloader"))
> +			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_LOADER;
> +		else if (!strcmp(cmd, "recovery"))
> +			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_RECOVER;
> +		else if (!strcmp(cmd, "charge"))
> +			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_CHARGING;
> +		else if (!strcmp(cmd, "fastboot"))
> +			*flag = SYS_LOADER_REBOOT_FLAG + BOOT_FASTBOOT;
> +	}
> +}
> +
> +static int rockchip_reboot_notify(struct notifier_block *this,
> +				  unsigned long mode, void *cmd)
> +{
> +	struct rockchip_reboot *reboot;
> +	u32 flag;
> +
> +	reboot = container_of(this, struct rockchip_reboot, reboot_notifier);
> +	rockchip_get_reboot_flag(cmd, &flag);
> +	regmap_write(reboot->map, reboot->offset, flag);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int __init rockchip_reboot_probe(struct platform_device *pdev)
> +{
> +	struct rockchip_reboot *reboot;
> +	int ret;
> +
> +	reboot = devm_kzalloc(&pdev->dev, sizeof(*reboot), GFP_KERNEL);
> +	if (!reboot)
> +		return -ENOMEM;
> +
> +	reboot->dev = &pdev->dev;
> +	reboot->map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +						      "rockchip,regmap");
> +	if (IS_ERR(reboot->map))
> +		return PTR_ERR(reboot->map);
> +
> +	if (of_property_read_u32(pdev->dev.of_node, "offset", &reboot->offset))
> +		return -EINVAL;
> +
> +	reboot->reboot_notifier.notifier_call = rockchip_reboot_notify;
> +	ret = register_reboot_notifier(&reboot->reboot_notifier);
> +	if (ret)
> +		dev_err(reboot->dev, "can't register reboot notifier\n");
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id rockchip_reboot_of_match[] = {
> +	{ .compatible = "rockchip,reboot" },
> +	{}
> +};
> +
> +static struct platform_driver rockchip_reboot_driver = {
> +	.probe = rockchip_reboot_probe,
> +	.driver = {
> +		.name = "rockchip-reboot",
> +		.of_match_table = rockchip_reboot_of_match,
> +	},
> +};
> +module_platform_driver(rockchip_reboot_driver);
> +
> +MODULE_AUTHOR("Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org");
> +MODULE_DESCRIPTION("Rockchip platform reboot notifier driver");
> +MODULE_LICENSE("GPL");
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > rockchip platform have a protocol to pass the kernel reboot
> > mode to bootloader by some special registers when system reboot.
> > By this way the bootloader can take different action according
> > to the different kernel reboot mode, for example, command
> > "reboot loader" will reboot the board to rockusb mode, this is
> > a very convenient way to get the board enter download mode.
> > 
> > Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> Adding John Stultz to Cc
> 
> I just saw this thread pop up again, and had to think of John's recent
> patch to unify this across platforms.
> 
> John, can you have a look at this driver too, and see how it fits in?
> I think this is yet another variant, using an MMIO register rather than
> RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> it conceptually fits in with what you had.

FWIW, Tegra typically does use an MMIO register as well. See
drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
HTC does, but if it's writing somewhere in RAM it isn't using the
standard way of resetting the SoC. There's early boot ROM code which I
think evaluates the PMC_SCRATCH0 register on Tegra to determine which
mode to boot into. That's before even any firmware gets the chance of
doing anything.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Arnd Bergmann]

On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote:
> On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > > rockchip platform have a protocol to pass the kernel reboot
> > > mode to bootloader by some special registers when system reboot.
> > > By this way the bootloader can take different action according
> > > to the different kernel reboot mode, for example, command
> > > "reboot loader" will reboot the board to rockusb mode, this is
> > > a very convenient way to get the board enter download mode.
> > > 
> > > Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > 
> > Adding John Stultz to Cc
> > 
> > I just saw this thread pop up again, and had to think of John's recent
> > patch to unify this across platforms.
> > 
> > John, can you have a look at this driver too, and see how it fits in?
> > I think this is yet another variant, using an MMIO register rather than
> > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> > it conceptually fits in with what you had.
> 
> FWIW, Tegra typically does use an MMIO register as well. See
> drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
> HTC does, but if it's writing somewhere in RAM it isn't using the
> standard way of resetting the SoC. There's early boot ROM code which I
> think evaluates the PMC_SCRATCH0 register on Tegra to determine which
> mode to boot into. That's before even any firmware gets the chance of
> doing anything.

HTC apparently uses a separate RAM area to pass the reboot reason,
and they have a driver to store that, which is separate from the
driver that they use for actually rebooting the machine.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Heiko Stübner]

Am Dienstag, 15. Dezember 2015, 17:34:00 schrieb Arnd Bergmann:
> On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote:
> > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > > > rockchip platform have a protocol to pass the kernel reboot
> > > > mode to bootloader by some special registers when system reboot.
> > > > By this way the bootloader can take different action according
> > > > to the different kernel reboot mode, for example, command
> > > > "reboot loader" will reboot the board to rockusb mode, this is
> > > > a very convenient way to get the board enter download mode.
> > > > 
> > > > Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > > 
> > > Adding John Stultz to Cc
> > > 
> > > I just saw this thread pop up again, and had to think of John's recent
> > > patch to unify this across platforms.
> > > 
> > > John, can you have a look at this driver too, and see how it fits in?
> > > I think this is yet another variant, using an MMIO register rather than
> > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> > > it conceptually fits in with what you had.
> > 
> > FWIW, Tegra typically does use an MMIO register as well. See
> > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
> > HTC does, but if it's writing somewhere in RAM it isn't using the
> > standard way of resetting the SoC. There's early boot ROM code which I
> > think evaluates the PMC_SCRATCH0 register on Tegra to determine which
> > mode to boot into. That's before even any firmware gets the chance of
> > doing anything.
> 
> HTC apparently uses a separate RAM area to pass the reboot reason,
> and they have a driver to store that, which is separate from the
> driver that they use for actually rebooting the machine.

same on Rockchip. The general restart handling doesn't care about any reason, 
it is merely an agreement between kernel and bootloader to store a reason 
value in some reboot-safe register of the soc.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

On Tue, Dec 15, 2015 at 05:34:00PM +0100, Arnd Bergmann wrote:
> On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote:
> > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > > > rockchip platform have a protocol to pass the kernel reboot
> > > > mode to bootloader by some special registers when system reboot.
> > > > By this way the bootloader can take different action according
> > > > to the different kernel reboot mode, for example, command
> > > > "reboot loader" will reboot the board to rockusb mode, this is
> > > > a very convenient way to get the board enter download mode.
> > > > 
> > > > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> > > 
> > > Adding John Stultz to Cc
> > > 
> > > I just saw this thread pop up again, and had to think of John's recent
> > > patch to unify this across platforms.
> > > 
> > > John, can you have a look at this driver too, and see how it fits in?
> > > I think this is yet another variant, using an MMIO register rather than
> > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> > > it conceptually fits in with what you had.
> > 
> > FWIW, Tegra typically does use an MMIO register as well. See
> > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
> > HTC does, but if it's writing somewhere in RAM it isn't using the
> > standard way of resetting the SoC. There's early boot ROM code which I
> > think evaluates the PMC_SCRATCH0 register on Tegra to determine which
> > mode to boot into. That's before even any firmware gets the chance of
> > doing anything.
> 
> HTC apparently uses a separate RAM area to pass the reboot reason,
> and they have a driver to store that, which is separate from the
> driver that they use for actually rebooting the machine.

I wasn't very clear, but the PMC_SCRATCH0 register is used to store the
reset reason. It supports the recovery mode, which I think is really an
Android thing, "bootloader" will typically cause the bootloader not to
boot anything, and "forced-recovery" will go into a recovery mode that
is used to bootstrap the device (usually by uploading a "miniloader"
that initializes RAM, downloads a bootloader for booting or flashing an
operating system, ...).

The write that resets the SoC is to a different register.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Arnd Bergmann]

On Tuesday 15 December 2015 18:42:36 Thierry Reding wrote:
> On Tue, Dec 15, 2015 at 05:34:00PM +0100, Arnd Bergmann wrote:
> > On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote:
> > > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> > > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > > > > rockchip platform have a protocol to pass the kernel reboot
> > > > > mode to bootloader by some special registers when system reboot.
> > > > > By this way the bootloader can take different action according
> > > > > to the different kernel reboot mode, for example, command
> > > > > "reboot loader" will reboot the board to rockusb mode, this is
> > > > > a very convenient way to get the board enter download mode.
> > > > > 
> > > > > Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > > > 
> > > > Adding John Stultz to Cc
> > > > 
> > > > I just saw this thread pop up again, and had to think of John's recent
> > > > patch to unify this across platforms.
> > > > 
> > > > John, can you have a look at this driver too, and see how it fits in?
> > > > I think this is yet another variant, using an MMIO register rather than
> > > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> > > > it conceptually fits in with what you had.
> > > 
> > > FWIW, Tegra typically does use an MMIO register as well. See
> > > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
> > > HTC does, but if it's writing somewhere in RAM it isn't using the
> > > standard way of resetting the SoC. There's early boot ROM code which I
> > > think evaluates the PMC_SCRATCH0 register on Tegra to determine which
> > > mode to boot into. That's before even any firmware gets the chance of
> > > doing anything.


I just checked the android lollipop tree, and I could not find a pmc_restart_notify
function, only this file

https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/drivers/htc_debug/stability/reboot_params.c

with the device that stores into RAM. It looks like HTC ported over
a driver that they were already using on some Qualcomm MSM8960 device,
as in 

https://gitlab.com/MaC/android_kernel_htc_msm8960/blob/859977fc723f59a6b707df1d70e80826ee1dccc4/arch/arm/mach-msm/htc/htc_restart_handler.c

On Android marshmallow (Flounder), that file again does not exist, and
I don't see how it's done.

> > HTC apparently uses a separate RAM area to pass the reboot reason,
> > and they have a driver to store that, which is separate from the
> > driver that they use for actually rebooting the machine.
> 
> I wasn't very clear, but the PMC_SCRATCH0 register is used to store the
> reset reason. It supports the recovery mode, which I think is really an
> Android thing, "bootloader" will typically cause the bootloader not to
> boot anything, and "forced-recovery" will go into a recovery mode that
> is used to bootstrap the device (usually by uploading a "miniloader"
> that initializes RAM, downloads a bootloader for booting or flashing an
> operating system, ...).
> 
> The write that resets the SoC is to a different register.

So is this scratch register interpreted by some maskrom code, or by code that
can be provided by the OEM?

	Arnd

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4879 bytes --]

On Tue, Dec 15, 2015 at 09:38:41PM +0100, Arnd Bergmann wrote:
> On Tuesday 15 December 2015 18:42:36 Thierry Reding wrote:
> > On Tue, Dec 15, 2015 at 05:34:00PM +0100, Arnd Bergmann wrote:
> > > On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote:
> > > > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote:
> > > > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> > > > > > rockchip platform have a protocol to pass the kernel reboot
> > > > > > mode to bootloader by some special registers when system reboot.
> > > > > > By this way the bootloader can take different action according
> > > > > > to the different kernel reboot mode, for example, command
> > > > > > "reboot loader" will reboot the board to rockusb mode, this is
> > > > > > a very convenient way to get the board enter download mode.
> > > > > > 
> > > > > > Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > > > > 
> > > > > Adding John Stultz to Cc
> > > > > 
> > > > > I just saw this thread pop up again, and had to think of John's recent
> > > > > patch to unify this across platforms.
> > > > > 
> > > > > John, can you have a look at this driver too, and see how it fits in?
> > > > > I think this is yet another variant, using an MMIO register rather than
> > > > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
> > > > > it conceptually fits in with what you had.
> > > > 
> > > > FWIW, Tegra typically does use an MMIO register as well. See
> > > > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what
> > > > HTC does, but if it's writing somewhere in RAM it isn't using the
> > > > standard way of resetting the SoC. There's early boot ROM code which I
> > > > think evaluates the PMC_SCRATCH0 register on Tegra to determine which
> > > > mode to boot into. That's before even any firmware gets the chance of
> > > > doing anything.
> 
> 
> I just checked the android lollipop tree, and I could not find a pmc_restart_notify
> function, only this file
> 
> https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/drivers/htc_debug/stability/reboot_params.c
> 
> with the device that stores into RAM. It looks like HTC ported over
> a driver that they were already using on some Qualcomm MSM8960 device,
> as in 
> 
> https://gitlab.com/MaC/android_kernel_htc_msm8960/blob/859977fc723f59a6b707df1d70e80826ee1dccc4/arch/arm/mach-msm/htc/htc_restart_handler.c
> 
> On Android marshmallow (Flounder), that file again does not exist, and
> I don't see how it's done.
> 
> > > HTC apparently uses a separate RAM area to pass the reboot reason,
> > > and they have a driver to store that, which is separate from the
> > > driver that they use for actually rebooting the machine.
> > 
> > I wasn't very clear, but the PMC_SCRATCH0 register is used to store the
> > reset reason. It supports the recovery mode, which I think is really an
> > Android thing, "bootloader" will typically cause the bootloader not to
> > boot anything, and "forced-recovery" will go into a recovery mode that
> > is used to bootstrap the device (usually by uploading a "miniloader"
> > that initializes RAM, downloads a bootloader for booting or flashing an
> > operating system, ...).
> > 
> > The write that resets the SoC is to a different register.
> 
> So is this scratch register interpreted by some maskrom code, or by code that
> can be provided by the OEM?

My understanding is that its interpreted both by what's called BootROM
on Tegra (I guess that's what you call "maskrom code") and the system's
bootloader. The BootROM cannot typically be replaced by the OEM, but it
is quite typical for the bootloader to differ between devices.

The BootROM will interpret a subset of the bits in that register. Most
notable the "force recovery" bit. That will cause the BootROM to go into
a mode which can be used to bootstrap the device. It implements a simple
protocol (RCM) which can be used to upload a loader (usually referred to
as mini-loader) to the device's IRAM which in turn will initialize the
memory and which can download a proper bootloader (such as U-Boot) to
memory using a slightly more complex protocol (the standard protocol is
called nv3p, but the particular choice of protocol no longer matters at
this point).

The bootloader can also access this register and will interpret the
"bootloader" and "recovery" bits. On the, argueably small, sample of
Android devices I've worked with, the "bootloader" bit will make the
bootloader go into fastboot mode, whereas the "recovery" bit will make
it initiate the RCK mode, which is used to update through OTA.

There are a couple of other bits in this register, but they are badly
documented and don't seem to relate to reboot.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Arnd Bergmann]

On Monday 28 December 2015 10:20:56 Thierry Reding wrote:
> > > > HTC apparently uses a separate RAM area to pass the reboot reason,
> > > > and they have a driver to store that, which is separate from the
> > > > driver that they use for actually rebooting the machine.
> > > 
> > > I wasn't very clear, but the PMC_SCRATCH0 register is used to store the
> > > reset reason. It supports the recovery mode, which I think is really an
> > > Android thing, "bootloader" will typically cause the bootloader not to
> > > boot anything, and "forced-recovery" will go into a recovery mode that
> > > is used to bootstrap the device (usually by uploading a "miniloader"
> > > that initializes RAM, downloads a bootloader for booting or flashing an
> > > operating system, ...).
> > > 
> > > The write that resets the SoC is to a different register.
> > 
> > So is this scratch register interpreted by some maskrom code, or by code that
> > can be provided by the OEM?
> 
> My understanding is that its interpreted both by what's called BootROM
> on Tegra (I guess that's what you call "maskrom code") and the system's
> bootloader. The BootROM cannot typically be replaced by the OEM, but it
> is quite typical for the bootloader to differ between devices.

Ok, so not maskrom (which would not be OEM specific, but hardcoded for
the chip) but rather some form of PROM. This means we can only guess
that all OEMs use the same protocol but in theory someone could have
implemented an incompatible BootROM, but it's also possible that HTC
just ignore the register entirely and implement the same thing separately.

> The BootROM will interpret a subset of the bits in that register. Most
> notable the "force recovery" bit. That will cause the BootROM to go into
> a mode which can be used to bootstrap the device. It implements a simple
> protocol (RCM) which can be used to upload a loader (usually referred to
> as mini-loader) to the device's IRAM which in turn will initialize the
> memory and which can download a proper bootloader (such as U-Boot) to
> memory using a slightly more complex protocol (the standard protocol is
> called nv3p, but the particular choice of protocol no longer matters at
> this point).
> 
> The bootloader can also access this register and will interpret the
> "bootloader" and "recovery" bits. On the, argueably small, sample of
> Android devices I've worked with, the "bootloader" bit will make the
> bootloader go into fastboot mode, whereas the "recovery" bit will make
> it initiate the RCK mode, which is used to update through OTA.
> 
> There are a couple of other bits in this register, but they are badly
> documented and don't seem to relate to reboot.

Ok.

	Arnd

Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

On Mon, Dec 28, 2015 at 04:35:46PM +0100, Arnd Bergmann wrote:
> On Monday 28 December 2015 10:20:56 Thierry Reding wrote:
> > > > > HTC apparently uses a separate RAM area to pass the reboot reason,
> > > > > and they have a driver to store that, which is separate from the
> > > > > driver that they use for actually rebooting the machine.
> > > > 
> > > > I wasn't very clear, but the PMC_SCRATCH0 register is used to store the
> > > > reset reason. It supports the recovery mode, which I think is really an
> > > > Android thing, "bootloader" will typically cause the bootloader not to
> > > > boot anything, and "forced-recovery" will go into a recovery mode that
> > > > is used to bootstrap the device (usually by uploading a "miniloader"
> > > > that initializes RAM, downloads a bootloader for booting or flashing an
> > > > operating system, ...).
> > > > 
> > > > The write that resets the SoC is to a different register.
> > > 
> > > So is this scratch register interpreted by some maskrom code, or by code that
> > > can be provided by the OEM?
> > 
> > My understanding is that its interpreted both by what's called BootROM
> > on Tegra (I guess that's what you call "maskrom code") and the system's
> > bootloader. The BootROM cannot typically be replaced by the OEM, but it
> > is quite typical for the bootloader to differ between devices.
> 
> Ok, so not maskrom (which would not be OEM specific, but hardcoded for
> the chip) but rather some form of PROM. This means we can only guess
> that all OEMs use the same protocol but in theory someone could have
> implemented an incompatible BootROM, but it's also possible that HTC
> just ignore the register entirely and implement the same thing separately.

I wasn't being clear, the BootROM is hardcoded for the chip, I'm not
aware of a way to replace it once the chip's taped out.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 5/5] ARM64: dts: rockchip: add reboot node

[Andy Yan]

Add reboot notifier driver DT node for rk3368 platform

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v3:
 - add DT node

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3368.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index a712bea..c390f61 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -202,6 +202,12 @@
 		method = "smc";
 	};
 
+	reboot {
+		compatible = "rockchip,reboot";
+		rockchip,regmap = <&pmugrf>;
+		offset = <0x200>;
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
-- 
1.9.1

[PATCH v3 4/5] ARM: dts: rockchip: add reboot node

[Andy Yan]

Add reboot notifier driver DT node for rk3xxx,rk3288 platform

Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

---

Changes in v3:
 - add DT node

Changes in v2: None

 arch/arm/boot/dts/rk3288.dtsi | 6 ++++++
 arch/arm/boot/dts/rk3xxx.dtsi | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 906e938..975b8c9 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -169,6 +169,12 @@
 		};
 	};
 
+	reboot {
+		compatible = "rockchip,reboot";
+		rockchip,regmap = <&pmu>;
+		offset = <0x94>;
+	};
+
 	reserved-memory {
 		#address-cells = <1>;
 		#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 4497d28..7b14d7a 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -103,6 +103,12 @@
 		};
 	};
 
+	reboot {
+		compatible = "rockchip,reboot";
+		rockchip,regmap = <&pmu>;
+		offset = <0x40>;
+	};
+
 	xin24m: oscillator {
 		compatible = "fixed-clock";
 		clock-frequency = <24000000>;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] Add reboot notifier driver for rockchip platform

[Heiko Stübner]

Am Mittwoch, 18. November 2015, 17:47:24 schrieb Andy Yan:
> rockchip platform have a protocol to pass the kernel reboot
> mode to bootloader by some special registers when system reboot.
> By this way the bootloader can take different action according
> to the different kernel reboot mode, for example, command
> "reboot loader" will reboot the board to rockusb mode, this is
> a very convenient way to get the board enter download mode.
> And Android system also use this protocol to pass "recovery"、
> “fastboot” reboot mode to bootloader. In upstream land, We found
> tegra platform also use this mechanism.
> 
> Before this version, I have sent two Series, which can be found
> at [0] [1]
> [0] https://patchwork.kernel.org/patch/7140751/
> [1] https://patchwork.kernel.org/patch/7153531/

just so it doesn't get lost, this ties into a slightly more generic approach, 
John Stultz is doing for the android reboot reasons:

http://thread.gmane.org/gmane.linux.kernel/2103447

The only difference is that we'd need to use the pmu syscon for it.



> Changes in v3:
>  - rename a pinctrl node in rk3288-veyron, the original name will be
>    used in the incoming reboot notifier driver
>  - add dt binding
>  - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
>  - use dts pass the related register
>  - add DT node
> 
> Changes in v2:
>   - check cpu dt node
>   - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
>   - fix a align issue
>   - use reboot_notifier instead of restart_handler
> 
> Andy Yan (5):
>   ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset
>   dt-bindings: soc: add document for rockchip reboot notifier driver
>   soc: rockchip: add reboot notifier driver
>   ARM: dts: rockchip: add reboot node
>   ARM64: dts: rockchip: add reboot node
> 
>  .../bindings/soc/rockchip/rockchip-reboot.txt      | 18 ++++
>  arch/arm/boot/dts/rk3288-veyron.dtsi               |  2 +-
>  arch/arm/boot/dts/rk3288.dtsi                      |  6 ++
>  arch/arm/boot/dts/rk3xxx.dtsi                      |  6 ++
>  arch/arm64/boot/dts/rockchip/rk3368.dtsi           |  6 ++
>  drivers/soc/rockchip/Kconfig                       |  7 ++
>  drivers/soc/rockchip/Makefile                      |  1 +
>  drivers/soc/rockchip/loader.h                      | 22 +++++
>  drivers/soc/rockchip/reboot.c                      | 98
> ++++++++++++++++++++++ 9 files changed, 165 insertions(+), 1 deletion(-)
>  create mode 100644
> Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt create
> mode 100644 drivers/soc/rockchip/loader.h
>  create mode 100644 drivers/soc/rockchip/reboot.c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] Add reboot notifier driver for rockchip platform

[Andy Yan]

Hi John:

On 2015年12月12日 05:29, Heiko Stübner wrote:
> Am Mittwoch, 18. November 2015, 17:47:24 schrieb Andy Yan:
>> rockchip platform have a protocol to pass the kernel reboot
>> mode to bootloader by some special registers when system reboot.
>> By this way the bootloader can take different action according
>> to the different kernel reboot mode, for example, command
>> "reboot loader" will reboot the board to rockusb mode, this is
>> a very convenient way to get the board enter download mode.
>> And Android system also use this protocol to pass "recovery"、
>> “fastboot” reboot mode to bootloader. In upstream land, We found
>> tegra platform also use this mechanism.
>>
>> Before this version, I have sent two Series, which can be found
>> at [0] [1]
>> [0] https://patchwork.kernel.org/patch/7140751/
>> [1] https://patchwork.kernel.org/patch/7153531/
> just so it doesn't get lost, this ties into a slightly more generic approach,
> John Stultz is doing for the android reboot reasons:
>
> http://thread.gmane.org/gmane.linux.kernel/2103447
>
> The only difference is that we'd need to use the pmu syscon for it.
>
>
     Very glad to see your work on this. And your work is more generic 
than my.
      So hope we can share the code. As you can see, the rockchip 
platform use
      a GRF or PMU register to store the reboot reason. So maybe we need 
a phandle
      in the DT node to describe it.
>> Changes in v3:
>>   - rename a pinctrl node in rk3288-veyron, the original name will be
>>     used in the incoming reboot notifier driver
>>   - add dt binding
>>   - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
>>   - use dts pass the related register
>>   - add DT node
>>
>> Changes in v2:
>>    - check cpu dt node
>>    - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
>>    - fix a align issue
>>    - use reboot_notifier instead of restart_handler
>>
>> Andy Yan (5):
>>    ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset
>>    dt-bindings: soc: add document for rockchip reboot notifier driver
>>    soc: rockchip: add reboot notifier driver
>>    ARM: dts: rockchip: add reboot node
>>    ARM64: dts: rockchip: add reboot node
>>
>>   .../bindings/soc/rockchip/rockchip-reboot.txt      | 18 ++++
>>   arch/arm/boot/dts/rk3288-veyron.dtsi               |  2 +-
>>   arch/arm/boot/dts/rk3288.dtsi                      |  6 ++
>>   arch/arm/boot/dts/rk3xxx.dtsi                      |  6 ++
>>   arch/arm64/boot/dts/rockchip/rk3368.dtsi           |  6 ++
>>   drivers/soc/rockchip/Kconfig                       |  7 ++
>>   drivers/soc/rockchip/Makefile                      |  1 +
>>   drivers/soc/rockchip/loader.h                      | 22 +++++
>>   drivers/soc/rockchip/reboot.c                      | 98
>> ++++++++++++++++++++++ 9 files changed, 165 insertions(+), 1 deletion(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/soc/rockchip/rockchip-reboot.txt create
>> mode 100644 drivers/soc/rockchip/loader.h
>>   create mode 100644 drivers/soc/rockchip/reboot.c
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] Add reboot notifier driver for rockchip platform

[John Stultz]

On Mon, Dec 14, 2015 at 2:30 AM, Andy Yan <andy.yan@rock-chips.com> wrote:
> On 2015年12月12日 05:29, Heiko Stübner wrote:
>> Am Mittwoch, 18. November 2015, 17:47:24 schrieb Andy Yan:
>>>
>>> rockchip platform have a protocol to pass the kernel reboot
>>> mode to bootloader by some special registers when system reboot.
>>> By this way the bootloader can take different action according
>>> to the different kernel reboot mode, for example, command
>>> "reboot loader" will reboot the board to rockusb mode, this is
>>> a very convenient way to get the board enter download mode.
>>> And Android system also use this protocol to pass "recovery"、
>>> “fastboot” reboot mode to bootloader. In upstream land, We found
>>> tegra platform also use this mechanism.
>>>
>>> Before this version, I have sent two Series, which can be found
>>> at [0] [1]
>>> [0] https://patchwork.kernel.org/patch/7140751/
>>> [1] https://patchwork.kernel.org/patch/7153531/
>>
>> just so it doesn't get lost, this ties into a slightly more generic
>> approach,
>> John Stultz is doing for the android reboot reasons:
>>
>> http://thread.gmane.org/gmane.linux.kernel/2103447
>>
>> The only difference is that we'd need to use the pmu syscon for it.
>>
>>
>     Very glad to see your work on this. And your work is more generic than
> my.
>      So hope we can share the code. As you can see, the rockchip platform
> use
>      a GRF or PMU register to store the reboot reason. So maybe we need a
> phandle
>      in the DT node to describe it.

Yea. I need to figure out the best way to represent these various methods in DT.

Apologies for not being super responsive here. Been a bit heads down
trying to finish up some items before holiday break.

From some of Arnd's comments, it sounds like we may need a larger
generic subsystem for kernel to firmware communications here.  So more
then likely I'll not have a chance to take another swing at this after
the new year.

So if in the meantime you want to try to take my code and rework it,
or extend your code in a similar way, I'll not take any offence. :)

thanks
-john