Hi Rajendra
one brief comment below. Also, when you post an updated version, could
you cc the linux-arm-kernel mailing list? That way I won't have to do
it...
On Fri, 28 Jan 2011, Rajendra Nayak wrote:
> The _add_optional_clock_alias function expects an entry
> already existing in the clkdev table in the form of
> <dev-id=NULL, con-id=role> which might not be the case
> always.
>
> Instead, just check if an entry already exists in clkdev
> in the <dev-id=dev_name, con-id=role> form, else go ahead
> and add one.
>
> Remove any assumption of an entry already existing in clkdev
> table in any form.
>
> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
> Reported-by: Sumit Semwal <sumit.semwal@xxxxxx>
> ---
> arch/arm/plat-omap/omap_device.c | 25 +++++++++++++++++++------
> 1 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 57adb27..80d4f35 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -83,9 +83,11 @@
> #include <linux/err.h>
> #include <linux/io.h>
> #include <linux/clk.h>
> +#include <linux/clkdev.h>
>
> #include <plat/omap_device.h>
> #include <plat/omap_hwmod.h>
> +#include <plat/clock.h>
>
> /* These parameters are passed to _omap_device_{de,}activate() */
> #define USE_WAKEUP_LAT 0
> @@ -244,7 +246,7 @@ static inline struct omap_device *_find_by_pdev(struct platform_device *pdev)
> *
> * For every optional clock present per hwmod per omap_device, this function
> * adds an entry in the clocks list of the form <dev-id=dev_name, con-id=role>
> - * if an entry is already present in it with the form <dev-id=NULL, con-id=role>
> + * if it does not exist already.
> *
> * The function is called from inside omap_device_build_ss(), after
> * omap_device_register.
> @@ -258,21 +260,32 @@ static void _add_optional_clock_alias(struct omap_device *od,
> struct omap_hwmod *oh)
> {
> int i;
> + struct clk_lookup *l;
> + struct omap_hwmod_opt_clk *oc;
>
> for (i = 0; i < oh->opt_clks_cnt; i++) {
> - struct omap_hwmod_opt_clk *oc;
> - int r;
> + struct clk *r;
>
> oc = &oh->opt_clks[i];
>
> if (!oc->_clk)
> continue;
>
> - r = clk_add_alias(oc->role, dev_name(&od->pdev.dev),
> - (char *)oc->clk, &od->pdev.dev);
> - if (r)
> + r = clk_get_sys(dev_name(&od->pdev.dev), oc->role);
> + if (!IS_ERR(r))
> + continue; /* clkdev entry exists */
> +
> + r = omap_clk_get_by_name((char *)oc->clk);
> + if (IS_ERR(r)) {
> pr_err("omap_device: %s: clk_add_alias for %s failed\n",
> dev_name(&od->pdev.dev), oc->role);
> + continue;
> + }
> +
> + l = clkdev_alloc(r, oc->role, dev_name(&od->pdev.dev));
> + if (!l)
> + return;
Please add a pr_err() in this case, similar to the above one; that way
in the unlikely event that this fails, there will be some sign of what is
happening... other than that, it looks good to me
> + clkdev_add(l);
> }
> }
>
> --
> 1.7.0.4
>
> --
> 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
>
- 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
