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

[Wolfgang Denk]

Dear Timur Tabi,

In message <4BF68E6A.4070801 at freescale.com> you wrote:
> 
> > Do you really expect me to review all patches submitted on this list
> > in scrupulous detail? 
> 
> No, apparently just mine.

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

> > It would be nice if you also fixed all the other dozen places where
> > such code has crept in.
> 
> Can I do one thing at a time?  I agree that a lot of those could be
> optimized, and if you look at the git log for our stuff, you'll see we've
> been doing that a lot in the past year.  But in general, fixes like these
> usually work better if the board support is already present in your repository.

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.

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.

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.


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?

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


> >>> PCIE1 => pci1 => pcie1_hose
> >>> PCIE2 => pci0 => pcie2_hose
> >>> PCIE3 => pci2 => pcie3_hose
> >>>
> >>> Weird, weird...
> >>
> >> I asked Kumar about it, but he didn't really have much to say, so I left it.
> > 
> > You mean neither of you knows if this is correct or wrong ???
> 
> 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.

> > No, it's not. It is largely the same as mpc8610hpcd_diu.c; common
> > video driver code belongs into drivers/video/
> 
> 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.

> >>>> +#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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To program is to be.