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.
Regards, Yuantian
York
Amicalement,