* Nishanth Menon <nm@xxxxxx> [091009 11:47]: > Tony Lindgren had written, on 10/09/2009 01:03 PM, the following: >> * Aguirre Rodriguez, Sergio Alberto <saaguirre@xxxxxx> [091009 08:59]: >>> >>> >>>> -----Original Message----- >>>> From: Menon, Nishanth Sent: Friday, October 09, 2009 10:56 AM >>>> To: linux-omap >>>> Cc: Menon, Nishanth; Chikkature Rajashekar, Madhusudhan; Pandita, >>>> Vikram; Pais, Allen; Gadiyar, Anand; Cousson, Benoit; Felipe Balbi; >>>> Kevin Hilman; Premi, Sanjeev; Shilimkar, Santosh; Aguirre >>>> Rodriguez, Sergio Alberto; Tony Lindgren >>>> Subject: [PATCH v4] OMAP3: introduce OMAP3630 >>>> >>>> **--- SNIP ME -- >>>> Hi, >>>> A bit of history for this patchset >>>> V4 - Sergio's improvement to the handling of rev decision as we just >>>> change the define to 3630 and remove crapy override logic in >>>> omap3_check_revision from v3 patch >>> Now looks much better ;) >>> >>>> V3 - Fixes from Sergio's comments + boot tested on SDP3430+3630. >>>> V2 - fixes of generic comments from Felipe Balbi+minor cleanups >>>> V1 - inital implementation of (a) approach >>>> V0 - original approach introducing a new silicon family >>>> **--- END OF SNIP ME -- >>>> Device intro: >>>> OMAP3630 is the latest in the family of OMAP3 devices >>>> and among the changes it introduces are: >>>> >>>> New OPP levels for new voltage and frequency levels. a bunch of >>>> Bug fixes to various modules feature additions, notably with ISP, >>>> sDMA etc. >>>> >>>> Details about the chip is available here: >>>> http://focus.ti.com/general/docs/wtbu/wtbuproductcontent.tsp?t >>>> emplateId=6123&navigationId=12836&contentId=52606 >>>> >>>> Strategy used: >>>> Strategy to introduce this device into Linux was discussed here: >>>> Ref: http://marc.info/?t=125343303400003&r=1&w=2 >>>> >>>> Two approaches were available: >>>> a) Consider 3630 generation of devices as a new family of silicon >>>> b) Consider 3630 as an offshoot of 3430 family of devices >>>> >>>> As a common consensus, (b) seems to be more valid for 3630 as: >>>> * There are changes which are easily handled by using "FEATURES" >>>> infrastructure. >>>> For details how to do this, see thread: >>>> http://marc.info/?t=125050998500001&r=1&w=2 >>>> * Most of existing 34xx infrastructure can be reused(almost 90%+) >>>> - so no ugly if (cpu_is_omap34xx() || cpu_is_omap36xx()) >>>> all over the place >>>> - lesser chance of bugs due to reuse of proven code flow >>>> - 36xx specific handling can still be done where required >>>> within the existing infrastructure >>>> >>>> NOTE: >>>> * If additional 34xx series are added, OMAP3430_REV_ESXXXX can be >>>> added on top of the existing 3630 ones are renumbered >>>> >>>> This patch was tested on SDP3430, boot tested on 3630 platform using >>>> 3430sdp defconfig >>>> >>>> Signed-off-by: Madhusudhan Chikkature Rajashekar <madhu.cr@xxxxxx> >>>> Signed-off-by: Nishanth Menon <nm@xxxxxx> >>>> Signed-off-by: Vikram Pandita <vikram.pandita@xxxxxx> >>>> Cc: Allen Pais <allen.pais@xxxxxx> >>>> Cc: Anand Gadiyar <gadiyar@xxxxxx> >>>> Cc: Benoit Cousson <b-cousson@xxxxxx> >>>> Cc: Felipe Balbi <felipe.balbi@xxxxxxxxx> >>>> Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx> >>>> Cc: Sanjeev Premi <premi@xxxxxx> >>>> Cc: Santosh Shilimkar <santosh.shilimkar@xxxxxx> >>>> Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@xxxxxx> >>>> Cc: Tony Lindgren <tony@xxxxxxxxxxx> >>> Acked-by: Sergio Aguirre <saaguirre@xxxxxx> >>> >>>> --- >>>> arch/arm/mach-omap2/id.c | 24 ++++++++++++++++++++++-- >>>> arch/arm/plat-omap/include/mach/cpu.h | 6 ++++++ >>>> 2 files changed, 28 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c >>>> index 03b80f2..ee3bb69 100644 >>>> --- a/arch/arm/mach-omap2/id.c >>>> +++ b/arch/arm/mach-omap2/id.c >>>> @@ -210,7 +210,9 @@ void __init omap3_check_revision(void) >>>> hawkeye = (idcode >> 12) & 0xffff; >>>> rev = (idcode >> 28) & 0xff; >>>> - if (hawkeye == 0xb7ae) { >>>> + switch (hawkeye) { >>>> + case 0xb7ae: >>>> + /* Handle 34xx devices */ >>>> switch (rev) { >>>> case 0: >>>> omap_revision = OMAP3430_REV_ES2_0; >>>> @@ -231,8 +233,26 @@ void __init omap3_check_revision(void) >>>> default: >>>> /* Use the latest known revision as default */ >>>> omap_revision = OMAP3430_REV_ES3_1; >>>> - rev_name = "Unknown revision\n"; >>>> + rev_name = "Unknown 34xx revision\n"; >>>> } >>>> + break; >>>> + case 0xb891: >>>> + /* Handle 36xx devices */ >>>> + switch (rev) { >>>> + case 0: >>>> + omap_revision = OMAP3630_REV_ES1_0; >>>> + rev_name = "ES1.0"; >>>> + break; >>>> + default: >>>> + /* Use the latest known revision as default */ >>>> + omap_revision = OMAP3630_REV_ES1_0; >>>> + rev_name = "Unknown 36xx revision\n"; >>>> + } >>>> + break; >>>> + default: >>>> + /* Unknown default to latest rev as default*/ >>>> + omap_revision = OMAP3630_REV_ES1_0; >>>> + rev_name = "Unknown revision\n"; >>>> } >>>> out: >>>> diff --git a/arch/arm/plat-omap/include/mach/cpu.h >>>> b/arch/arm/plat-omap/include/mach/cpu.h >>>> index 431fec4..65f1882 100644 >>>> --- a/arch/arm/plat-omap/include/mach/cpu.h >>>> +++ b/arch/arm/plat-omap/include/mach/cpu.h >>>> @@ -383,6 +383,12 @@ IS_OMAP_TYPE(3430, 0x3430) >>>> #define OMAP3430_REV_ES2_1 0x34302034 >>>> #define OMAP3430_REV_ES3_0 0x34303034 >>>> #define OMAP3430_REV_ES3_1 0x34304034 >>>> +/* NOTE: Add 36xx series below >>>> + * If additional 34xx series are added, OMAP3430_REV_ESXXXX can be >>>> + * added above the 3630 defines and series renumbered to ensure >>>> + * rev() > checks to work >>>> + */ >>>> +#define OMAP3630_REV_ES1_0 0x36301034 >> >> Hmm, to me it looks that this breaks cpu_is_omap34xx() for 36xx, right? >> >> How about build a kernel that boots both on 3430sdp and on some >> 36xx board and make sure things get detected properly by adding >> some debug printk statements? > Tested with the following patch: > diff --git a/arch/arm/mach-omap2/board-3430sdp.c > b/arch/arm/mach-omap2/board-3430sdp.c > index 81aabac..2aac26d 100644 > --- a/arch/arm/mach-omap2/board-3430sdp.c > +++ b/arch/arm/mach-omap2/board-3430sdp.c > @@ -498,6 +498,8 @@ static struct ehci_hcd_omap_platform_data ehci_pdata > __initconst = { > > static void __init omap_3430sdp_init(void) > { > + if (cpu_is_omap34xx()) > + printk(KERN_ERR "_______________------ 34xx detected\n"); > omap3430_i2c_init(); > platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices)); > if (omap_rev() > OMAP3430_REV_ES1_0) > > > and yes, it prints on both 3430 and 3630 platforms (i am hacking at the > moment by using 3430sdpdefconfig for both platforms). > > Here is why it works: > arch/arm/plat-omap/include/mach/cpu.h > cpu_is_omap343x() depends on is_omap34xx() depends on GET_OMAP_CLASS > depends on lower 8 bits of value returned by omap_rev() > omap_rev()(arch/arm/mach-omap2/id.c) returns omap_revision. this means > omap_rev() will return 0x36301034 -> so class is 0x34.. causing > cpu_is_omap343x to be true on 3630 also. Ah, that's right, sorry for being confused. I totally forgot we're using the lower bits for the class.. I guess I need to start reading the code more :) > we might want to consider a follow on patch replacing 0x34 with 0x30 and > changing cpu_is_omap343x with cpu_is_omap3xxx -> at a later point > ofcourse. Yeah if necessary. Let's not do it right now though, we already have quite a pile of clean-up patches coming for the next merge window. Regards, Tony -- 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