Skip to contentSkip to conversation

cleaner ceil function for uv to vsel conversion

PATCH In review Romit Dasgupta 3 participants 5 messages Jan 11, 2010 – Jan 12, 2010

Subsystems (2)
Arm (32-bit) Arm sub-architectures
lore
Share & tools
lore

Romit Dasgupta

author
#0 lore raw

PATCH 1/1 cleaner ceil function for uv to vsel conversion

Cleaner ceil function.

Signed-off-by Romit Dasgupta <[email protected]>

---

arch/arm/plat-omap/opp_twl_tps.c +1 -8
index e0db39b..1caa414 100644
@@ -36,14 +36,7 @@ unsigned long omap_twl_vsel_to_uv(const u8 vsel)
  */
 u8 omap_twl_uv_to_vsel(unsigned long uv)
 {
-	u8 vsel;
 
-	vsel = ((uv / 100) - 6000) / 125;
+	return (((uv + 99) / 100 - 6000) + 124) / 125;
 
-	/* round off to higher voltage */
-	/* XXX Surely not the best way to handle this. */
-	if (uv > omap_twl_vsel_to_uv(vsel))
-		vsel++;
-
-	return vsel;
 }
#1 lore raw
Quoted 1 line · Romit Dasgupta

Cleaner ceil function.

Needs better subject and better changelog.

Subject should probably be something like: OMAP:

[PATCH] OPP: TWL/TPS: optimize uv to vsel function

And changelog should describe the motiviation for the patch or a description of the problem you're trying to solve. IOW, is this a correctness fix, or an optimization, etc. Looks to me like it's tring to compensate for some rounding issues, but please describe in detail in the changelog.

Thanks,

Kevin

Quoted 22 lines
Signed-off-by Romit Dasgupta <[email protected]>

---

arch/arm/plat-omap/opp_twl_tps.c +1 -8
index e0db39b..1caa414 100644
@@ -36,14 +36,7 @@ unsigned long omap_twl_vsel_to_uv(const u8 vsel)
  */
 u8 omap_twl_uv_to_vsel(unsigned long uv)
 {
-	u8 vsel;
 
-	vsel = ((uv / 100) - 6000) / 125;
+	return (((uv + 99) / 100 - 6000) + 124) / 125;
 
-	/* round off to higher voltage */
-	/* XXX Surely not the best way to handle this. */
-	if (uv > omap_twl_vsel_to_uv(vsel))
-		vsel++;
-
-	return vsel;
 }
author
#2 lore raw
Quoted 4 lines · Kevin Hilman
Quoted 1 line · Romit Dasgupta

Cleaner ceil function.

Quoted 8 lines

Needs better subject and better changelog.

Subject should probably be something like: OMAP:

[PATCH] OPP: TWL/TPS: optimize uv to vsel function

And changelog should describe the motiviation for the patch or a description of the problem you're trying to solve.

The description of the problem is "Cleaner ceil function for uv to vsel conversion".

I think this patch is simple enough for people on this list to understand the optimization. I am sorry I cant be more descriptive.

Quoted 4 lines

IOW, is this a
correctness fix, or an optimization, etc. Looks to me like it's tring to compensate for some rounding issues, but please describe in detail in the changelog.

#3 lore raw

Romit Dasgupta said the following on 01/12/2010 02:58 AM:

Quoted 13 lines
Quoted 11 lines · Kevin Hilman
Quoted 2 lines · Romit Dasgupta

Cleaner ceil function.

Needs better subject and better changelog.

Subject should probably be something like: OMAP:

[PATCH] OPP: TWL/TPS: optimize uv to vsel function

Quoted 10 lines
Quoted 3 lines

And changelog should describe the motiviation for the patch or a description of the problem you're trying to solve.

The description of the problem is "Cleaner ceil function for uv to vsel conversion".

I think this patch is simple enough for people on this list to understand the optimization. I am sorry I cant be more descriptive.

here is a try:
-----

From patchwork Thu Dec 31 13:29:05 2009
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [pm-wip-opp] [PATCH] opp: twl/tps: optimize uv to vsel function
Date: Thu, 31 Dec 2009 13:29:05 -0000
From: Dasgupta, Romit <[email protected]>
X-Patchwork-Id: 70374

For integer values x and y; int div x / y causes truncation. Current omap_twl_uv_to_vsel function implements an equivalent of ceil which is based on an if condition to check truncation and round up. We can do this in a more optimal manner without the if condition. The round up is handled by adding the round off factor prior to truncation as:
(x + (y - 1)) / y

---
Currently putting signed-off in --- section. If Romit is ok, please re-send as needed.

Signed-off-by Dasgupta, Romit <[email protected]>
arch/arm/plat-omap/opp_twl_tps.c +0 -0
index e0db39b..43dee2d 100644
@@ -36,14 +36,7 @@ unsigned long omap_twl_vsel_to_uv(const u8 vsel)
 */

u8 omap_twl_uv_to_vsel(unsigned long uv)
{

-    u8 vsel;
+    /* Takes care of precision loss due to integer division */
+    return (((uv + 99) / 100 - 6000) + 124) / 125;

- vsel = ((uv / 100) - 6000) / 125;
-

-    /* round off to higher voltage */
-    /* XXX Surely not the best way to handle this. */
-    if (uv > omap_twl_vsel_to_uv(vsel))

- vsel++;
-
- return vsel;
}

author
#4 lore raw
Quoted 20 lines
Quoted 7 lines

The description of the problem is "Cleaner ceil function for uv to vsel conversion".

I think this patch is simple enough for people on this list to understand the optimization. I am sorry I cant be more descriptive.

here is a try:
-----

From patchwork Thu Dec 31 13:29:05 2009
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [pm-wip-opp] [PATCH] opp: twl/tps: optimize uv to vsel function
Date: Thu, 31 Dec 2009 13:29:05 -0000
From: Dasgupta, Romit <[email protected]>
X-Patchwork-Id: 70374

Quoted 8 lines

For integer values x and y; int div x / y causes truncation. Current omap_twl_uv_to_vsel function implements an equivalent of ceil which is based on an if condition to check truncation and round up. We can do this in a more optimal manner without the if condition. The round up is handled by adding the round off factor prior to truncation as:
(x + (y - 1)) / y

Wow that is nice... Please send it if you can and add your Signed-off for the work!