Re: [PATCH] OMAP3: PM: Fix VDD2 OPP1 issue

[Kevin Hilman <khilman@deeprootsystems.com>]

"Reddy, Teerth" <teerth@ti.com> writes:

> From 144669d941a432875db37ae9431847f6753e566e Mon Sep 17 00:00:00 2001
> From: Teerth Reddy <teerth@ti.com>
> Date: Wed, 9 Sep 2009 11:01:04 +0530
> Subject: [PATCH] ARM: OMAP3: PM: Fix VDD2 OPP1 issue
>
> This patch fixes the VDD2 OPP1 issue. The patch has change
> which does not allow VDD2 OPP setting to 1.VDD2 should not be put 
> at OPP1 as this is not a supported OPP for VDD2

Patch looks fine, but shortlog (subject) and changelog should be more
clear.  These should be written for people who are not as familiar
with the code.  For example, seeing this shortlog in a git history
would not be helpful as the irst thing one would ask is "what OPP1
issue?"

How about something like this:

Subject: OMAP3: PM: do not allow OPP1 allow for VDD2

Since OPP1 is not a supported OPP for VDD2, do not allow it to be
changed using the sysfs interface.

Kevin


> Signed-off-by: Teerth Reddy <teerth@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index fec7d00..d0e03c4 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -195,7 +195,7 @@ static ssize_t vdd_opp_store(struct kobject *kobj, struct kobj_attribute *attr,
>  		}
>  		resource_set_opp_level(VDD1_OPP, value, flags);
>  	} else if (attr == &vdd2_opp_attr) {
> -		if (value < 1 || value > 3) {
> +		if (value < 2 || value > 3) {
>  			printk(KERN_ERR "vdd_opp_store: Invalid value\n");
>  			return -EINVAL;
>  		}
> -- 
> 1.5.4.7