Hi
a comment:
On Wed, 21 Sep 2011, jean.pihet@xxxxxxxxxxxxxx wrote:
> From: Jean Pihet <j-pihet@xxxxxx>
>
> Hwmod is queried from the OMAP_PM layer to manage the power domains
> wake-up latency constraints. Hwmod retrieves the correct power domain
> and if it exists it calls the corresponding power domain function.
>
> Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
> latency constraints on MPU, CORE and PER.
>
> Signed-off-by: Jean Pihet <j-pihet@xxxxxx>
> ---
> arch/arm/mach-omap2/omap_hwmod.c | 26 +++++++++++++++++++++++++-
> arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 ++
> 2 files changed, 27 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 84cc0bd..c6b1cc9 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2618,11 +2619,34 @@ ohsps_unlock:
> return ret;
> }
>
> +/*
> + * omap_hwmod_set_wkup_constraint- set/release a wake-up latency constraint
> + *
> + * @oh: struct omap_hwmod* to which the target device belongs to.
> + * @cookie: identifier of the constraints list for @oh.
> + * @min_latency: the minimum allowed wake-up latency for @oh.
> + *
> + * Returns 0 upon success.
> + */
It's good that there is some documentation here, but it would be better if
it were kerneldoc-style documentation. Please see
Documentation/kernel-doc-nano-HOWTO.txt.
Also it would be good to have a bit more documentation here beyond simply
"Returns 0 upon success." For example, how should a caller remove a
wakeup latency constraint? Also, this function can return -EINVAL, not
counting whatever pwrdm_set_wkup_lat_constraint() can return, so that
should be documented above also.
This applies to the function comments in the rest of the patches too.
Some of them have quite good kerneldoc comments, such as
pwrdm_wakeuplat_update_pwrst(); others need some work, like
pwrdm_set_wkup_lat_constraint().
> +int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh,
> + void *cookie, long min_latency)
> +{
> + struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
> +
> + if (!pwrdm) {
> + pr_err("%s: Error: could not find powerdomain "
> + "for %s\n", __func__, oh->name);
> + return -EINVAL;
> + }
> +
> + return pwrdm_set_wkup_lat_constraint(pwrdm, cookie, min_latency);
> +}
> +
> /**
> * omap_hwmod_get_context_loss_count - get lost context count
> * @oh: struct omap_hwmod *
> *
> - * Query the powerdomain of of @oh to get the context loss
> + * Query the powerdomain of @oh to get the context loss
> * count for this device.
> *
> * Returns the context loss count of the powerdomain assocated with @oh
> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> index 0e329ca..75e0e7a 100644
> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> @@ -603,6 +603,8 @@ int omap_hwmod_for_each_by_class(const char *classname,
> void *user);
>
> int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
> +int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh, void *cookie,
> + long min_latency);
> u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
>
> int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
> --
> 1.7.4.1
>
- 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
