[U-Boot] [PATCH v3] powerpc: Restore core of mpc8xx

Thu Jun 22 11:20:00 UTC 2017

Le 22/06/2017 à 11:59, Wolfgang Denk a écrit :
> Dear Christophe,
> 
> In message <0784ad6e-86ab-9c1d-1b81-a5cacaf2b01f at c-s.fr> you wrote:
>>
>>> Please see my previous comments to Tom's message.  At least, please
>>> clean up the directory structure and get rid of unrelated (4xx) and
>>> untested/broken code (bedbug, probably kgdb, pcmcia, usb).
>>
>> I got 4xx unrelated stuff out in v3, as mentioned above.
> 
> I still see these files in your patch:
> 
> 	drivers/net/4xx_enet.c
> 	include/configs/CPCI4052.h
> 	include/configs/MIP405.h
> 	include/configs/PIP405.h
> 	include/configs/PLU405.h

Those are

#undef	CONFIG_IDE_8xx_DIRECT

My patch brings back CONFIG_IDE_8xx_DIRECT, so if somebody has decided 
that this CONFIG_ shall explicitly be undefined for 4xx targets, I 
believe there is a reason and I cannot decide by myself to remove that.

> 
> To me these look very much like PPC4xx related.  In addition, it
> makes zero sense to keep PPC405 board config files without any
> related board code.

Why do you say that ? There is a lot of code in arch/powerpc/ppc4xx/ and 
in configs/ you find the following defconfigs:
-rw-r--r--. 1 root root 641 Jun 22 01:34 CPCI4052_defconfig
-rw-r--r--. 1 root root 871 Jun 22 01:34 MIP405_defconfig
-rw-r--r--. 1 root root 799 Jun 22 01:34 MIP405T_defconfig
-rw-r--r--. 1 root root 869 Jun 22 01:34 PIP405_defconfig
-rw-r--r--. 1 root root 675 Jun 22 01:34 PLU405_defconfig
-rw-r--r--. 1 root root 588 Jun 22 01:34 PMC405DE_defconfig
-rw-r--r--. 1 root root 449 Jun 22 01:34 VOM405_defconfig
-rw-r--r--. 1 root root 747 Jun 22 01:34 xilinx-ppc405-generic_defconfig

> 
> On the other hand I do not see a single board configuration for 8xx,
> so you cannot even compile the code for a MPC8xx board, or am I
> missing something?

Yes you are right, Tom asked me to bring back only the core part for the 
time being, until I have cleaned up the code for our two boards.

> 
> 
> 
>>> Also, are you going to support the whole family, including MPC823(E)?
>>> If not, we should also omit stuff like the 823 video support.
>>
>> I agree, but please no big-bang now. It is already complicated enough.
> 
> I try to get used to the idea of doing code level cleanup later,
> even though my personal feeling for re-adding known-to-be-broken
> code is not exactly positive.
> 
> 
> But at least at file level we should perform the needed cleanup now:
> - omit unrelated stuff
> - omit known to be unsupported/untested/broken code
> - move drivers to the correct places according to today's file
>    system hierarchy.
> 
> As I explained before, I doubt that you will be able to test things
> like  drivers/video/mpc8xx_lcd.c  on your boards, and I strongly
> doubt that you are willing to invest efforts in adapting and testing
> things like PCMCIA, bedbug, SIL680 support, or USB on your hardware.

Yes I agree, that's probably the first things I will remove (except USB 
probably).

> 
> [In which way is the sil680 driver related to mpx8xx, btw?]
> 
> 
> So when we know in advance that this old, crappy code will never be
> tested, why should we spend effforts even in adding it?

Just because it was there 2 weeks ago. I prefer starting from a known 
situation.

> 
>> We have something that fits and that was unexpectedly removed entirely
>> last week. Lets come back first to a well known - working situation and
>> do the removal of unnecessary parts step by step as if we were 3 weeks
>> ago. The more we wait to get 8xx back in, the more difficult it will be.
> 
> To be frank: you don't have a very extensive track record of working
> with the U-Boot community.  At the moment all we have is your claim
> that you will care about 8xx in the furture.  In the past, we have
> seen many precedents of "please add this code now, I know it is not
> really good, but I will fix it later, promised" - and then the code
> was in mainline, until somebody else digged into cleaning it up.

Well, I don't know what to answer to that, I understand your reluctance 
but you probably also understand mine, I try crawling in one direction 
to get my boards in while others are removing more and more stuff (51xx, 
82xx, 5xx ...) which makes the come back of 8xx more difficult each day.

> 
> 
> I've been using MPC8xx for 18 years, and I know all too well in how
> many places the code is in urgent need of a cleanup (heck, I'm even
> responsible for major parts of that mess - but those were the times
> way back then).  I'm not sure if you already anticipate the size of
> job you have taken.  Just looking at the POST code - are you aware
> how much effort it will be to adapt it to your board and to test it?
> And this is mandatory requirement if you want to have it added. It
> must be maintained, i. e. tested.

We have it running on our boards and it works, it is just not in a clean 
submittable (doesn't respect the Codying Style, has several hard coded 
stuff in core parts, etc ...), so I cannot submit it just now, and I 
fear that by end august/early septembre, when I come with my cleaned 
board code, so much modification have been done to uboot core that 8xx 
cannot be back without tones of reverts.

> 
> It would be very sad if we add this code now, and in a few
> weeks/months you say: oh, we don't need this, and I don't find time
> to care about it, let's remove it again.  And this is what's bound
> to happen - I bet a beer or two that you have never used most of
> this code before, nor had a look at the code.
> 
> 
> I think it is much, much better to perform at least the file level
> cleanup in a sngle step right at the beginning, starting with the
> minimum set of needed files for your system.  Adding needed things
> later is a much better guaranteee for good code than removing
> unmaintained crap when it starts hurting.
> 

Ok but please stop removing or deapply modifying additional stuff that 
impacts the 8xx.

> 
> So please check your own board configuration and come up with a list
> of needed features, and re-add only those, cleaning up file
> hierarchy while you go.  See .sig below (non-random today).

Yes but that's not something that is doable in a few hours. So be 
compassionate and don't jeopardise it.

Thanks
Christophe

> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
> 