All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: Andy Yan <andyshrk@gmail.com>
Cc: Rob Herring <robh@kernel.org>, Andy Yan <andy.yan@rock-chips.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Kumar Gala <galak@codeaurora.org>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Kevin Hilman <khilman@linaro.org>,
	Thierry Reding <treding@nvidia.com>,
	Caesar Wang <wxt@rock-chips.com>, Simon Glass <sjg@chromium.org>,
	Ben Chan <benchan@google.com>
Subject: Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver
Date: Tue, 01 Dec 2015 16:47:36 +0100	[thread overview]
Message-ID: <10512572.1QQvFtIA3T@diego> (raw)
In-Reply-To: <CANbgqARCYP0rfTPSJLNEnCdrb1+mCYYznnRVH6o8Bj5sWt9fDw@mail.gmail.com>

Hi Andy,

Am Dienstag, 1. Dezember 2015, 23:10:15 schrieb Andy Yan:
> 2015-11-23 21:15 GMT+08:00 Andy Yan <andyshrk@gmail.com>:
> > 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";

    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

WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
To: Andy Yan <andyshrk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Russell King - ARM Linux
	<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Ben Chan <benchan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver
Date: Tue, 01 Dec 2015 16:47:36 +0100	[thread overview]
Message-ID: <10512572.1QQvFtIA3T@diego> (raw)
In-Reply-To: <CANbgqARCYP0rfTPSJLNEnCdrb1+mCYYznnRVH6o8Bj5sWt9fDw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver
Date: Tue, 01 Dec 2015 16:47:36 +0100	[thread overview]
Message-ID: <10512572.1QQvFtIA3T@diego> (raw)
In-Reply-To: <CANbgqARCYP0rfTPSJLNEnCdrb1+mCYYznnRVH6o8Bj5sWt9fDw@mail.gmail.com>

Hi Andy,

Am Dienstag, 1. Dezember 2015, 23:10:15 schrieb Andy Yan:
> 2015-11-23 21:15 GMT+08:00 Andy Yan <andyshrk@gmail.com>:
> > 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 at 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 at 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

  reply	other threads:[~2015-12-01 15:47 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-18  9:47 [PATCH v3 0/5] Add reboot notifier driver for rockchip platform Andy Yan
2015-11-18  9:47 ` Andy Yan
2015-11-18  9:47 ` Andy Yan
2015-11-18  9:50 ` [PATCH v3 1/5] ARM: dts: rockchip: rk3288-veyron: rename pinctrl node reboot to reset Andy Yan
2015-11-18  9:50   ` Andy Yan
2015-11-18 14:18   ` Sergei Shtylyov
2015-11-18 14:18     ` Sergei Shtylyov
2015-11-19  0:51     ` Andy Yan
2015-11-18  9:53 ` [PATCH v3 2/5] dt-bindings: soc: add document for rockchip reboot notifier driver Andy Yan
2015-11-18  9:53   ` Andy Yan
2015-11-18 22:59   ` Rob Herring
2015-11-18 22:59     ` Rob Herring
2015-11-18 22:59     ` Rob Herring
2015-11-19  1:17     ` Andy Yan
2015-11-19  1:17       ` Andy Yan
2015-11-19  1:17       ` Andy Yan
2015-11-19  4:35       ` Heiko Stuebner
2015-11-19  4:35         ` Heiko Stuebner
2015-11-19  4:35         ` Heiko Stuebner
2015-11-20  1:16         ` Andy Yan
2015-11-20  1:16           ` Andy Yan
2015-11-20  1:16           ` Andy Yan
2015-11-20  1:58           ` Rob Herring
2015-11-20  1:58             ` Rob Herring
2015-11-20  1:58             ` Rob Herring
2015-11-23 13:15             ` Andy Yan
2015-12-01 15:10               ` Andy Yan
2015-12-01 15:47                 ` Heiko Stübner [this message]
2015-12-01 15:47                   ` Heiko Stübner
2015-12-01 15:47                   ` Heiko Stübner
2015-11-19 12:56       ` Thierry Reding
2015-11-19 12:56         ` Thierry Reding
2015-11-19 12:56         ` Thierry Reding
2015-11-19 13:39         ` Andy Yan
2015-11-19 15:30           ` Thierry Reding
2015-11-19 15:30             ` Thierry Reding
2015-11-19 15:30             ` Thierry Reding
2015-11-18  9:56 ` [PATCH v3 3/5] soc: rockchip: add " Andy Yan
2015-11-18  9:56   ` Andy Yan
2015-11-19  0:39   ` kbuild test robot
2015-11-19  0:39     ` kbuild test robot
2015-11-19  0:39     ` kbuild test robot
2015-12-14 11:39   ` Arnd Bergmann
2015-12-14 11:39     ` Arnd Bergmann
2015-12-14 11:39     ` Arnd Bergmann
2015-12-15 16:31     ` Thierry Reding
2015-12-15 16:31       ` Thierry Reding
2015-12-15 16:31       ` Thierry Reding
2015-12-15 16:34       ` Arnd Bergmann
2015-12-15 16:34         ` Arnd Bergmann
2015-12-15 16:34         ` Arnd Bergmann
2015-12-15 17:27         ` Heiko Stübner
2015-12-15 17:27           ` Heiko Stübner
2015-12-15 17:27           ` Heiko Stübner
2015-12-15 17:42         ` Thierry Reding
2015-12-15 17:42           ` Thierry Reding
2015-12-15 17:42           ` Thierry Reding
2015-12-15 20:38           ` Arnd Bergmann
2015-12-15 20:38             ` Arnd Bergmann
2015-12-15 20:38             ` Arnd Bergmann
2015-12-28  9:20             ` Thierry Reding
2015-12-28  9:20               ` Thierry Reding
2015-12-28  9:20               ` Thierry Reding
2015-12-28 15:35               ` Arnd Bergmann
2015-12-28 15:35                 ` Arnd Bergmann
2015-12-28 15:35                 ` Arnd Bergmann
2016-01-21 16:20                 ` Thierry Reding
2016-01-21 16:20                   ` Thierry Reding
2016-01-21 16:20                   ` Thierry Reding
2015-11-18 10:00 ` [PATCH v3 4/5] ARM: dts: rockchip: add reboot node Andy Yan
2015-11-18 10:00   ` Andy Yan
2015-11-18 10:00   ` Andy Yan
2015-11-18 10:05 ` [PATCH v3 5/5] ARM64: " Andy Yan
2015-11-18 10:05   ` Andy Yan
2015-12-11 21:29 ` [PATCH v3 0/5] Add reboot notifier driver for rockchip platform Heiko Stübner
2015-12-11 21:29   ` Heiko Stübner
2015-12-11 21:29   ` Heiko Stübner
2015-12-14 10:30   ` Andy Yan
2015-12-14 10:30     ` Andy Yan
2015-12-14 10:30     ` Andy Yan
2015-12-17  1:16     ` John Stultz
2015-12-17  1:16       ` John Stultz
2015-12-17  1:16       ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=10512572.1QQvFtIA3T@diego \
    --to=heiko@sntech.de \
    --cc=andy.yan@rock-chips.com \
    --cc=andyshrk@gmail.com \
    --cc=benchan@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=khilman@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh@kernel.org \
    --cc=sjg@chromium.org \
    --cc=treding@nvidia.com \
    --cc=wxt@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.