do we need CHIP_GE_OMAP3630ES1in .oc? (was Re: [PATCH 02/19] omap3630: hwmod: sr: enable for higher ES)

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

 



Nishanth Menon wrote, on 02/20/2011 10:56 AM:
Vishwanath Sripathy wrote, on 02/19/2011 06:52 PM:
-----Original Message-----
From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
owner@xxxxxxxxxxxxxxx] On Behalf Of Nishanth Menon
Sent: Saturday, February 19, 2011 5:32 PM
To: linux-omap
Cc: Tony Lindgren; Kevin Hilman; Nishanth Menon
Subject: [PATCH 02/19] omap3630: hwmod: sr: enable for higher ES

Enable hwmod entries for OMAP3630 for higher ES revisions as
well. This is to ensure that SR can be used in all revisions of
OMAP3630 as of this posting.

Signed-off-by: Nishanth Menon<nm@xxxxxx>
---
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index ea1f49a..bbcbea6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1318,7 +1318,9 @@ static struct omap_hwmod
omap36xx_sr1_hwmod = {
},
.slaves = omap3_sr1_slaves,
.slaves_cnt = ARRAY_SIZE(omap3_sr1_slaves),
- .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
+ .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1 |
+ CHIP_IS_OMAP3630ES1_1 |
+ CHIP_IS_OMAP3630ES1_2),
What's the need of this?
Here is the code snippet from id.c
-----snip-----
case 0xb891:
/* Handle 36xx devices */
omap_chip.oc |= CHIP_IS_OMAP3630ES1;

switch(rev) {
case 0: /* Take care of early samples */
omap_revision = OMAP3630_REV_ES1_0;
break;
case 1:
omap_revision = OMAP3630_REV_ES1_1;
omap_chip.oc |= CHIP_IS_OMAP3630ES1_1;
break;
case 2:
default:
omap_revision = OMAP3630_REV_ES1_2;
omap_chip.oc |= CHIP_IS_OMAP3630ES1_2;
}
So it's clear that omap_chip.oc will have CHIP_IS_OMAP3630ES1 anyway on
all ES versions.

Hmm... thanks for picking this up, but this is inconsistent and
confusing implementation in id.c. So the way CHIP_IS_OMAP3630ES1 behaves
is entirely different from CHIP_IS_OMAP3630ES1_1 or
CHIP_IS_OMAP3630ES1_2 or any of the OMAP3430ES* which will only be set
for the specific rev of the silicon.

I can post a patch later on to replace CHIP_IS_OMAP3630ES1 with
CHIP_IS_OMAP3630ES_ALL which allows the code to be readable and make
CHIP_IS_OMAP3630ES1 mean precisely what it should have meant like the
rest of the ESs - only for ES1.0.

Better might be to introduce and use CHIP_GE_OMAP3630ES1 in hwmod structs in that case - that should ease things up. but the ES1 story should be fixed I guess.


What do people think of this?


+ Anand from commit b0a1a6ce0597662c06f970643da60b8ebb5cdd1c which introduced the code in id.c to hear his views as well.


--
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