Re: [PATCH v4 15/21] ARM: exynos4/5: convert pmu wakeup to stacked domains

[Marc Zyngier <marc.zyngier@xxxxxxx>]

Hi Pankaj,

On 2015-01-20 07:42, Pankaj Dubey wrote:
> Hi Marc,
>
> On Monday 19 January 2015 03:14 PM, Marc Zyngier wrote:
> > Exynos has been (ab)using the gic_arch_extn to provide
> > wakeup from suspend, and it makes a lot of sense to convert
> > this code to use stacked domains instead.
> >
> > This patch does just this, updating the DT files to actually
> > reflect what the HW provides.
> >
> > BIG FAT WARNING: because the DTs were so far lying by not
> > exposing the fact that the PMU block is actually the first
> > interrupt controller in the chain for RTC, kernels with this patch
> > applied wont have any suspend-resume facility when booted
> > with old DTs, and old kernels with updated DTs may not even boot.
> >
> > Also, I stronly suspect that there is more than two wake-up
> > interrupts on these platforms, but I leave it to the maintainers
> > to fix their mess.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > ---
>
> I tested this patch series on SMDK5250 board.
>
> With the addition of "#interrupt-cells = <3>;"in PMU device node S2R
> is working on Exynos5250 based SMDK board.

Thanks for letting me know. Can I add your Tested-by tag on this?

> >   arch/arm/boot/dts/exynos4.dtsi    |   4 ++
> >   arch/arm/boot/dts/exynos5250.dtsi |   4 ++
> >   arch/arm/boot/dts/exynos5420.dtsi |   4 ++
> >   arch/arm/mach-exynos/exynos.c     |  14 ++---
> >   arch/arm/mach-exynos/suspend.c    | 122 
> > ++++++++++++++++++++++++++++++++++----
> >   5 files changed, 129 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos4.dtsi 
> > b/arch/arm/boot/dts/exynos4.dtsi
> > index b8168f1..0e7d74e 100644
> > --- a/arch/arm/boot/dts/exynos4.dtsi
> > +++ b/arch/arm/boot/dts/exynos4.dtsi
> > @@ -141,6 +141,9 @@
> >   	pmu_system_controller: system-controller@10020000 {
> >   		compatible = "samsung,exynos4210-pmu", "syscon";
> >   		reg = <0x10020000 0x4000>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +		interrupt-parent = <&gic>;
> >   	};
> >
> >   	dsi_0: dsi@11C80000 {
> > @@ -253,6 +256,7 @@
> >   	rtc@10070000 {
> >   		compatible = "samsung,s3c6410-rtc";
> >   		reg = <0x10070000 0x100>;
> > +		interrupt-parent = <&pmu_system_controller>;
> >   		interrupts = <0 44 0>, <0 45 0>;
> >   		clocks = <&clock CLK_RTC>;
> >   		clock-names = "rtc";
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
> > b/arch/arm/boot/dts/exynos5250.dtsi
> > index 0a229fc..1dc5f6b 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -194,6 +194,9 @@
> >   		clock-names = "clkout16";
> >   		clocks = <&clock CLK_FIN_PLL>;
> >   		#clock-cells = <1>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +		interrupt-parent = <&gic>;
> >   	};
> >
> >   	sysreg_system_controller: syscon@10050000 {
> > @@ -230,6 +233,7 @@
> >   	rtc: rtc@101E0000 {
> >   		clocks = <&clock CLK_RTC>;
> >   		clock-names = "rtc";
> > +		interrupt-parent = <&pmu_system_controller>;
> >   		status = "disabled";
> >   	};
> >
> > diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
> > b/arch/arm/boot/dts/exynos5420.dtsi
> > index 517e50f..35ecd36 100644
> > --- a/arch/arm/boot/dts/exynos5420.dtsi
> > +++ b/arch/arm/boot/dts/exynos5420.dtsi
> > @@ -309,6 +309,7 @@
> >   	rtc: rtc@101E0000 {
> >   		clocks = <&clock CLK_RTC>;
> >   		clock-names = "rtc";
> > +		interrupt-parent = <&pmu_system_controller>;
> >   		status = "disabled";
> >   	};
> >
> > @@ -748,6 +749,9 @@
> >   		clock-names = "clkout16";
> >   		clocks = <&clock CLK_FIN_PLL>;
> >   		#clock-cells = <1>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +		interrupt-parent = <&gic>;
> >   	};
> >
> >   	sysreg_system_controller: syscon@10050000 {
> > diff --git a/arch/arm/mach-exynos/exynos.c 
> > b/arch/arm/mach-exynos/exynos.c
> > index c13d083..e417fdc 100644
> > --- a/arch/arm/mach-exynos/exynos.c
> > +++ b/arch/arm/mach-exynos/exynos.c
> > @@ -175,16 +175,15 @@ static void __init exynos_init_io(void)
> >   	exynos_map_io();
> >   }
> >
> > +/*
> > + * Apparently, these SoCs are not able to wake-up from suspend 
> > using
> > + * the PMU. Too bad. Should they suddenly become capable of such a
> > + * feat, the matches below should be moved to suspend.c.
> > + */
> >   static const struct of_device_id exynos_dt_pmu_match[] = {
> >   	{ .compatible = "samsung,exynos3250-pmu" },
>
> As I know Exynos3250, S2R support has been added in kgene/for-next
> and should work as expected so we may need to do update
> "exynos_wkup_irq" for exynos3250 and remove it from this list, so 
> that
> it's S2R should not break. I am adding concern engineer (+cc: Chanwoo
> Choi) in the loop.

That would have to be an additional patch, unless we decide to delay 
this series.

Thanks,

        M.
--
Fast, cheap, reliable. Pick two.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html