[U-Boot] [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Yuantian,

On Tue, 28 Oct 2014 02:24:10 +0000, Yuantian Tang
<Yuantian.Tang@freescale.com> wrote:
> 
> > -----Original Message-----
> > From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> > Sent: Monday, October 27, 2014 5:42 PM
> > To: Tang Yuantian-B29983
> > Cc: Sun York-R58495; u-boot at lists.denx.de; Jin Zhengxiong-R64188
> > Subject: Re: [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable
> > before relocation
> > 
> > Hello Yuantian,
> > 
> > On Thu, 16 Oct 2014 04:42:06 +0000, Yuantian Tang
> > <Yuantian.Tang@freescale.com> wrote:
> > > > >> Wouldn't it be better to declare gic_dist_base as a local variable?
> > > > >>It is only used  once outside function armv7_switch_nonsec(). It
> > > > >>could be replaced with
> > > > >> get_gicd_base_address() call.
> > > > >>
> > > > >I am with you. That's what I did in the first version of this patch.
> > > > >Patch links is at: http://patchwork.ozlabs.org/patch/391065/
> > > > >But Albert seems have some concerns. The attached is what we discussed.
> > 
> > FTR, I only had concerns with the patch subject / commit summary.
> > Regarding the patch itself, I just asked whether the global was not used as some
> > means of coordination which would have been broken by turning it into a local,
> > but you had checked, so that was fine.
> > 
> > > > >Now on the second thought, I prefer the way this patch proposed
> > > > >because if we define gic_dist_base as local variable, That means
> > > > >function
> > > > >get_gicd_base_address() should be usable at any time in any mode.
> > > > >Can we make sure of that in the future?
> > > >
> > > > I don't strongly object introducing a new local variable. But I
> > > > don't see how the global variable is useful. Function
> > > > get_gicd_base_address() should be available all the time. It reads
> > > > PERIPHBASE register, or return a macro. It hasn't changed since the
> > > > first patch added it in 2013. Not sure if the original author Andre Przywara is
> > available to comments.
> > > >
> > > Thanks for your comments.
> > > If no one objects the original patch, I like to resubmit it.
> > >
> > > Hi Albert, what's your opinion on this?
> > 
> > 
> > Which 'original patch' do you mean?
> > 
> > If it is http://patchwork.ozlabs.org/patch/391065/ then I'm fine with it and will
> > apply it.
> > 
> Yes, it is.
> But I marked it as superseded because, as you suggested, this patch is resent as part of "deep sleep" patch set.
> I will send deep sleep patch set v2 to address TOM's concerns. You can apply them all together.

Ok, thanks for the clarification.

> Thanks,
> Yuantian

Amicalement,
-- 
Albert.