| Message ID | 1348242975-19184-14-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 21, 2012 at 04:56:11PM +0100, Cyril Chemparathy wrote: > This patch moves the TTBR1 offset calculation and the T1SZ calculation out > of the TTB setup assembly code. This should not affect functionality in > any way, but improves code readability as well as readability of subsequent > patches in this series. > > Signed-off-by: Cyril Chemparathy <cyril@ti.com> > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > Acked-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/include/asm/pgtable-3level-hwdef.h | 10 ++++++++++ > arch/arm/mm/proc-v7-3level.S | 16 ++++------------ > 2 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/include/asm/pgtable-3level-hwdef.h b/arch/arm/include/asm/pgtable-3level-hwdef.h > index d795282..b501650 100644 > --- a/arch/arm/include/asm/pgtable-3level-hwdef.h > +++ b/arch/arm/include/asm/pgtable-3level-hwdef.h > @@ -74,4 +74,14 @@ > #define PHYS_MASK_SHIFT (40) > #define PHYS_MASK ((1ULL << PHYS_MASK_SHIFT) - 1) > > +#if defined CONFIG_VMSPLIT_2G > +#define TTBR1_OFFSET (1 << 4) /* skip two L1 entries */ I know that was my code but I'm wondering why the (1<<4) rather than just a plain 16. > +#elif defined CONFIG_VMSPLIT_3G > +#define TTBR1_OFFSET (4096 * (1 + 3)) /* only L2, skip pgd + 3*pmd */ > +#else > +#define TTBR1_OFFSET 0 > +#endif > + > +#define TTBR1_SIZE (((PAGE_OFFSET >> 30) - 1) << 16) You could also move the comment about TxSZ in proc-v7-3level.S, it makes it easier to figure out why TTBR1_SIZE is defined this way. > --- a/arch/arm/mm/proc-v7-3level.S > +++ b/arch/arm/mm/proc-v7-3level.S > @@ -128,18 +128,10 @@ ENDPROC(cpu_v7_set_pte_ext) > * booting secondary CPUs would end up using TTBR1 for the identity > * mapping set up in TTBR0. > */ Higher up in this file there is a cmp with a "branch below" comment. You should remove that as well since the 9001 branch was removed.
On 9/24/2012 10:45 AM, Catalin Marinas wrote: > On Fri, Sep 21, 2012 at 04:56:11PM +0100, Cyril Chemparathy wrote: >> This patch moves the TTBR1 offset calculation and the T1SZ calculation out >> of the TTB setup assembly code. This should not affect functionality in >> any way, but improves code readability as well as readability of subsequent >> patches in this series. >> >> Signed-off-by: Cyril Chemparathy <cyril@ti.com> >> Signed-off-by: Vitaly Andrianov <vitalya@ti.com> >> Acked-by: Nicolas Pitre <nico@linaro.org> >> --- >> arch/arm/include/asm/pgtable-3level-hwdef.h | 10 ++++++++++ >> arch/arm/mm/proc-v7-3level.S | 16 ++++------------ >> 2 files changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/arch/arm/include/asm/pgtable-3level-hwdef.h b/arch/arm/include/asm/pgtable-3level-hwdef.h >> index d795282..b501650 100644 >> --- a/arch/arm/include/asm/pgtable-3level-hwdef.h >> +++ b/arch/arm/include/asm/pgtable-3level-hwdef.h >> @@ -74,4 +74,14 @@ >> #define PHYS_MASK_SHIFT (40) >> #define PHYS_MASK ((1ULL << PHYS_MASK_SHIFT) - 1) >> >> +#if defined CONFIG_VMSPLIT_2G >> +#define TTBR1_OFFSET (1 << 4) /* skip two L1 entries */ > > I know that was my code but I'm wondering why the (1<<4) rather than > just a plain 16. > ... to test my admittedly limited mental arithmetic skills, I'm guessing? :-) Will modify. >> +#elif defined CONFIG_VMSPLIT_3G >> +#define TTBR1_OFFSET (4096 * (1 + 3)) /* only L2, skip pgd + 3*pmd */ >> +#else >> +#define TTBR1_OFFSET 0 >> +#endif >> + >> +#define TTBR1_SIZE (((PAGE_OFFSET >> 30) - 1) << 16) > > You could also move the comment about TxSZ in proc-v7-3level.S, it makes > it easier to figure out why TTBR1_SIZE is defined this way. > Sure. Fixed for v4 of this series. >> --- a/arch/arm/mm/proc-v7-3level.S >> +++ b/arch/arm/mm/proc-v7-3level.S >> @@ -128,18 +128,10 @@ ENDPROC(cpu_v7_set_pte_ext) >> * booting secondary CPUs would end up using TTBR1 for the identity >> * mapping set up in TTBR0. >> */ > > Higher up in this file there is a cmp with a "branch below" comment. You > should remove that as well since the 9001 branch was removed. > Fixed for v4 of this series.
diff --git a/arch/arm/include/asm/pgtable-3level-hwdef.h b/arch/arm/include/asm/pgtable-3level-hwdef.h index d795282..b501650 100644 --- a/arch/arm/include/asm/pgtable-3level-hwdef.h +++ b/arch/arm/include/asm/pgtable-3level-hwdef.h @@ -74,4 +74,14 @@ #define PHYS_MASK_SHIFT (40) #define PHYS_MASK ((1ULL << PHYS_MASK_SHIFT) - 1) +#if defined CONFIG_VMSPLIT_2G +#define TTBR1_OFFSET (1 << 4) /* skip two L1 entries */ +#elif defined CONFIG_VMSPLIT_3G +#define TTBR1_OFFSET (4096 * (1 + 3)) /* only L2, skip pgd + 3*pmd */ +#else +#define TTBR1_OFFSET 0 +#endif + +#define TTBR1_SIZE (((PAGE_OFFSET >> 30) - 1) << 16) + #endif diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index c4f4251..5d93f00 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -128,18 +128,10 @@ ENDPROC(cpu_v7_set_pte_ext) * booting secondary CPUs would end up using TTBR1 for the identity * mapping set up in TTBR0. */ - bhi 9001f @ PHYS_OFFSET > PAGE_OFFSET? - orr \tmp, \tmp, #(((PAGE_OFFSET >> 30) - 1) << 16) @ TTBCR.T1SZ -#if defined CONFIG_VMSPLIT_2G - /* PAGE_OFFSET == 0x80000000, T1SZ == 1 */ - add \ttbr1, \ttbr1, #1 << 4 @ skip two L1 entries -#elif defined CONFIG_VMSPLIT_3G - /* PAGE_OFFSET == 0xc0000000, T1SZ == 2 */ - add \ttbr1, \ttbr1, #4096 * (1 + 3) @ only L2 used, skip pgd+3*pmd -#endif - /* CONFIG_VMSPLIT_1G does not need TTBR1 adjustment */ -9001: mcr p15, 0, \tmp, c2, c0, 2 @ TTB control register - mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 + orrls \tmp, \tmp, #TTBR1_SIZE @ TTBCR.T1SZ + mcr p15, 0, \tmp, c2, c0, 2 @ TTBCR + addls \ttbr1, \ttbr1, #TTBR1_OFFSET + mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 .endm __CPUINIT