On Tue, Dec 11, 2012 at 2:57 PM, Peter Maydell <peter.mayd...@linaro.org>wrote:

> On 11 December 2012 13:08, M P <buser...@gmail.com> wrote:
> > I've added imx23 support to qemu, with quite a lot of working
> peripherals,
> > and enough to boot linux with SD card and USB working. There are bits
> > missing (audio, video outputs) but it's otherwise working pretty well.
> >
> > I've collated and cleaned the patches onto this github branch, I'd
> > appreciate any comments and/or complementary work before I submit the
> > patches for merging here...
> >
> > https://github.com/buserror-uk/qemu-buserror/commits/dev-imx233
>
> "Showing 11 changed files with 2,236 additions and 0 deletions."
>
> That commit absolutely has to be broken up into a coherent set of
> smaller patches; it is far too big to code review as it stands.
>

Well I pondered that, but really I don't see how to split that since they
are all new files.., and all the bits are part of one 'unit'.. Any
suggestion ?


>
> You don't seem to be following the QEMU coding style. (CODING_STYLE
> and scripts/checkpatch.pl may be of use).
>

I'll study the coding style. I have to say I did a bit of cut/paste from
other files and 'tried' to stick to what I've seen, and otherwise defaulted
to kernel mode..


>
> Information on how to test the board model would be good. Also
> unit tests, maybe?
>
> Are you going to be sticking around to help maintain and improve
> the board model in the future?
>

Yes I'd definitely would want that, as well as add imx28 -- I did the
mini2440 qemu support years ago, but let it diverge and
always regretted that as now it's it would require more or less a rewrite...


>
> http://wiki.qemu.org/Contribute/SubmitAPatch has more pointers
> on patch submission.
>

Thanks, thats exactly the sort of information I wanted :-)


>
> thanks
> -- PMM
>

Michael

Reply via email to