[U-Boot-Users] Breakage of board ports on new features.
Tolunay Orkun
listmember at orkun.us
Tue Dec 5 06:15:03 CET 2006
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
More information about the U-Boot
mailing list