On Wed, Jan 28, 2015 at 10:27:47PM +0100, Daniel Schwierzeck wrote:
> Am 28.01.2015 um 22:05 schrieb Paul Burton:
> > On Wed, Jan 28, 2015 at 09:31:25PM +0100, Daniel Schwierzeck wrote:
> >> Hi Paul,
> >>
> >> Am 26.01.2015 um 16:02 schrieb Paul Burton:
> >>> This series cleans up the MIPS cache code somewhat, and unifies the
> >>> mips32 & mips64 implementations of it. This is largely in preparation
> >>> for further patches adding L2 cache support. The final patch of this
> >>> series fixes a bug encountered with recent cores on Malta boards.
> >>
> >> thanks for doing this. I also have this on my to-do list.
> >>
> >> Just a thought: if we finally have just the mips_cache_reset function in
> >> cache_init.S, couldn't we reimplement it in C entirely? I see no reason
> >> to implement it in assembler.
> >
> > The only issue with that is that mips_cache_reset currently runs before
> > we have a stack set up, so if the C compiler were to attempt to use the
> > stack things would fall over. I guess we could set up the temporary
> > stack earlier, and it's probably unlikely the compiler would actually
> > make use of it so it shouldn't really slow things down.
>
> I think if mips_cache_reset is a leaf function and takes no arguments or
> return values, the compiler shouldn't add any stack reservation in the
> function prologue.
My thoughts are more if the compiler tries to spill registers to the
stack. Whilst that seems unlikely to happen for such a small piece of
code, at least with optimisations turned on, I can imagine it happening
if someone adds more code (eg. L2 support ;) ) in future. At that point
we'd get crashes in the best case & weird corruption in the worst. The
compiler is also going to use the stack if optimisations are disabled
which isn't beyond the realms of possibility if debugging. So I don't
think there's any way we can use C & guarantee not to use the stack.
If we have an uncached stack then we can at least run, and assuming the
compiler doesn't make much use of the stack we could run without slowing
down too much. The only downside to this I can think of is that it
prevents us from ever moving the call to mips_cache_reset before the
call to lowlevel_init, which is something I have done for internal
builds in order to speed up execution on very slow systems (emulators
where running cached that bit earlier saves minutes in boot time).
That's something I'm somewhat hesitant to give up on... I'll keep
thinking.
> > Any chance we
> > could squeeze this into this merge window though, and I'll move the
> > cache init to C next time? That will simplify my L2 code somewhat too -
> > thanks for the suggestion!
>
> sure
Great :) I'll address the comments you made on patch 1 & submit a v2 of
that in the morning.
Thanks,
Paul
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
