[U-Boot] [PATCH] powerpc: add support for the Freescale P1022DS reference board

Timur Tabi
2010-05-21T14:33:43Z

Wolfgang Denk wrote:

> Unless you pay me for this (or FSL) you should not even rely on this.

Heh.

> Well, there are things and things.
>
> When we discover that crap has piled up here and there, it's a good
> idea to perform a clean up.

I agree 100%.

> When the first one adds some feature or new architecture or similar,
> it is pretty normal that he cannot yet always decide correctly what
> needs to go where, or which parts are specific to CPU or board and
> which are common. But when more boards get added, this usually becomes
> clear pretty quickly.

Ok.

> It has always been more efficient to prevent code that is known to
> need such cleanup to prevent from going into mainline in the first
> place, than to allow it to go in and hope that somebody, sometime will
> find time and resources and zest for a clean-up.

I'm not sure I agree with that completely. In many cases, it's more
efficient to work on tasks in bite-size chunks.

> Is it easier for you to convince your manager that you need N more
> hours to get that code accepted for mainline, or to get allownce for
> N hours to perform some cleanup in U-Boot that is not related to your
> current project?

Definitely the latter. Cleaning up U-Boot (and Linux) code so that it's
overall easier to maintain and more efficient is one of the primary tasks of
my department.

> [I have somemore tasks at hand if the latter should be the case :-) ]

Please, send me a list. We have an internal bug tracking system, and I
would be more than happy to log all the items that concern you (as long as
they affect Freescale PowerPC products in some way).

>> I guess I should clarify. Kumar said it was based on the order of the
>> addresses of the PCI devices within CCSR space, but he didn't offer any
>> insight as to whether it should be fixed or how.
>
> So please check this again. It looks broken to me.

Ok.

>> Ok, will you allow me to merge those two functions into a single driver at a
>> later time, when I have the opportunity to examine the code and test it on
>> both boards?
>
> Please let's do it now, instead of adding code that is known to need
> fixing. See above for my reasoning.

And see my above response for why I'd prefer not to solve this now.

I'll try to fix it anyway, though.

>>>>>> +#ifndef __ASSEMBLY__
>>>>>> +extern unsigned long calculate_board_sys_clk(void);
>>>>>> +extern unsigned long calculate_board_ddr_clk(void);
>>>>>> +#endif
>>>>>
>>>>> Please move to appropriate header file.
> ...
>> Well, this particular change would probably need to be made later, because I
>> would probably need to change all the board header files at once. It's not
>> trivial.
>
> What exactly is not trivial in moving some prototype declarations to
> another header file? Please elucidate. The most difficult part is
> probably determining where they actually belong to.

Well, I'll take a closer look, but my concern is that there is no good
header file to move these into that wouldn't require modifying dozens of
files. Since I'm trying to add support for one particular board, I don't
want to modify every other board in the same patch set.

I'll try to fix it anyway, though.

--
Timur Tabi
Linux kernel developer at Freescale