Messages in this thread | | | Date | Fri, 15 Nov 2019 13:37:16 -0800 | From | Paul Burton <> | Subject | Re: [PATCH 2/2] MIPS: Ingenic: Disable abandoned HPTLB function. |
| |
Hi Zhou,
On Thu, Oct 24, 2019 at 05:29:01PM +0800, Zhou Yanjie wrote: > JZ4760/JZ4770/JZ4775/X1000/X1500 has an abandoned huge page > tlb, write 0xa9000000 to cp0 config5 sel4 to disable this > function to prevent getting stuck.
Can you describe how we "get stuck"? What actually goes wrong on the affected CPUs? Do they misinterpret EntryLo values? Which bits do they misinterpret?
> Confirmed by Ingenic, > this operation will not adversely affect processors > without HPTLB function. > > Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com> > --- > arch/mips/kernel/cpu-probe.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c > index 16033a4..cfebf8c 100644 > --- a/arch/mips/kernel/cpu-probe.c > +++ b/arch/mips/kernel/cpu-probe.c > @@ -1966,11 +1966,23 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu) > } > > /* > - * The config0 register in the Xburst CPUs with a processor ID of > + * The config0 register in the XBurst CPUs with a processor ID of > + * PRID_COMP_INGENIC_D1 has an abandoned huge page tlb, write > + * 0xa9000000 to cp0 config5 sel4 to disable this function to
Saying "config5" suggests $16 sel 5 to me - Config5 is after all an architecturally defined register & it's not this one. It'd be better to say "cop0 register 5 sel 4".
> + * prevent getting stuck. > + */ > + if ((c->processor_id & PRID_COMP_MASK) == PRID_COMP_INGENIC_D1) { > + __asm__ ( > + "li $2, 0xa9000000 \n\t" > + "mtc0 $2, $5, 4 \n\t" > + "nop \n\t" > + ::"r"(2));
I'd prefer that you add #defines to asm/mipsregs.h to provide a write_c0_X() function where X is replaced with whatever the name of this register is, and preferably also #define macros describing the fields present in the register. Writing a magic number isn't ideal.
> + /* > + * The config0 register in the XBurst CPUs with a processor ID of > * PRID_COMP_INGENIC_D0 report themselves as MIPS32r2 compatible, > * but they don't actually support this ISA. > */ > - if ((c->processor_id & PRID_COMP_MASK) == PRID_COMP_INGENIC_D0) > + } else if ((c->processor_id & PRID_COMP_MASK) == PRID_COMP_INGENIC_D0)
It might be cleaner to use a switch statement rather than writing out the & PRID_COMP_MASK condition twice?
Thanks, Paul
| |