Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dasgupta, Romit had written, on 01/21/2010 01:38 AM, the following:
 arch/arm/mach-omap2/dspbridge.c |   59 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
index 26b860f..120d8a2 100644
--- a/arch/arm/mach-omap2/dspbridge.c
+++ b/arch/arm/mach-omap2/dspbridge.c
@@ -42,7 +42,64 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
 static int get_opp_table(struct dspbridge_platform_data *pdata)
 {
 #ifdef CONFIG_BRIDGE_DVFS
-	/* Do nothing now  - fill based on PM implementation */
+	/* legacy values for 3430 */
+	u32 vdd1_dsp_freq[6][4] = {
+		{0, 0, 0, 0},
+		/*OPP1*/
+		{0, 90000, 0, 86000},
+		/*OPP2*/
+		{0, 180000, 80000, 170000},
+		/*OPP3*/
+		{0, 360000, 160000, 340000},
+		/*OPP4*/
+		{0, 396000, 325000, 376000},
+		/*OPP5*/
+		{0, 430000, 355000, 430000},
+	};
+	struct omap_opp vdd1_rate_table_bridge[] = {
+		{0, 0, 0},
+		/*OPP1*/
+		{S125M, VDD1_OPP1, 0},
+		/*OPP2*/
+		{S250M, VDD1_OPP2, 0},
+		/*OPP3*/
+		{S500M, VDD1_OPP3, 0},
+		/*OPP4*/
+		{S550M, VDD1_OPP4, 0},
+		/*OPP5*/
+		{S600M, VDD1_OPP5, 0},
+	};
+	pdata->dsp_num_speeds = VDD1_OPP5;
Why dont you use ARRAY_SIZE - 1 ?
This is a 1-1 transposition of old code here. further improvement patches is a good thing to have (though I find the usage of ARRAY_SIZE still wont make it cpu independent).

+	pdata->mpu_speeds = kzalloc(sizeof(u32) * pdata->dsp_num_speeds,
+			GFP_KERNEL);
I understand pdata->dsp_num_speeds == pdata->mpu_num_speeds. But don't you think
passing pdata->mpu_num_speeds makes more sense here?
thanks for catching this -> yeah, it is right to use mpu_num_speeds (even though the nums are the same)

+	if (!pdata->mpu_speeds) {
+		pr_err("unable to allocate memory for the mpu"
+		"frequencies\n");
+		return -ENOMEM;
+	}
As I mentioned in my earlier email. You return to the caller here but you free
pdata->dsp_freq_table in the caller even if pdata->dsp_freq_table is not allocated.
yes, that is coz kfree is NULL safe. you can verify that by writing a simple code and testing on checkpatch:
if (ptr)
	kfree(ptr);


+	pdata->dsp_freq_table = kzalloc(
+			sizeof(struct dsp_shm_freq_table) *
+			(pdata->dsp_num_speeds + 1), GFP_KERNEL);
+	if (!pdata->dsp_freq_table) {
+		pr_err("unable to allocate memory for the dsp"
+		"frequencies\n");
+		return -ENOMEM;
+	}
+	for (i = 0; i < 6; i++)
Why are you hard coding numeric 6 here?
carry over of old code - intention is *not* to optimize this.


+		pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
+	pdata->mpu_max_speed = pdata->mpu_speed[VDD1_OPP5];
You can use ARRAY_SIZE.
+	pdata->mpu_min_speed = pdata->mpu_speed[VDD1_OPP1];
+	pdata->dsp_num_speeds = VDD1_OPP5;
Same..ARRAY_SIZE...
intention was to retain the old logic here.
+	for (i = 0; i <= pdata->dsp_num_speeds; i++) {
+		pdata->dsp_freq_table[i].u_volts =
+				vdd1_dsp_freq[i][0];
+		frequency = pdata->dsp_freq_table[i].dsp_freq =
+			frequency = vdd1_dsp_freq[i][1];
+		pdata->dsp_freq_table[i].thresh_min_freq =
+			vdd1_dsp_freq[i][2];
+		pdata->dsp_freq_table[i].thresh_max_freq =
+			vdd1_dsp_freq[i][3];
+	}
 #endif
 	return 0;
 }


--
Regards,
Nishanth Menon
--
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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux