| Message ID | 1352931637-3405-5-git-send-email-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Russell, With the 2 changes I have done on according your comments do you think you can give your acked-by for this patch? Thanks On 11/14/2012 11:20 PM, Gregory CLEMENT wrote: > From: Yehuda Yitschak <yehuday@marvell.com> > > Signed-off-by: Yehuda Yitschak <yehuday@marvell.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > arch/arm/mach-mvebu/Kconfig | 2 +- > arch/arm/mm/Kconfig | 4 ++++ > arch/arm/mm/proc-v7.S | 51 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 56 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > index 17d246b..9bfaa0c 100644 > --- a/arch/arm/mach-mvebu/Kconfig > +++ b/arch/arm/mach-mvebu/Kconfig > @@ -22,7 +22,7 @@ config MVEBU_CLK_CPU > config MACH_ARMADA_370_XP > bool > select ARMADA_370_XP_TIMER > - select CPU_V7 > + select CPU_PJ4B > > config MACH_ARMADA_370 > bool "Marvell Armada 370 boards" > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > index 94186b6..3fd629d 100644 > --- a/arch/arm/mm/Kconfig > +++ b/arch/arm/mm/Kconfig > @@ -352,6 +352,10 @@ config CPU_PJ4 > select ARM_THUMBEE > select CPU_V7 > > +config CPU_PJ4B > + bool > + select CPU_V7 > + > # ARMv6 > config CPU_V6 > bool "Support ARM V6 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > index 846d279..a4c0ccf 100644 > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -169,6 +169,47 @@ __v7_ca15mp_setup: > orreq r0, r0, r10 @ Enable CPU-specific SMP bits > mcreq p15, 0, r0, c1, c0, 1 > #endif > + > +__v7_pj4b_setup: > +#ifdef CONFIG_CPU_PJ4B > + > +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ > +#define CWF (1 << 27) /* Disable Critical Word First feature */ > +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ > +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ > + > +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) > + > + /* Auxiliary Debug Modes Control 1 Register */ > + mrc p15, 1, r0, c15, c1, 1 > + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. > + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr > + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling > + bic r0, r0, #(1 << 2) @ Disable Static BP > + mcr p15, 1, r0, c15, c1, 1 > + > + /* Auxiliary Debug Modes Control 2 Register */ > + mrc p15, 1, r0, c15, c1, 2 > + bic r0, r0, #(1 << 23) @ Enable fast LDR. > + orr r0, r0, #AUX_DBG_CTRL2 > + mcr p15, 1, r0, c15, c1, 2 > + > + /* Auxiliary Functional Modes Control Register 0 */ > + mrc p15, 1, r0, c15, c2, 0 > +#ifdef CONFIG_SMP > + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric > +#endif > + orr r0, r0, #(1 << 2) @ Support L1 parity checking > + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance > + mcr p15, 1, r0, c15, c2, 0 > + > + /* Auxiliary Debug Modes Control 0 Register */ > + mrc p15, 1, r0, c15, c1, 0 > + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle > + mcr p15, 1, r0, c15, c1, 0 > + > +#endif /* CONFIG_CPU_PJ4B */ > + > __v7_setup: > adr r12, __v7_setup_stack @ the local stack > stmia r12, {r0-r5, r7, r9, r11, lr} > @@ -342,6 +383,16 @@ __v7_ca9mp_proc_info: > .long 0xff0ffff0 > __v7_proc __v7_ca9mp_setup > .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info > + > + /* > + * Marvell PJ4B processor. > + */ > + .type __v7_pj4b_proc_info, #object > +__v7_pj4b_proc_info: > + .long 0x562f5840 > + .long 0xfffffff0 > + __v7_proc __v7_pj4b_setup > + .size __v7_pj4b_proc_info, . - __v7_pj4b_proc_info > #endif /* CONFIG_ARM_LPAE */ > > /* >
On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: > Hello Russell, > > With the 2 changes I have done on according your comments > do you think you can give your acked-by for this patch? > Hello Russell, I didn't have any feedback from you for this version. If you are fine with this, could you give your Acked-by? Thanks again, Gregory > On 11/14/2012 11:20 PM, Gregory CLEMENT wrote: >> From: Yehuda Yitschak <yehuday@marvell.com> >> >> Signed-off-by: Yehuda Yitschak <yehuday@marvell.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> arch/arm/mach-mvebu/Kconfig | 2 +- >> arch/arm/mm/Kconfig | 4 ++++ >> arch/arm/mm/proc-v7.S | 51 +++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 56 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig >> index 17d246b..9bfaa0c 100644 >> --- a/arch/arm/mach-mvebu/Kconfig >> +++ b/arch/arm/mach-mvebu/Kconfig >> @@ -22,7 +22,7 @@ config MVEBU_CLK_CPU >> config MACH_ARMADA_370_XP >> bool >> select ARMADA_370_XP_TIMER >> - select CPU_V7 >> + select CPU_PJ4B >> >> config MACH_ARMADA_370 >> bool "Marvell Armada 370 boards" >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig >> index 94186b6..3fd629d 100644 >> --- a/arch/arm/mm/Kconfig >> +++ b/arch/arm/mm/Kconfig >> @@ -352,6 +352,10 @@ config CPU_PJ4 >> select ARM_THUMBEE >> select CPU_V7 >> >> +config CPU_PJ4B >> + bool >> + select CPU_V7 >> + >> # ARMv6 >> config CPU_V6 >> bool "Support ARM V6 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX >> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S >> index 846d279..a4c0ccf 100644 >> --- a/arch/arm/mm/proc-v7.S >> +++ b/arch/arm/mm/proc-v7.S >> @@ -169,6 +169,47 @@ __v7_ca15mp_setup: >> orreq r0, r0, r10 @ Enable CPU-specific SMP bits >> mcreq p15, 0, r0, c1, c0, 1 >> #endif >> + >> +__v7_pj4b_setup: >> +#ifdef CONFIG_CPU_PJ4B >> + >> +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ >> +#define CWF (1 << 27) /* Disable Critical Word First feature */ >> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ >> +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ >> + >> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) >> + >> + /* Auxiliary Debug Modes Control 1 Register */ >> + mrc p15, 1, r0, c15, c1, 1 >> + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. >> + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr >> + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling >> + bic r0, r0, #(1 << 2) @ Disable Static BP >> + mcr p15, 1, r0, c15, c1, 1 >> + >> + /* Auxiliary Debug Modes Control 2 Register */ >> + mrc p15, 1, r0, c15, c1, 2 >> + bic r0, r0, #(1 << 23) @ Enable fast LDR. >> + orr r0, r0, #AUX_DBG_CTRL2 >> + mcr p15, 1, r0, c15, c1, 2 >> + >> + /* Auxiliary Functional Modes Control Register 0 */ >> + mrc p15, 1, r0, c15, c2, 0 >> +#ifdef CONFIG_SMP >> + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric >> +#endif >> + orr r0, r0, #(1 << 2) @ Support L1 parity checking >> + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance >> + mcr p15, 1, r0, c15, c2, 0 >> + >> + /* Auxiliary Debug Modes Control 0 Register */ >> + mrc p15, 1, r0, c15, c1, 0 >> + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle >> + mcr p15, 1, r0, c15, c1, 0 >> + >> +#endif /* CONFIG_CPU_PJ4B */ >> + >> __v7_setup: >> adr r12, __v7_setup_stack @ the local stack >> stmia r12, {r0-r5, r7, r9, r11, lr} >> @@ -342,6 +383,16 @@ __v7_ca9mp_proc_info: >> .long 0xff0ffff0 >> __v7_proc __v7_ca9mp_setup >> .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info >> + >> + /* >> + * Marvell PJ4B processor. >> + */ >> + .type __v7_pj4b_proc_info, #object >> +__v7_pj4b_proc_info: >> + .long 0x562f5840 >> + .long 0xfffffff0 >> + __v7_proc __v7_pj4b_setup >> + .size __v7_pj4b_proc_info, . - __v7_pj4b_proc_info >> #endif /* CONFIG_ARM_LPAE */ >> >> /* >> > >
On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: > On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: > > Hello Russell, > > > > With the 2 changes I have done on according your comments > > do you think you can give your acked-by for this patch? > > > > Hello Russell, > > I didn't have any feedback from you for this version. > If you are fine with this, could you give your Acked-by? > > Gregory, From the looks of things, Russell has his head wrapped around buggy sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas would care to weigh in? I'm reluctant to take the series without an Acked-By from at least one of them. thx, Jason. > > > On 11/14/2012 11:20 PM, Gregory CLEMENT wrote: > >> From: Yehuda Yitschak <yehuday@marvell.com> > >> > >> Signed-off-by: Yehuda Yitschak <yehuday@marvell.com> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > >> --- > >> arch/arm/mach-mvebu/Kconfig | 2 +- > >> arch/arm/mm/Kconfig | 4 ++++ > >> arch/arm/mm/proc-v7.S | 51 +++++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 56 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > >> index 17d246b..9bfaa0c 100644 > >> --- a/arch/arm/mach-mvebu/Kconfig > >> +++ b/arch/arm/mach-mvebu/Kconfig > >> @@ -22,7 +22,7 @@ config MVEBU_CLK_CPU > >> config MACH_ARMADA_370_XP > >> bool > >> select ARMADA_370_XP_TIMER > >> - select CPU_V7 > >> + select CPU_PJ4B > >> > >> config MACH_ARMADA_370 > >> bool "Marvell Armada 370 boards" > >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > >> index 94186b6..3fd629d 100644 > >> --- a/arch/arm/mm/Kconfig > >> +++ b/arch/arm/mm/Kconfig > >> @@ -352,6 +352,10 @@ config CPU_PJ4 > >> select ARM_THUMBEE > >> select CPU_V7 > >> > >> +config CPU_PJ4B > >> + bool > >> + select CPU_V7 > >> + > >> # ARMv6 > >> config CPU_V6 > >> bool "Support ARM V6 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX > >> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > >> index 846d279..a4c0ccf 100644 > >> --- a/arch/arm/mm/proc-v7.S > >> +++ b/arch/arm/mm/proc-v7.S > >> @@ -169,6 +169,47 @@ __v7_ca15mp_setup: > >> orreq r0, r0, r10 @ Enable CPU-specific SMP bits > >> mcreq p15, 0, r0, c1, c0, 1 > >> #endif > >> + > >> +__v7_pj4b_setup: > >> +#ifdef CONFIG_CPU_PJ4B > >> + > >> +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ > >> +#define CWF (1 << 27) /* Disable Critical Word First feature */ > >> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ > >> +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ > >> + > >> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) > >> + > >> + /* Auxiliary Debug Modes Control 1 Register */ > >> + mrc p15, 1, r0, c15, c1, 1 > >> + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. > >> + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr > >> + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling > >> + bic r0, r0, #(1 << 2) @ Disable Static BP > >> + mcr p15, 1, r0, c15, c1, 1 > >> + > >> + /* Auxiliary Debug Modes Control 2 Register */ > >> + mrc p15, 1, r0, c15, c1, 2 > >> + bic r0, r0, #(1 << 23) @ Enable fast LDR. > >> + orr r0, r0, #AUX_DBG_CTRL2 > >> + mcr p15, 1, r0, c15, c1, 2 > >> + > >> + /* Auxiliary Functional Modes Control Register 0 */ > >> + mrc p15, 1, r0, c15, c2, 0 > >> +#ifdef CONFIG_SMP > >> + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric > >> +#endif > >> + orr r0, r0, #(1 << 2) @ Support L1 parity checking > >> + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance > >> + mcr p15, 1, r0, c15, c2, 0 > >> + > >> + /* Auxiliary Debug Modes Control 0 Register */ > >> + mrc p15, 1, r0, c15, c1, 0 > >> + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle > >> + mcr p15, 1, r0, c15, c1, 0 > >> + > >> +#endif /* CONFIG_CPU_PJ4B */ > >> + > >> __v7_setup: > >> adr r12, __v7_setup_stack @ the local stack > >> stmia r12, {r0-r5, r7, r9, r11, lr} > >> @@ -342,6 +383,16 @@ __v7_ca9mp_proc_info: > >> .long 0xff0ffff0 > >> __v7_proc __v7_ca9mp_setup > >> .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info > >> + > >> + /* > >> + * Marvell PJ4B processor. > >> + */ > >> + .type __v7_pj4b_proc_info, #object > >> +__v7_pj4b_proc_info: > >> + .long 0x562f5840 > >> + .long 0xfffffff0 > >> + __v7_proc __v7_pj4b_setup > >> + .size __v7_pj4b_proc_info, . - __v7_pj4b_proc_info > >> #endif /* CONFIG_ARM_LPAE */ > >> > >> /* > >> > > > > > > > -- > Gregory Clement, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 11/19/2012 04:12 AM, Jason Cooper wrote: > On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: >> On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: >>> Hello Russell, >>> >>> With the 2 changes I have done on according your comments >>> do you think you can give your acked-by for this patch? >>> >> >> Hello Russell, >> >> I didn't have any feedback from you for this version. >> If you are fine with this, could you give your Acked-by? >> >> > > Gregory, > > From the looks of things, Russell has his head wrapped around buggy > sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas > would care to weigh in? I'm reluctant to take the series without an > Acked-By from at least one of them. I am fine with this as Will already have done the review. Arnd, Olof, Do you agree to get the pull from Jason with an acked-by from Will and/or Catalin instead of Russell? Catalin, Will, If Arnd and Olof agree, would you mind give your acked-by on this pacth? Russell, if you had a couple of minutes, the best would be to have your acked-by of course. Thanks all, Gregory
On Sun, Nov 18, 2012 at 10:12:10PM -0500, Jason Cooper wrote: > On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: > > On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: > > > Hello Russell, > > > > > > With the 2 changes I have done on according your comments > > > do you think you can give your acked-by for this patch? > > > > > > > Hello Russell, > > > > I didn't have any feedback from you for this version. > > If you are fine with this, could you give your Acked-by? > > > > > > Gregory, > > >From the looks of things, Russell has his head wrapped around buggy > sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas > would care to weigh in? I'm reluctant to take the series without an > Acked-By from at least one of them. Well, I would like to say - it should go through my tree, as it's generic ARM changes, and as very little (even generic stuff) seems to come through my tree anymore. But that's going to make the remainder of your patches difficult to merge.
On Mon, Nov 19, 2012 at 09:19:12AM +0000, Russell King - ARM Linux wrote: > On Sun, Nov 18, 2012 at 10:12:10PM -0500, Jason Cooper wrote: > > On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: > > > On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: > > > > Hello Russell, > > > > > > > > With the 2 changes I have done on according your comments > > > > do you think you can give your acked-by for this patch? > > > > > > > > > > Hello Russell, > > > > > > I didn't have any feedback from you for this version. > > > If you are fine with this, could you give your Acked-by? > > > > > > > > > > Gregory, > > > > >From the looks of things, Russell has his head wrapped around buggy > > sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas > > would care to weigh in? I'm reluctant to take the series without an > > Acked-By from at least one of them. > > Well, I would like to say - it should go through my tree, as it's generic > ARM changes, and as very little (even generic stuff) seems to come through > my tree anymore. But that's going to make the remainder of your patches > difficult to merge. We'd really appreciate it if we could take it with your Ack. This series and one more that depends on it are my only hold overs for v3.8. thx, Jason.
Hi Gregory, On 14 November 2012 22:20, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > index 94186b6..3fd629d 100644 > --- a/arch/arm/mm/Kconfig > +++ b/arch/arm/mm/Kconfig > @@ -352,6 +352,10 @@ config CPU_PJ4 > select ARM_THUMBEE > select CPU_V7 > > +config CPU_PJ4B > + bool > + select CPU_V7 With ARMv6 we started just using the generic architecture name for different processors (i.e. CPU_V6, CPU_V7). Is PJ4B another ARMv7 implementation (like Cortex-A9)? > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > index 846d279..a4c0ccf 100644 > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -169,6 +169,47 @@ __v7_ca15mp_setup: > orreq r0, r0, r10 @ Enable CPU-specific SMP bits > mcreq p15, 0, r0, c1, c0, 1 > #endif > + > +__v7_pj4b_setup: > +#ifdef CONFIG_CPU_PJ4B > + > +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ > +#define CWF (1 << 27) /* Disable Critical Word First feature */ > +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ > +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ > + > +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) > + > + /* Auxiliary Debug Modes Control 1 Register */ > + mrc p15, 1, r0, c15, c1, 1 > + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. > + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr > + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling > + bic r0, r0, #(1 << 2) @ Disable Static BP > + mcr p15, 1, r0, c15, c1, 1 > + > + /* Auxiliary Debug Modes Control 2 Register */ > + mrc p15, 1, r0, c15, c1, 2 > + bic r0, r0, #(1 << 23) @ Enable fast LDR. > + orr r0, r0, #AUX_DBG_CTRL2 > + mcr p15, 1, r0, c15, c1, 2 > + > + /* Auxiliary Functional Modes Control Register 0 */ > + mrc p15, 1, r0, c15, c2, 0 > +#ifdef CONFIG_SMP > + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric > +#endif > + orr r0, r0, #(1 << 2) @ Support L1 parity checking > + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance > + mcr p15, 1, r0, c15, c2, 0 > + > + /* Auxiliary Debug Modes Control 0 Register */ > + mrc p15, 1, r0, c15, c1, 0 > + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle > + mcr p15, 1, r0, c15, c1, 0 Any chance that these could be set by the firmware prior to starting the kernel? We don't have any guidance for Linux here but longer term it seems to cause problems (i.e. you add some secure layer in a new CPU version).
On Mon, Nov 19, 2012 at 08:13:05AM +0000, Gregory CLEMENT wrote: > On 11/19/2012 04:12 AM, Jason Cooper wrote: > > On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: > >> On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: > >>> Hello Russell, > >>> > >>> With the 2 changes I have done on according your comments > >>> do you think you can give your acked-by for this patch? > >>> > >> > >> Hello Russell, > >> > >> I didn't have any feedback from you for this version. > >> If you are fine with this, could you give your Acked-by? > >> > >> > > > > Gregory, > > > > From the looks of things, Russell has his head wrapped around buggy > > sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas > > would care to weigh in? I'm reluctant to take the series without an > > Acked-By from at least one of them. > > > I am fine with this as Will already have done the review. I reviewed the thing, so you can add: Reviewed-by: Will Deacon <will.deacon@arm.com> However, if Russell wants to take this via his tree then perhaps it's best for arm-soc to pull a branch from him containing this, like we've done in the past. The alternative is to order the pull requests to Linus, but that sounds like a bad idea (although I suspect the rest of your series still at least compiles without this patch?). Will
On 11/19/2012 11:51 AM, Catalin Marinas wrote: > Hi Gregory, > > On 14 November 2012 22:20, Gregory CLEMENT > <gregory.clement@free-electrons.com> wrote: >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig >> index 94186b6..3fd629d 100644 >> --- a/arch/arm/mm/Kconfig >> +++ b/arch/arm/mm/Kconfig >> @@ -352,6 +352,10 @@ config CPU_PJ4 >> select ARM_THUMBEE >> select CPU_V7 >> >> +config CPU_PJ4B >> + bool >> + select CPU_V7 > > With ARMv6 we started just using the generic architecture name for > different processors (i.e. CPU_V6, CPU_V7). Is PJ4B another ARMv7 > implementation (like Cortex-A9)? Yes it is. > >> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S >> index 846d279..a4c0ccf 100644 >> --- a/arch/arm/mm/proc-v7.S >> +++ b/arch/arm/mm/proc-v7.S >> @@ -169,6 +169,47 @@ __v7_ca15mp_setup: >> orreq r0, r0, r10 @ Enable CPU-specific SMP bits >> mcreq p15, 0, r0, c1, c0, 1 >> #endif >> + >> +__v7_pj4b_setup: >> +#ifdef CONFIG_CPU_PJ4B >> + >> +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ >> +#define CWF (1 << 27) /* Disable Critical Word First feature */ >> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ >> +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ >> + >> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) >> + >> + /* Auxiliary Debug Modes Control 1 Register */ >> + mrc p15, 1, r0, c15, c1, 1 >> + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. >> + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr >> + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling >> + bic r0, r0, #(1 << 2) @ Disable Static BP >> + mcr p15, 1, r0, c15, c1, 1 >> + >> + /* Auxiliary Debug Modes Control 2 Register */ >> + mrc p15, 1, r0, c15, c1, 2 >> + bic r0, r0, #(1 << 23) @ Enable fast LDR. >> + orr r0, r0, #AUX_DBG_CTRL2 >> + mcr p15, 1, r0, c15, c1, 2 >> + >> + /* Auxiliary Functional Modes Control Register 0 */ >> + mrc p15, 1, r0, c15, c2, 0 >> +#ifdef CONFIG_SMP >> + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric >> +#endif >> + orr r0, r0, #(1 << 2) @ Support L1 parity checking >> + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance >> + mcr p15, 1, r0, c15, c2, 0 >> + >> + /* Auxiliary Debug Modes Control 0 Register */ >> + mrc p15, 1, r0, c15, c1, 0 >> + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle >> + mcr p15, 1, r0, c15, c1, 0 > > Any chance that these could be set by the firmware prior to starting > the kernel? We don't have any guidance for Linux here but longer term > it seems to cause problems (i.e. you add some secure layer in a new > CPU version). > I will ask to Marvell engineers. However I hope it won't be show stopper for merging this code in 3.8. This patch set was released 7 weeks ago, so would have though there was a lot of time to raise the issue related to this code. That's said, if the change to be done are small enough it could be done as a fix for the 3.8 release candidate. Thanks, Gregory
On 11/19/2012 10:19 AM, Russell King - ARM Linux wrote: > On Sun, Nov 18, 2012 at 10:12:10PM -0500, Jason Cooper wrote: >> On Fri, Nov 16, 2012 at 10:46:15PM +0100, Gregory CLEMENT wrote: >>> On 11/14/2012 11:31 PM, Gregory CLEMENT wrote: >>>> Hello Russell, >>>> >>>> With the 2 changes I have done on according your comments >>>> do you think you can give your acked-by for this patch? >>>> >>> >>> Hello Russell, >>> >>> I didn't have any feedback from you for this version. >>> If you are fine with this, could you give your Acked-by? >>> >>> >> >> Gregory, >> >> >From the looks of things, Russell has his head wrapped around buggy >> sound driver code for the CuBox. Perhaps Will Deacon or Catalin Marinas >> would care to weigh in? I'm reluctant to take the series without an >> Acked-By from at least one of them. > > Well, I would like to say - it should go through my tree, as it's generic > ARM changes, and as very little (even generic stuff) seems to come through > my tree anymore. But that's going to make the remainder of your patches > difficult to merge. > When I asked if it was possible to Arnd and Olof to take this patch in their tree with your Acked-by, you didn't raise any objection. At this moment it would have been possible to rework the patch set. But now I am afraid that we are out of time to do it in a proper way. Now if you really want to get this patch through your patch system instead of giving a acked-by, well then I will do my best, of course, to fulfil your requirements. But in this case I would have appreciate to be notified earlier. Thanks, Gregory
On Mon, Nov 19, 2012 at 12:18:14PM +0000, Gregory CLEMENT wrote: > On 11/19/2012 11:51 AM, Catalin Marinas wrote: > > On 14 November 2012 22:20, Gregory CLEMENT > > <gregory.clement@free-electrons.com> wrote: > >> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > >> index 846d279..a4c0ccf 100644 > >> --- a/arch/arm/mm/proc-v7.S > >> +++ b/arch/arm/mm/proc-v7.S > >> @@ -169,6 +169,47 @@ __v7_ca15mp_setup: > >> orreq r0, r0, r10 @ Enable CPU-specific SMP bits > >> mcreq p15, 0, r0, c1, c0, 1 > >> #endif > >> + > >> +__v7_pj4b_setup: > >> +#ifdef CONFIG_CPU_PJ4B > >> + > >> +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ > >> +#define CWF (1 << 27) /* Disable Critical Word First feature */ > >> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ > >> +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ > >> + > >> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) > >> + > >> + /* Auxiliary Debug Modes Control 1 Register */ > >> + mrc p15, 1, r0, c15, c1, 1 > >> + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. > >> + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr > >> + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling > >> + bic r0, r0, #(1 << 2) @ Disable Static BP > >> + mcr p15, 1, r0, c15, c1, 1 > >> + > >> + /* Auxiliary Debug Modes Control 2 Register */ > >> + mrc p15, 1, r0, c15, c1, 2 > >> + bic r0, r0, #(1 << 23) @ Enable fast LDR. > >> + orr r0, r0, #AUX_DBG_CTRL2 > >> + mcr p15, 1, r0, c15, c1, 2 BTW, for some bits you defined macros while others are just immediate values. Just a cosmetic inconsistency (I would have preferred to use either macros or just immediate values). > >> + > >> + /* Auxiliary Functional Modes Control Register 0 */ > >> + mrc p15, 1, r0, c15, c2, 0 > >> +#ifdef CONFIG_SMP > >> + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric > >> +#endif > >> + orr r0, r0, #(1 << 2) @ Support L1 parity checking > >> + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance > >> + mcr p15, 1, r0, c15, c2, 0 > >> + > >> + /* Auxiliary Debug Modes Control 0 Register */ > >> + mrc p15, 1, r0, c15, c1, 0 > >> + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle > >> + mcr p15, 1, r0, c15, c1, 0 > > > > Any chance that these could be set by the firmware prior to starting > > the kernel? We don't have any guidance for Linux here but longer term > > it seems to cause problems (i.e. you add some secure layer in a new > > CPU version). > > I will ask to Marvell engineers. > > However I hope it won't be show stopper for merging this code in 3.8. > This patch set was released 7 weeks ago, so would have though there was > a lot of time to raise the issue related to this code. It shouldn't prevent the code being merged as we don't have clear requirement in Linux for this. But it would be good practice to push some sane defaults to the firmware. For example you set some option like L1 replacement which is optimal for your platform. Later you put the same CPU on a different SoC which may have different optimal value. There is no way in proc-v7.S to detect the SoC. Otherwise the patch looks fine to me. Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On 11/19/2012 03:50 PM, Catalin Marinas wrote: > On Mon, Nov 19, 2012 at 12:18:14PM +0000, Gregory CLEMENT wrote: >> On 11/19/2012 11:51 AM, Catalin Marinas wrote: >>> On 14 November 2012 22:20, Gregory CLEMENT >>> <gregory.clement@free-electrons.com> wrote: >>>> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S >>>> index 846d279..a4c0ccf 100644 >>>> --- a/arch/arm/mm/proc-v7.S >>>> +++ b/arch/arm/mm/proc-v7.S >>>> @@ -169,6 +169,47 @@ __v7_ca15mp_setup: >>>> orreq r0, r0, r10 @ Enable CPU-specific SMP bits >>>> mcreq p15, 0, r0, c1, c0, 1 >>>> #endif >>>> + >>>> +__v7_pj4b_setup: >>>> +#ifdef CONFIG_CPU_PJ4B >>>> + >>>> +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ >>>> +#define CWF (1 << 27) /* Disable Critical Word First feature */ >>>> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ >>>> +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ >>>> + >>>> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) >>>> + >>>> + /* Auxiliary Debug Modes Control 1 Register */ >>>> + mrc p15, 1, r0, c15, c1, 1 >>>> + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. >>>> + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr >>>> + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling >>>> + bic r0, r0, #(1 << 2) @ Disable Static BP >>>> + mcr p15, 1, r0, c15, c1, 1 >>>> + >>>> + /* Auxiliary Debug Modes Control 2 Register */ >>>> + mrc p15, 1, r0, c15, c1, 2 >>>> + bic r0, r0, #(1 << 23) @ Enable fast LDR. >>>> + orr r0, r0, #AUX_DBG_CTRL2 >>>> + mcr p15, 1, r0, c15, c1, 2 > > BTW, for some bits you defined macros while others are just immediate > values. Just a cosmetic inconsistency (I would have preferred to use > either macros or just immediate values). Oh yes indeed: I made some change after Russell comments. But I should have review the whole code to notice it. If I didn't have time to change the code before the merge, I will fix it for the v3.8-rc1 > >>>> + >>>> + /* Auxiliary Functional Modes Control Register 0 */ >>>> + mrc p15, 1, r0, c15, c2, 0 >>>> +#ifdef CONFIG_SMP >>>> + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric >>>> +#endif >>>> + orr r0, r0, #(1 << 2) @ Support L1 parity checking >>>> + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance >>>> + mcr p15, 1, r0, c15, c2, 0 >>>> + >>>> + /* Auxiliary Debug Modes Control 0 Register */ >>>> + mrc p15, 1, r0, c15, c1, 0 >>>> + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle >>>> + mcr p15, 1, r0, c15, c1, 0 >>> >>> Any chance that these could be set by the firmware prior to starting >>> the kernel? We don't have any guidance for Linux here but longer term >>> it seems to cause problems (i.e. you add some secure layer in a new >>> CPU version). >> >> I will ask to Marvell engineers. >> >> However I hope it won't be show stopper for merging this code in 3.8. >> This patch set was released 7 weeks ago, so would have though there was >> a lot of time to raise the issue related to this code. > > It shouldn't prevent the code being merged as we don't have clear > requirement in Linux for this. But it would be good practice to push > some sane defaults to the firmware. For example you set some option like > L1 replacement which is optimal for your platform. Later you put the > same CPU on a different SoC which may have different optimal value. > There is no way in proc-v7.S to detect the SoC. > OK I see your point. At this stage that would imply to change the bootloader already deployed. I don't know how it will be easy to do it quickly. So I am not sure that we can change it immediately, but maybe for future release. However I still wait for more feedback on this subject for Marvell. > Otherwise the patch looks fine to me. > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> Thanks a lot!
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index 17d246b..9bfaa0c 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -22,7 +22,7 @@ config MVEBU_CLK_CPU config MACH_ARMADA_370_XP bool select ARMADA_370_XP_TIMER - select CPU_V7 + select CPU_PJ4B config MACH_ARMADA_370 bool "Marvell Armada 370 boards" diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig index 94186b6..3fd629d 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -352,6 +352,10 @@ config CPU_PJ4 select ARM_THUMBEE select CPU_V7 +config CPU_PJ4B + bool + select CPU_V7 + # ARMv6 config CPU_V6 bool "Support ARM V6 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index 846d279..a4c0ccf 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -169,6 +169,47 @@ __v7_ca15mp_setup: orreq r0, r0, r10 @ Enable CPU-specific SMP bits mcreq p15, 0, r0, c1, c0, 1 #endif + +__v7_pj4b_setup: +#ifdef CONFIG_CPU_PJ4B + +#define SNOOP_DATA (1 << 25) /* Dont interleave write and snoop data */ +#define CWF (1 << 27) /* Disable Critical Word First feature */ +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */ +#define L1_REP_RR (1 << 30) /* L1 replacement - Strict round robin */ + +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR) + + /* Auxiliary Debug Modes Control 1 Register */ + mrc p15, 1, r0, c15, c1, 1 + orr r0, r0, #(1 << 16) @ Disable data transfer for clean line. + orr r0, r0, #(1 << 5) @ Enable the back off of STREX instr + orr r0, r0, #(1 << 8) @ Disable Internal Parity Handling + bic r0, r0, #(1 << 2) @ Disable Static BP + mcr p15, 1, r0, c15, c1, 1 + + /* Auxiliary Debug Modes Control 2 Register */ + mrc p15, 1, r0, c15, c1, 2 + bic r0, r0, #(1 << 23) @ Enable fast LDR. + orr r0, r0, #AUX_DBG_CTRL2 + mcr p15, 1, r0, c15, c1, 2 + + /* Auxiliary Functional Modes Control Register 0 */ + mrc p15, 1, r0, c15, c2, 0 +#ifdef CONFIG_SMP + orr r0, r0, #(1 << 1) @ Set SMP mode. Join the coherency fabric +#endif + orr r0, r0, #(1 << 2) @ Support L1 parity checking + orr r0, r0, #(1 << 8) @ Broadcast Cache and TLB maintenance + mcr p15, 1, r0, c15, c2, 0 + + /* Auxiliary Debug Modes Control 0 Register */ + mrc p15, 1, r0, c15, c1, 0 + orr r0, r0, #(1 << 22) @ WFI/WFE - serve the DVM and back to idle + mcr p15, 1, r0, c15, c1, 0 + +#endif /* CONFIG_CPU_PJ4B */ + __v7_setup: adr r12, __v7_setup_stack @ the local stack stmia r12, {r0-r5, r7, r9, r11, lr} @@ -342,6 +383,16 @@ __v7_ca9mp_proc_info: .long 0xff0ffff0 __v7_proc __v7_ca9mp_setup .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info + + /* + * Marvell PJ4B processor. + */ + .type __v7_pj4b_proc_info, #object +__v7_pj4b_proc_info: + .long 0x562f5840 + .long 0xfffffff0 + __v7_proc __v7_pj4b_setup + .size __v7_pj4b_proc_info, . - __v7_pj4b_proc_info #endif /* CONFIG_ARM_LPAE */ /*