Re: [PATCH][RFC] OMAP3630: Create architecture macros and config entries.

[Kevin Hilman <khilman@deeprootsystems.com>]

"Gadiyar, Anand" <gadiyar@ti.com> writes:

>> >
>> > Hi Allen,
>> >
>> > > -----Original Message-----
>> > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> > > owner@vger.kernel.org] On Behalf Of Pais, Allen
>> > > Sent: Sunday, September 20, 2009 9:47 AM
>> > > To: linux-omap@vger.kernel.org; Raju, Veeramanikandan; Bongale,
>> > Hariprasad
>> > > Subject: [PATCH][RFC] OMAP3630: Create architecture macros and config
>> > > entries.
>> > >
>> > >
>> > > This patch creates the architectural macros for OMAP3630.
>> > >
>> > > Signed-off-by: Allen Pais <allen.pais@ti.com>
>> > >
>> > > arch/arm/mach-omap2/Kconfig                 |   13 ++
>> > > arch/arm/plat-omap/include/mach/cpu.h       |   30 +++++-
>> > >
>> > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>> > > index 75b1c7e..618b7d5 100755
>> > > --- a/arch/arm/mach-omap2/Kconfig
>> > > +++ b/arch/arm/mach-omap2/Kconfig
>> > > @@ -19,11 +19,20 @@ config ARCH_OMAP34XX
>> > >     bool "OMAP34xx Based System"
>> > >     depends on ARCH_OMAP3
>> > >
>> > > +config ARCH_OMAP36XX
>> > > +   bool "OMAP36xx Based System"
>> > > +   depends on ARCH_OMAP3
>> > > +
>> > >  config ARCH_OMAP3430
>> > >     bool "OMAP3430 support"
>> > >     depends on ARCH_OMAP3 && ARCH_OMAP34XX
>> > >     select ARCH_OMAP_OTG
>> > >
>> > > +config ARCH_OMAP3630
>> > > +   bool "OMAP3630 support"
>> > > +   depends on ARCH_OMAP3 && ARCH_OMAP34XX && ARCH_OMAP36XX
>> > > +   select ARCH_OMAP_OTG
>> > > +
>> > >  comment "OMAP Board Type"
>> > >     depends on ARCH_OMAP2 || ARCH_OMAP3 || ARCH_OMAP4
>> > >
>> > > @@ -73,6 +82,10 @@ config MACH_OMAP_3430SDP
>> > >     bool "OMAP 3430 SDP board"
>> > >     depends on ARCH_OMAP3 && ARCH_OMAP34XX
>> > >
>> > > +config MACH_OMAP_3630SDP
>> > > +   bool "OMAP 3630 SDP board"
>> > > +   depends on ARCH_OMAP3 && ARCH_OMAP34XX & ARCH_OMAP36XX
>> > > +
>> > >  config MACH_NOKIA_N8X0
>> > >     bool "Nokia N800/N810"
>> > >     depends on ARCH_OMAP2420
>> > > diff --git a/arch/arm/plat-omap/include/mach/cpu.h b/arch/arm/plat-
>> > > omap/include/mach/cpu.h
>> > > index 7a5f9e8..73c656c 100755
>> > > --- a/arch/arm/plat-omap/include/mach/cpu.h
>> > > +++ b/arch/arm/plat-omap/include/mach/cpu.h
>> > > @@ -157,10 +157,12 @@ IS_OMAP_CLASS(15xx, 0x15)
>> > >  IS_OMAP_CLASS(16xx, 0x16)
>> > >  IS_OMAP_CLASS(24xx, 0x24)
>> > >  IS_OMAP_CLASS(34xx, 0x34)
>> > > +IS_OMAP_CLASS(36xx, 0x36)
>> >
>> > OMAP3630 is "just" an OMAP3430 in disguise.
>> > I don't think it deserves a new class. It should probably be handled like
>> > it was done for 1610 and 1710.
>> >
>> > Theoretically, it should be considered as a 3430 ES4.0, because it is an
>> > OMAP3430 ES3 + couple of bug fixes + couple of improvements.
>> >
>> > I think, that the proposal from Sanjeev to support 35xx
>> > (http://marc.info/?l=linux-omap&m=125050987112798&w=2 ) might be leveraged
>> > to handle 36xx as well.
>> >
>> 
>> I respectfully tend to disagree with this, since there are some components
>> inside the chip that aren't specifically fixes, so IMHO they need to start
>> from scratch about silicon revisions because of that.
>> 
>> If there are many common points between 34xx/35xx/36xx, then rename the
>> reused functions/defines to omap3, instead of omap34xx/omap35xx/omap36xx.
>> 
>> Regards,
>> Sergio
>> 
>
> I agree with Sergio.
>
> While it is definitely possible to write code treating the 3430
> and the 3630 as the same, they are not the same animal. We will
> need to distinguish between the two at more than a few locations
> in code, and we might as well add the ability to do that now.
>
> I see a need to distinguish between 3430 and 3630 in several locations
> - there are changes in hardware IPs that are not reflected in the IP
> revision information (meaning we cannot always go by CPU_HAS_FEATURE() ),
> and we will need some kind of a cpu_is_* check for sure.

And you're sure these HW IP changes require software changes?  Please
provide examples.

So, TI is changing HW IP in a way that requires software changes and
not providing a way for software to detect these changes?

IMHO, This is completely broken HW design.

Kevin