Hello Russell,
On Sat, 4 Oct 2008, Russell King - ARM Linux wrote:
> On Thu, Oct 02, 2008 at 10:37:50AM -0600, Paul Walmsley wrote:
> > static void omap2_clk_wait_ready(struct clk *clk)
> > {
> > - void __iomem *reg, *other_reg, *st_reg;
> > - u32 bit;
> > -
> > - /*
> > - * REVISIT: This code is pretty ugly. It would be nice to generalize
> > - * it and pull it into struct clk itself somehow.
> > - */
> > - reg = clk->enable_reg;
> > - if ((((u32)reg & 0xff) >= CM_FCLKEN1) &&
> > - (((u32)reg & 0xff) <= OMAP24XX_CM_FCLKEN2))
> > - other_reg = (void __iomem *)(((u32)reg & ~0xf0) | 0x10); /* CM_ICLKEN* */
> > - else if ((((u32)reg & 0xff) >= CM_ICLKEN1) &&
> > - (((u32)reg & 0xff) <= OMAP24XX_CM_ICLKEN4))
> > - other_reg = (void __iomem *)(((u32)reg & ~0xf0) | 0x00); /* CM_FCLKEN* */
> > + u32 bit, other_reg, st_reg;
> > + unsigned long reg;
> > +
> > + reg = (unsigned long)clk->enable_reg;
> > + if (((reg & 0xff) >= CM_FCLKEN1) &&
> > + ((reg & 0xff) <= OMAP24XX_CM_FCLKEN2))
> > + other_reg = ((reg & ~0xf0) | 0x10); /* CM_ICLKEN* */
> > + else if (((reg & 0xff) >= CM_ICLKEN1) &&
> > + ((reg & 0xff) <= OMAP24XX_CM_ICLKEN4))
> > + other_reg = ((reg & ~0xf0) | 0x00); /* CM_FCLKEN* */
> > else
> > return;
> >
> > /* REVISIT: What are the appropriate exclusions for 34XX? */
> > /* No check for DSS or cam clocks */
> > - if (cpu_is_omap24xx() && ((u32)reg & 0x0f) == 0) { /* CM_{F,I}CLKEN1 */
> > + if (cpu_is_omap24xx() && (reg & 0x0f) == 0) { /* CM_{F,I}CLKEN1 */
> > if (clk->enable_bit == OMAP24XX_EN_DSS2_SHIFT ||
> > clk->enable_bit == OMAP24XX_EN_DSS1_SHIFT ||
> > clk->enable_bit == OMAP24XX_EN_CAM_SHIFT)
> > @@ -249,25 +245,25 @@ static void omap2_clk_wait_ready(struct clk *clk)
> > /* REVISIT: What are the appropriate exclusions for 34XX? */
> > /* OMAP3: ignore DSS-mod clocks */
> > if (cpu_is_omap34xx() &&
> > - (((u32)reg & ~0xff) == (u32)OMAP_CM_REGADDR(OMAP3430_DSS_MOD, 0) ||
> > - ((((u32)reg & ~0xff) == (u32)OMAP_CM_REGADDR(CORE_MOD, 0)) &&
> > - clk->enable_bit == OMAP3430_EN_SSI_SHIFT)))
> > + ((reg & ~0xff) == (u32)OMAP_CM_REGADDR(OMAP3430_DSS_MOD, 0) ||
> > + (((reg & ~0xff) == (u32)OMAP_CM_REGADDR(CORE_MOD, 0)) &&
> > + clk->enable_bit == OMAP3430_EN_SSI_SHIFT)))
> > return;
> >
> > /* Check if both functional and interface clocks
> > * are running. */
> > bit = 1 << clk->enable_bit;
> > - if (!(__raw_readl(other_reg) & bit))
> > + if (!(__raw_readl((void __iomem *)other_reg) & bit))
>
> This isn't an improvement. Moving casts to the readl/writel macros is
> a definite no no.
OK. Tony's tree contains some patches which thoroughly clean this
function up, but they are large. Would you like me to send those along
with this patch set, or would you prefer that I just drop this part of the
patch and repost?
- Paul
--
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
