> -----Original Message----- > From: Felipe Balbi [mailto:felipe.balbi@xxxxxxxxx] > Sent: Wednesday, October 28, 2009 2:39 AM > To: Premi, Sanjeev > Cc: Balbi Felipe (Nokia-D/Helsinki); > linux-omap@xxxxxxxxxxxxxxx; tony@xxxxxxxxxxx > Subject: Re: [PATCH 1/2] AM35xx: Runtime detection of the device > > Hi, > > On Tue, Oct 27, 2009 at 07:08:22PM +0100, ext Premi, Sanjeev wrote: > > > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > > > > index 1c15112..87efb73 100644 > > > > --- a/arch/arm/mach-omap2/id.c > > > > +++ b/arch/arm/mach-omap2/id.c > > > > @@ -242,6 +242,21 @@ void __init omap3_check_revision(void) > > > > omap_revision = OMAP3630_REV_ES1_0; > > > > } > > > > break; > > > > + case 0xb868: > > > > + /* Handle OMAP35xx/AM35xx devices > > > > + * > > > > + * Set the device to be OMAP3517 here. > Actual device > > > > + * is identified later based on the features. > > > > + */ > > > > + switch (rev) { > > > > + case 0: > > > > + omap_revision = OMAP3505_REV(rev); > > > > + break; > > > > + default: > > > > + /* Use the latest known > revision as default */ > > > > + omap_revision = OMAP3505_REV(rev); > > > > > > if both are the same, what's the point of having this switch ? > > > > [sp] I was just following the style for 3630, while re-basing > > this patch :( > > I see, but that's clearly bogus (in a sense), then if you come up with > another version of the chip, there will be two place to be > fixed. Tony, > what do you think about applying the following cleanup patch to id.c ? > > From: Felipe Balbi <felipe.balbi@xxxxxxxxx> > Subject: [PATCH] arm: omap: code cleanup to id.c > > Cleanup the coding style in id.c while avoiding unneeded switch() > statements. > > Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxx> > --- > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > index 1c15112..dbdeb09 100644 > --- a/arch/arm/mach-omap2/id.c > +++ b/arch/arm/mach-omap2/id.c > @@ -53,11 +53,11 @@ int omap_type(void) > { > u32 val = 0; > > - if (cpu_is_omap24xx()) > + if (cpu_is_omap24xx()) { > val = omap_ctrl_readl(OMAP24XX_CONTROL_STATUS); > - else if (cpu_is_omap34xx()) > + } else if (cpu_is_omap34xx()) { > val = omap_ctrl_readl(OMAP343X_CONTROL_STATUS); > - else { > + } else { > pr_err("Cannot detect omap type!\n"); > goto out; > } > @@ -224,24 +224,14 @@ void __init omap3_check_revision(void) > omap_revision = OMAP3430_REV_ES3_0; > break; > case 4: > - omap_revision = OMAP3430_REV_ES3_1; > - break; > + /* FALLTHROUGH */ > default: > /* Use the latest known revision as default */ > omap_revision = OMAP3430_REV_ES3_1; > } > break; > case 0xb891: > - /* Handle 36xx devices */ > - switch (rev) { > - case 0: > - omap_revision = OMAP3630_REV_ES1_0; > - break; > - default: > - /* Use the latest known revision as default */ > - omap_revision = OMAP3630_REV_ES1_0; > - } > - break; > + /* FALLTHROUGH */ > default: > /* Unknown default to latest silicon rev as default*/ > omap_revision = OMAP3630_REV_ES1_0; [sp] Haven't applied the patch. But, if FALLTHROUGH will make the device detected as OMAP3630, then it may not be right. The fall through should be on most common device. OMAP3430 ES21./3.1 should be ideal. Thoughts? ~sanjeev > @@ -266,19 +256,17 @@ void __init omap3_cpuinfo(void) > * on available features. Upon detection, update the CPU id > * and CPU class bits. > */ > - if (cpu_is_omap3630()) > + if (cpu_is_omap3630()) { > strcpy(cpu_name, "3630"); > - else if (omap3_has_iva() && omap3_has_sgx()) > + } else if (omap3_has_iva() && omap3_has_sgx()) { > strcpy(cpu_name, "3430/3530"); > - else if (omap3_has_sgx()) { > + } else if (omap3_has_sgx()) { > omap_revision = OMAP3525_REV(rev); > strcpy(cpu_name, "3525"); > - } > - else if (omap3_has_iva()) { > + } else if (omap3_has_iva()) { > omap_revision = OMAP3515_REV(rev); > strcpy(cpu_name, "3515"); > - } > - else { > + } else { > omap_revision = OMAP3503_REV(rev); > strcpy(cpu_name, "3503"); > } > @@ -297,8 +285,7 @@ void __init omap3_cpuinfo(void) > strcpy(cpu_rev, "3.0"); > break; > case OMAP_REVBITS_40: > - strcpy(cpu_rev, "3.1"); > - break; > + /* FALLTHROUGH */ > default: > /* Use the latest known revision as default */ > strcpy(cpu_rev, "3.1"); > @@ -325,18 +312,18 @@ void __init omap2_check_revision(void) > * At this point we have an idea about the processor > revision set > * earlier with omap2_set_globals_tap(). > */ > - if (cpu_is_omap24xx()) > + if (cpu_is_omap24xx()) { > omap24xx_check_revision(); > - else if (cpu_is_omap34xx()) { > + } else if (cpu_is_omap34xx()) { > omap3_check_features(); > omap3_check_revision(); > omap3_cpuinfo(); > - } > - else if (cpu_is_omap44xx()) { > + } else if (cpu_is_omap44xx()) { > printk(KERN_INFO "FIXME: CPU revision = OMAP4430\n"); > return; > - } else > + } else { > pr_err("OMAP revision unknown, please fix!\n"); > + } > > /* > * OK, now we know the exact revision. Initialize omap_chip bits > > -- > balbi > > -- 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
