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.
The rest of the patch is fine.
--
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
