Hi Prodyut,
On Monday 11 August 2008, Prodyut Hazarika wrote:
Please put your views on the replies below:
OK. But on which 4xx boards did you test this change? And could you
please remove the duplicated code from the "other", non-AMCC boards too with your > next patch version (DU440, PMC440, korat, lwmon5, pcs440ep). I added the other board maintainers to CC.
I have tested on Canyonlands, Glacier, Yosemite and Kilauea boards.
OK, thanks.
I will make the changes for the other non-AMCC boards and resubmit. I cannot test on non-AMCC boards, though I don't think this patch can cause an issue.
Of course you can't test this on non-AMCC boards. Thanks for further cleaning up.
- mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
- mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
- {
I don't particularly like this extra "{" level without need. Please
remove it.
The { is kept to avoid warnings. Without the "{", I would need to define the local variable near The beginning of the function. That would cause warning (Unused variable) for non 440EX/GT families. Please advice whether you want this warning to come up, if I remove the "{"
I knew why you did it this way (have been there before), but your "solution" is not the preferred one. As Wolfgang already explained the variable declarations should go to the beginning of the function and if warnings are seen on other platforms, then the declaration needs an #ifdef too.
How about 440EPx/GRx and it's problem with the PLB4A0_ACR[WRP].
Currently this gets cleared in a board specific file because of the MAL burst
problems in Linux when its set. Shouldn't we configure it correctly in
this place for all 440EPx/GRx and remove it from the board specific code?
This patch is getting bigger and bigger. I will put that cleanup in a separate patch, since it for a different set of cleanups.
OK. I just came to my mind after looking at the arbiter defines and their usage. We just should not forget to clean this up soon.
+#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C) +#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
I would really prefer to have uppercase defines here. I know that this
is not your "invention" but still we should probably clean this up now, that
we are busy with these PLB arbiter defines. So could you please change
those defines to uppercase? Thanks.
I will change all to uppercase and resubmit.
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================