| Message ID | 59ac182501e89d3b9ee1dc7c31ce358ff33c0877.1499629706.git.christophe.leroy@c-s.fr |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Dear Christophe, In message <59ac182501e89d3b9ee1dc7c31ce358ff33c0877.1499629706.git.christophe.leroy@c-s.fr> you wrote: > divider is calculated based on SCCR_DFBRG, with: > SCCR_DFBRG 00 => divider 1 = 1 << 0 > SCCR_DFBRG 01 => divider 4 = 1 << 2 > SCCR_DFBRG 10 => divider 16 = 1 << 4 > SCCR_DFBRG 11 => divider 64 = 1 << 6 > > This can be easily converted to a single shift operation: > divider = 1 << (SCCR_DFBRG * 2) Agreed, but... > - switch ((sccr & SCCR_DFBRG11) >> 11) { ... > + uint divider = 1 << ((sccr & SCCR_DFBRG11) >> 10); The code would be easier to read / understand if you made the calculation obvious, i. e. uint divider = 1 << (((sccr & SCCR_DFBRG11) >> 11) * 2); The compiler generates the same code, so there is no size effect. Reviewed-by: Wolfgang Denk <wd@denx.de> Best regards, Wolfgang Denk
Le 12/07/2017 à 14:52, Wolfgang Denk a écrit : > Dear Christophe, > > In message <59ac182501e89d3b9ee1dc7c31ce358ff33c0877.1499629706.git.christophe.leroy@c-s.fr> you wrote: >> divider is calculated based on SCCR_DFBRG, with: >> SCCR_DFBRG 00 => divider 1 = 1 << 0 >> SCCR_DFBRG 01 => divider 4 = 1 << 2 >> SCCR_DFBRG 10 => divider 16 = 1 << 4 >> SCCR_DFBRG 11 => divider 64 = 1 << 6 >> >> This can be easily converted to a single shift operation: >> divider = 1 << (SCCR_DFBRG * 2) > > Agreed, but... > >> - switch ((sccr & SCCR_DFBRG11) >> 11) { > ... >> + uint divider = 1 << ((sccr & SCCR_DFBRG11) >> 10); > > The code would be easier to read / understand if you made the > calculation obvious, i. e. > > uint divider = 1 << (((sccr & SCCR_DFBRG11) >> 11) * 2); > > The compiler generates the same code, so there is no size effect. Ok, done in v2 Christophe > > Reviewed-by: Wolfgang Denk <wd@denx.de> > > > Best regards, > > Wolfgang Denk >
diff --git a/arch/powerpc/cpu/mpc8xx/speed.c b/arch/powerpc/cpu/mpc8xx/speed.c index 8d43efff6c..21a3de317c 100644 --- a/arch/powerpc/cpu/mpc8xx/speed.c +++ b/arch/powerpc/cpu/mpc8xx/speed.c @@ -12,27 +12,6 @@ DECLARE_GLOBAL_DATA_PTR; -void get_brgclk(uint sccr) -{ - uint divider = 0; - - switch ((sccr & SCCR_DFBRG11) >> 11) { - case 0: - divider = 1; - break; - case 1: - divider = 4; - break; - case 2: - divider = 16; - break; - case 3: - divider = 64; - break; - } - gd->arch.brg_clk = gd->cpu_clk / divider; -} - /* * get_clocks() fills in gd->cpu_clock depending on CONFIG_8xx_GCLK_FREQ */ @@ -41,6 +20,8 @@ int get_clocks(void) uint immr = get_immr(0); /* Return full IMMR contents */ immap_t __iomem *immap = (immap_t __iomem *)(immr & 0xFFFF0000); uint sccr = in_be32(&immap->im_clkrst.car_sccr); + uint divider = 1 << ((sccr & SCCR_DFBRG11) >> 10); + /* * If for some reason measuring the gclk frequency won't * work, we return the hardwired value. @@ -57,7 +38,7 @@ int get_clocks(void) gd->bus_clk = gd->cpu_clk / 2; } - get_brgclk(sccr); + gd->arch.brg_clk = gd->cpu_clk / divider; return 0; }
divider is calculated based on SCCR_DFBRG, with: SCCR_DFBRG 00 => divider 1 = 1 << 0 SCCR_DFBRG 01 => divider 4 = 1 << 2 SCCR_DFBRG 10 => divider 16 = 1 << 4 SCCR_DFBRG 11 => divider 64 = 1 << 6 This can be easily converted to a single shift operation: divider = 1 << (SCCR_DFBRG * 2) Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/cpu/mpc8xx/speed.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-)