On Tue, Aug 22, 2017 at 01:18:02PM +0200, BALATON Zoltan wrote: > On Tue, 22 Aug 2017, David Gibson wrote: > > On Fri, Aug 18, 2017 at 02:46:42PM +0200, BALATON Zoltan wrote: > > > On Fri, 18 Aug 2017, David Gibson wrote: > > > > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote: > > > > > Add emulation of aCube Sam460ex board based on AMCC 460EX embedded > > > > > SoC. > > > > > This is not a full implementation yet with a lot of components still > > > > > missing but enough to start a Linux kernel and the U-Boot firmware. > > > > > > > > > > Signed-off-by: François Revol <re...@free.fr> > > > > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > > > > > > > > There are a *lot* of devices defined here. Most of them look like > > > > they belong to the SoC, not the board (since they use DCRs), so it > > > > doesn't really make sense to define them in a board file. It would > > > > also make it easier to review if they were split up into separate > > > > patches. > > > > > > I thought it's simpler to review a series with 12 reasonably sized patches > > > than one with twice as many which only modify a few lines here and there > > > each. Also adding a lot of code scattered in hw directories is probably > > > less > > > clear than having them all at one place. But of course each approach can > > > be > > > reasoned. I thought this might have to be split up but I've left it one > > > place for now for first review to get some advice on what's preferred. > > > > Well, it depends on the content of the patches. If splitting it up > > means a lot of looking between patches to make sense of what's going > > on then that's certainly not good. But if the small patches are > > independent of each other and can be assessed on their own merits, > > then that usually makes it easier. > > > > In this case the various new 440 devices should be pretty much > > independent of each other, so I think splitting is the better option. > > > > > Maybe I should put things that belong to the SoC in ppc440_uc.c (similar > > > to > > > ppc405uc.c we already have) and move common devices used by both to > > > ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup > > > is needed that could be done separately afterwards, I don't think it's a > > > good idea to mix in too much cleanup now to keep patches relatively > > > simple. > > > (I already have some moving around included as clean up patches but I'd > > > like > > > to focus on actual functions than clean up at this point). > > > > > > Does putting these devices from board code to ppc440_uc.c sound > > > acceptable? > > > > That'd be ok - though again, I'd prefer to see each device as a > > separate patch. It would probably be preferable to put each device in > > a separate file as well though, unless they're _really_ tiny. Nothing > > inherently wrong with small .c files, if they're still more or less > > self contained. > > Since most of these are just stubs for now and not really full emulation of > the device I'd leave them in one file similar to ppc405 for now. They both > could be cleaned up later if it's found necessary or devices that are deemed > ready and big enough to be moved out could be split off (like I did with i2c > and pcix). If you insist, I can split it now but I think it's more difficult > to work with a lot of small files scattered in hw instead of everything > related to ppc440 in one place until there's still missing fucntionality. I > consider splitting it up cleanup which could be done later separately unless > it's a requirement to get this series in.
Ok, that's a reasonable argument. Go ahead and put them in a ppc440_uc.c file. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature