Re: [PATCH 02/10] ARM: OMAP: Fix sparse, checkpatch warnings in OMAP2/3 PRCM/PM code

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

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.