On Fri, 2014-03-21 at 22:01 +0100, Olliver Schinagl wrote: > On 03/16/2014 06:34 PM, Ian Campbell wrote: > > Signed-off-by: Ian Campbell <i...@hellion.org.uk> > > --- > > arch/arm/cpu/armv7/sunxi/clock.c | 31 > > +++++++++++++++++++++++-------- > > arch/arm/include/asm/arch-sunxi/clock.h | 8 ++++++-- > > 2 files changed, 29 insertions(+), 10 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/sunxi/clock.c > > b/arch/arm/cpu/armv7/sunxi/clock.c > > index f7eb37b..54d801c 100644 > > --- a/arch/arm/cpu/armv7/sunxi/clock.c > > +++ b/arch/arm/cpu/armv7/sunxi/clock.c > > @@ -21,12 +21,18 @@ static void clock_init_safe(void) > > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > > > /* Set safe defaults until PMU is configured */ > > - writel(AXI_DIV_1 << 0 | AHB_DIV_2 << 4 | APB0_DIV_1 << 8 | > > - CPU_CLK_SRC_OSC24M << 16, &ccm->cpu_ahb_apb0_cfg); > > + writel(AXI_DIV_1 << AXI_DIV_SHIFT | > > + AHB_DIV_2 << AHB_DIV_SHIFT | > > + APB0_DIV_1 << APB0_DIV_SHIFT | > > + CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT, > > + &ccm->cpu_ahb_apb0_cfg); > Is this a pre-patch and should more be done here? I don't think this is > making anything more clear/removing magic values is it?
It is removing the 0, 4, 8 and 16 magic shifts and replacing them with descriptive named fields, which is what was asked for. This is a pretty common idiom in this sort of code. I don't think any more *needs* to be done (maybe you want to, that's up to you). > I probably would have done something like (ignore any stupid mistakes, > i'm a little rusty atm :p) (p.s. I didn't think define names completly > through either, so forgive me there too :) > > #define CPU_AXI_CLK_DIV_RATIO(n) ((((n) - 1) & 0x3) << 0) > #define CPU_ATB_APB_CLK_DIV_RATIO(n) ((((n) - 1) & 0x3) << 2) > /* note to self, isn't there some mathematical way to make this better*/ > #define CPU_AHB_CLK_DIV_RATIO(n) (((n) & 0x3) << 4) > #define __CPU_AHB_CLK_DIV_RATIO_1 0x0 > #define __CPU_AHB_CLK_DIV_RATIO_2 0x1 > #define __CPU_AHB_CLK_DIV_RATIO_4 0x2 > #define __CPU_AHB_CLK_DIV_RATIO_8 0x3 > #define CPU_AHB_CLK_DIV_RATIO_1 \ > CPU_AHB_CLK_DIV_RATIO(__CPU_AHB_CLK_DIV_RATIO_1) > #define CPU_AHB_CLK_DIV_RATIO_2 \ > CPU_AHB_CLK_DIV_RATIO(__CPU_AHB_CLK_DIV_RATIO_2) > #define CPU_AHB_CLK_DIV_RATIO_4 \ > CPU_AHB_CLK_DIV_RATIO(__CPU_AHB_CLK_DIV_RATIO_4) > #define CPU_AHB_CLK_DIV_RATIO_8 \ > CPU_AHB_CLK_DIV_RATIO(__CPU_AHB_CLK_DIV_RATIO_8) > #define CPU_AHB_CLK_SRC(n) ((n) Personally I don't think that is making an improvement. The "VALUE << SHIFT" pattern is well known. Ian. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
