On Tue, 5 Jul 2011, Tony Lindgren wrote: > * Raphaël Assénat <raph@xxxxxx> [110705 07:30]: > > On 05/07/11 07:19 AM, Tony Lindgren wrote: > > > * Raphaël Assénat <raph@xxxxxx> [110704 12:51]: > > >> > > >> The am3505 is apparently so similar to the 3430 that it was treated as such > > >> (omap_chip.oc was being set to CHIP_IS_OMAP3430ES3_1). There are however a > > >> few differences that need to be addressed. I have therefore created a new > > >> CHIP_IS and patched clocks, hwmod and power management related files > > >> consequently. My system now boots until it complains that it is unable > > >> to mount its root filesystem. > > > > > > Can you please describe where you need CHIP_IS for am3505? It seems that > > > your patches just enable the same CHIP_IS_OMAP3430 features for am3505 too? > > > > Actually it's only needed for the 3505/3517 specific UART 4 which should > > not be registered on real OMAP3430's. (see struct omap_hwmod > > am35xx_uart4_hwmod; in omap_hwmod_3xxx_data.c) > > > > But there are also cases where OMAP3430 hwmods must not be registered > > on AM35xx. For instance, omap3xxx_timer12_hwmod since timer12 does > > not exist general purpose AM3505's. > > Sounds like we should be able to handle both uart and gptimer12 the same way > as we'll be handling the hwmod reset for special cases like gptimer12 for > secure mode. So addding Paul to Cc. So the idea would be to mark those devices as unavailable from the board file? It sounds possible, but AM35xx also has some devices that are not present on 34xx chips, like the VPFE, EMAC, and HECC. And there are probably some other devices that are present on 34xx that are not present on AM35xx, like SSI, MSPRO, D2D, etc. I'd suggest adding a separate CHIP_IS_* for those AM35xx chips, as it sounds like Raphaël has done, since they are different silicon, unlike the 34xx HS and GP chips, which just have some eFuse differences. ... This is a bit of a side issue, but right now, the CHIP_IS_* macros are kind of messed up for OMAP3. That CHIP_IS_OMAP3430 macro is the main problem. The different CHIP_IS_OMAP* bits were intended to be mutually exclusive, so only one bit would be set at runtime for a given SoC. Then superset definitions would be defined as combinations of those bits, for use with data structures that are common to multiple OMAP3 SoCs. We do this the right way with OMAP2 and OMAP4, just not with OMAP3. I have a patch for this, but part of it involves renaming some CHIP_IS_OMAP3430 to CHIP_IS_OMAP3XXX, so I've been hesitant to post it -- sounds like that would be grounds for flaming? Or maybe it can be posted as a 'cleanup' patch for the 3.2 time frame? > Basically we can have a am35xx specific arch_initcall that sets the special > flags for these devices like noreset, disabled or unavailable. > > In this case we would just set some devices as unavailable on am35xx. > > > > I'd rather see us improve the code so we can support am3505 properly without > > > a need to patch all over the place.. > > Maybe instead of having one big array of hwmods registered for > > all CPUs in omap_hwmod_3xxx_data we could have a separate > > one for am35xx (and maybe, eventually others) such as: > > > > static __initdata struct omap_hwmod *omap3xxx_hwmods[] > > static __initdata struct omap_hwmod *am35xx_hwmods[] > > > > ...and maybe also a 'common' array. > > > > The init function would then register the appropriate hwmod array(s). The only > > thing I don't like is that for this to work we would have to keep setting > > CHIP_IS_OMAP3430 even on AM35xx CPUs. But maybe we could change the behaviour of > > omap_hwmod_register to trust the caller and accept the hwmods without looking > > at the omap_chip member. By doing this I think we could drop all the > > .omap_chip = OMAP_CHIP_INIT(...) instances. For what it's worth, there are some patches here that are 3.2 material that hopefully will make it easier to share hwmod data between different chips with large portions that are similar. > > Otherwise I think the current approach, despite the size of the initial patch, > > has at least the benefit of being explicit and less confusing than (ab)using the > > CHIP_IS from a different CPU. > > Well let's see how far we can get with passing the special case flags with > omap2_init_devices(). Initially we can call that from the board-*.c file > that should get your board booting. Then we can add the am35xx specific > arch_initcall. - Paul