Le 03/03/2018 à 17:46, Wolfgang Denk a écrit :
Dear Christophe,
In message
<622b8aec162cc43e774bde5da990a61fc961b4d9.1519976944.git.christophe.le...@c-s.fr>
you wrote:
Function get_immr() is almost not used and doesn't bring much
added value. Just replace it with mfspr(SPRN_IMMR) at the two
needed places.
...
static int check_CPU(long clock, uint pvr, uint immr)
{
- immap_t __iomem *immap = (immap_t __iomem *)(immr & 0xFFFF0000);
+ immap_t __iomem *immap = (immap_t __iomem *)CONFIG_SYS_IMMR;
This is a totally unrelated change, which additionally changes the
behaviour of the code - the content of the argument "immr" is now
not longer used here.
In many other places (eg. checkdcache(), do_reset(), etc.), immap is
defined as this. Why not doing the same everywhere ?
The lower part of immr is still used later in the function.
If this is necessary / intentional, it should be a separate commit
with proper explanation.
Ok
- uint immr = get_immr(0); /* Return full IMMR contents */
- immap_t __iomem *immap = (immap_t __iomem *)(immr & 0xFFFF0000);
+ immap_t __iomem *immap = (immap_t __iomem *)CONFIG_SYS_IMMR;
Ditto here.
- uint immr = get_immr(0); /* Return full IMMR contents */
- immap_t __iomem *immap = (immap_t __iomem *)(immr & 0xFFFF0000);
+ immap_t __iomem *immap = (immap_t __iomem *)CONFIG_SYS_IMMR;
And here again.
-#if defined(CONFIG_8xx)
-static inline uint get_immr(uint mask)
-{
- uint immr = mfspr(SPRN_IMMR);
-
- return mask ? (immr & mask) : immr;
-}
-#endif
Actually, I do not see what you win by this "cleanup". In my
opinion the function serves a good purpose; your code just becomes
more difficult to understand.
The main idea was to get rid of as much as possible specific 8xx stuff
in common header files.
Since SPRN_IMMR is defined regardless of the subarch, maybe I should
have just remove the #ifdef around get_immr()
Regarding the understandability of the code, I thought it would be
clearer to define immap the same way in all functions.
immr being set with CONFIG_SYS_IMMR early in start.S, and not changing
anywhere after that, I don't see any point in reading it from SPRN_IMMR
everytime we need it, especially as many other functions already set it
from CONFIG_SYS_IMMR. 83xx, 86xx etc... do set immap ptr from
CONFIG_SYS_IMMR too.
Regards
Christophe
Best regards,
Wolfgang Denk
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel
antivirus Avast.
https://www.avast.com/antivirus
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
