[U-Boot] [PATCH v2 07/10] powerpc: mpc8xx: get rid of get_immr()

[Wolfgang Denk]

Dear Christophe,

In message <622b8aec162cc43e774bde5da990a61fc961b4d9.1519976944.git.christophe.leroy at 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.

If this is necessary / intentional, it should be a separate commit
with proper explanation.


> -	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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
About the use of language: it is impossible to sharpen a pencil  with
a  blunt  ax.  It is equally vain to try to do it with ten blunt axes
instead.                                           -- Edsger Dijkstra