Kumar Gala wrote:
On Dec 4, 2006, at 7:13 PM, Tolunay Orkun wrote:
Kumar Gala wrote:
On Dec 4, 2006, at 5:20 PM, Timur Tabi wrote:
Wolfgang Denk wrote:
Are you absolutely sure we will *never* want to make a difference between a MPC8349 and any other type of MPC834x? What is the exact problem you're addressing?
I think Kumar's point is that the code that's correctly marked with CONFIG_MPC8349 is not 8349-specific. It's 834x-specific, and there already is a macro for 834x. If someone were to add support for an 8343 or 8347, they would need to apply Kumar's patch anyway.
*IF* some of this code is really 8349-specific, then the person adding support for the 8343 or 8347 would need to modify this code again. However, I don't think that's going to happen.
This is exactly what I'm saying. The CONFIG_MPC8349 was too specific and really meant CONFIG_MPC834X and thus I changed it. If/when someone's got something that is MPC8349 specific they can re- introduce CONFIG_MPC8349.
The AMCC 4XX based boards we define CONFIG_PPC4XX (for the family) as well as CONFIG_PPC405GP (for example) for specific processor support. The X'd version is used to enable common code while the specific version is used whenever divergences exist from the common code. Right now there might not be a specific difference from MPC8349 code but it might be good to define both MPC8349 and MPC834X for sake of consistency and for future proofing. Without defining the specific version when it is time to introduce that divergence you would wonder what boards were using the MPC8349 and what not (unless the board name gives it away).
The concept is nice, but I think we should add it when its needed. The differences between the MPC834X family of processors are handled by features and not by specifying the specific processor. This is because even on the specific processor you can choose to enable or disable the feature (PCI2, 32/64-bit ddr, etc.).
Well, you can have still have the feature enable/disable. The CONFIG_MPC8349 or whatever would make sure the feature could not be applied on a cpu architecture/platform that is not applicable. Basically it prevents configuration errors.
When someone comes up with a good reason for having a CONFIG_MPC8349 then we should add it, until then I think its likely to get used incorrectly (as it already has been).
What I am saying is that you should rename existing CONFIG_MPC8349 to CONFIG_MPC834X and only on the boards that actually use 8349 define the CONFIG_MPC8349 (in addition, not in lieu of). Of course other boards using other member (8343, 8347) need to have their specific CONFIG_MPC8343 and CONFIG_MPC8347 options defined appropriately as well (again in addition to CONFIG_MPC834X). It is better to do this while the number of boards is small.
Anyway, I just pointed to the existing practice in U-Boot. In the end, it's probably Wolfgang's decision as U-Boot project leader.
Tolunay