Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Kevin Hilman <khilman@deeprootsystems.com>]

Thara Gopinath <thara@ti.com> writes:

> Some clock nodes like wdt1_fck, sr1_fck, sr2_fck etc do not have a
> clock domain associated . For such nodes accessing the clock domain pointers 
> in _add_initiator_dep and _del_initiator_dep, will lead to null pointer
> defreferencing crash. Adding support in these functions to check for
> existence of clkdm pointer before trying to acess it. Even if tomorrow
> we correct all the clock nodes to have an associated clock domain, checking
> for the existence of the pointer is a good programming practice.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
> This patch depends on http://patchwork.kernel.org/patch/63383/

I think clocks without associated clockdomains should be handled with
a flag instead of a check for a NULL pointer.

Your current approach will silently fail if someone forgets to add
a clockdomain to a clock that should have one.

Kevin


>  arch/arm/mach-omap2/omap_hwmod.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 18e6478..3edc387 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -314,8 +314,10 @@ static int _add_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
>  	if (!oh->_clk)
>  		return -EINVAL;
>  
> -	return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
> +	if (oh->_clk->clkdm)
> +		return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>  				  init_oh->_clk->clkdm->pwrdm.ptr);
> +	return 0;
>  }
>  
>  /**
> @@ -335,8 +337,10 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
>  	if (!oh->_clk)
>  		return -EINVAL;
>  
> -	return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
> +	if (oh->_clk->clkdm)
> +		return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>  				  init_oh->_clk->clkdm->pwrdm.ptr);
> +	return 0;
>  }
>  
>  /**
> -- 
> 1.5.4.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html