> -----Original Message-----
> From: Felipe Balbi [mailto:felipe.ba...@nokia.com]
> Sent: Wednesday, October 28, 2009 2:39 AM
> To: Premi, Sanjeev
> Cc: Balbi Felipe (Nokia-D/Helsinki);
> linux-omap@vger.kernel.org; t...@atomide.com
> 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.ba...@nokia.com>
> 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.ba...@nokia.com>
> ---
>
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
