From: Wolfgang Denk Date: Sun, 04 Mar 2018 15:51:04 +0100 Subject: [U-Boot] [PATCH v2 07/10] powerpc: mpc8xx: get rid of get_immr() In-Reply-To: <71a3900b-f61e-e9f8-c12a-5ec5aa1420bc@c-s.fr> References: <622b8aec162cc43e774bde5da990a61fc961b4d9.1519976944.git.christophe.leroy@c-s.fr> <20180303164628.026692401B3@gemini.denx.de> <71a3900b-f61e-e9f8-c12a-5ec5aa1420bc@c-s.fr> Message-ID: <20180304145104.4DBF1240112@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 -- 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 Ninety-Ninety Rule of Project Schedules: The first ninety percent of the task takes ninety percent of the time, and the last ten percent takes the other ninety percent.