Dear Christophe,
In message 71a3900b-f61e-e9f8-c12a-5ec5aa1420bc@c-s.fr you wrote:
- 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 ?
Firwt, it is an unrelated and undocumented change - you don't mention it in the commit message. This _must_ be fixed. Actually I think this should be a separate patch, both for documentation and bisectability.
The other question is if there really is a guarantee that IMMR ist set to the value of CONFIG_SYS_IMMR. If that was the case, the whole code would make little sense, and I don't think it was written like that just for fun. So please double-check.
The lower part of immr is still used later in the function.
Now if that is the case,, then your modification makes even less sense. If we need the value anyway, why implement two different sources of information? This looks bogus to me.
Since SPRN_IMMR is defined regardless of the subarch, maybe I should have just remove the #ifdef around get_immr()
Indeed that would make more sense, IMO.
Best regards,
Wolfgang Denk