On Thu, Oct 02, 2008 at 10:37:52AM -0600, Paul Walmsley wrote: > From: Tony Lindgren <tony@xxxxxxxxxxx> > > Remove OMAP_PRM_REGADDR and OMAP_CM_REGADDR. Use > prm_read/write_mod_reg() or cm_read/write_mod_reg() instead. For > assembly, use OMAPXXXX_PRM_REGADDR or OMAPXXXX_CM_REGADDR macros. I do have concerns about this patch as well - hating those casts that are required to store an offset in 'enable_reg', which then have to be un-casted to add the correct base address. I've been trying to work out if there's a better way to do this using the existing structures. How about: struct clk { ... void __iomem *base; struct clk *other_clk; /* associated func/interface clock */ u16 enable_offset; u16 idlest_offset; u16 clksel_offset; u8 enable_bit; u8 idlest_bit; ... int (*enable)(struct clk *); void (*disable)(struct clk *); ... }; (or use u8 if the offsets always fit.) The setup of apll96_ck becomes: static struct clk apll96_ck = { .name = "apll96_ck", .parent = &sys_ck, .rate = 96000000, .flags = CLOCK_IN_OMAP242X | CLOCK_IN_OMAP243X | RATE_FIXED | RATE_PROPAGATES | ENABLE_ON_INIT, .clkdm_name = "wkup_clkdm", .enable_offset = CM_REG_OFFSET(PLL_MOD, CM_CLKEN), .idlest_offset = CM_REG_OFFSET(PLL_MOD, CM_IDLEST), .enable_bit = OFFSET_OF_BIT(EN_APLL_LOCKED << OMAP24XX_EN_96M_PLL_SHIFT), .idlest_bit = OFFSET_OF_BIT(OMAP24XX_ST_96M_APLL), .enable = clkll_enable32_and_wait, .disable = clkll_disable32, .recalc = &propagate_rate, }; (replacing OFFSET_OF_BIT() with the appropriate real bit position number). You register all such clocks thusly: void clk_register_offset_clks(void __iomem *base, struct clk **clks, size_t num) { size_t i; for (i = 0; i < num; i++) { struct clk *clk = clks[i]; clk->base = base; clk_register(clk); } } static struct clk cm_clks[] = { &apll96_ck, }; clk_register_offset_clks(cm_base, cm_clks, ARRAY_SIZE(cm_clks); and, eg, the accesses to the enable register become: int clkll_enable32(struct clk *clk) { u32 val, mask = 1 << clk->enable_bit; val = __raw_readl(clk->base + clk->enable_offset); if (clk->flags & INVERT_ENABLE) val &= ~mask; else val |= mask; __raw_writel(val, clk->base + clk->enable_offset); wmb(); return 0; } void clkll_disable32(struct clk *clk) { u32 val, mask = 1 << clk->enable_bit; val = __raw_readl(clk->base + clk->enable_offset); if (clk->flags & INVERT_ENABLE) val |= mask; else val &= ~mask; __raw_writel(val, clk->base + clk->enable_offset); wmb(); } int clkll_is_running32(struct clk *clk) { u32 val, mask = 1 << clk->enable_bit; val = __raw_readl(clk->base + clk->enable_offset) & mask; return clk->flags & INVERT_ENABLE ? !val : !!val; } int clkll_enable32_and_wait(struct clk *clk) { u32 mask, result, val; unsigned int tries = 0; if (!clkll_is_running32(clk)) clkll_enable32(clk); /* check if other clock, if any, is running */ if (clk->other_clk && !clkll_is_running32(clk->other_clk)) return 0; val = mask = 1 << clk->idlest_bit; if (inverted_on_this_cpu) val = 0; for (tries = 0; tries < MAX_CLOCK_ENABLE_WAIT; tries++, udelay(1)) { result = __raw_readl(clk->base + clk->idlest_offset) & mask; if (result == val) break; } if (result == val) pr_debug("Clock %s stable after %uus\n", clk->name, tries); else pr_err("Clock %s failed to stablize after %uus\n", clk->name, tries); return result == val ? 0 : -ETIMEDOUT; } Then, you use clkll_enable32() if you don't need to wait for the clock to stablize, or clkll_enable32_and_wait() if you do. If you need 16-bit, which I think I've seen, obviously create the corresponding versions. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html