[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch v8 0/5] i.MX31 support
From: |
Peter Chubb |
Subject: |
Re: [Qemu-devel] [patch v8 0/5] i.MX31 support |
Date: |
Thu, 07 Jun 2012 10:51:51 +1000 |
User-agent: |
Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (Gojō) APEL/10.8 Emacs/23.4 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) |
>>>>> "Andreas" == Andreas Färber <address@hidden> writes:
Andreas> Hello Peter, Am 06.06.2012 05:47, schrieb Peter Chubb:
>> There are no major changes since last time, just rebased to current
>> tip now that QEMU 1.2 is open.
>>
>> For those who have come into the story late, this is a series of
>> patches to allow QEMU to emulate a Freescale i.MX31 on a Kyoto
>> Microsystems evaluation board. It's pretty bare-bones, but runs
>> Linux and seL4 nicely.
Andreas> Something is wrong with the patch submission, they are shown
Andreas> as attachments and Thunderbird doesn't let me comment
Andreas> inline. Please use git-send-email to submit.
This is a known issue with some thunderbird versions --- Mozilla/Mime
support is a bit weird when it comes to emails with a
Content-Disposition: inline with a filename.
Andreas> Also the diffstat doesn't match the patch wrt file ordering,
Andreas> so it would be advisable to use git-format-patch.
Andreas> Our concept of topics in the subject seems to be troubling
Andreas> you, you have added " support" since v7 (still readable, so
Andreas> I'm okay) whereas what we usually use is a lower-case tag as
Andreas> described here plus an active description of what it's doing
Andreas> (e.g., "foo: Add/Introduce bar"):
Andreas> https://live.gnome.org/Git/CommitMessages
This series was broken in a couple of ways. Please ignore it! (my
laptop got restored from a backup and in moving forward again I think
some changes that were in v7 got lost, including this one).
Andreas> On patch 5: Please use cpu_arm_init() in place of cpu_init()
Andreas> and prefer ARMCPU. This is already in qemu.git and many more
Andreas> conversions are in the QOM CPUState part 3 PULL and on
Andreas> qom-next.
I dunno what happened there. I put that in, then the result wouldn;t
compile, so I reverted it.
Andreas> Also note that arm_pic_init_cpu() and arm_load_kernel() are
Andreas> being changed to take that ARMCPU, so this needs to be
Andreas> coordinated.
OK.
Andreas> Please remove the semicolon after machine_init() and check
Andreas> the indentation. After sysbus_create_varargs() for instance I
Andreas> spot one space too few and below for imx_serial_create()
Andreas> there's a double space.
OK. Dunno why checkpatch didn't catch this.
Andreas> Please also make all your TypeInfos static const, probably
Andreas> same for QEMUMachine.
OK.
Andreas> The description sounds misleading: In qemu-system-arm all
Andreas> boards are ARM architecture, and your wording may sound as if
Andreas> the board were from ARM (Holdings plc) when it is from Kyoto
Andreas> Micro and uses a Freescale SoC. Maybe also mention the exact
Andreas> board name from the commit message and mention i.MX31?
OK.
Andreas> s/I.MX31/i.MX31/ in the header?
Ok.
Andreas> On patch 4: There's an empty line after type_init(), please
Andreas> remove.
OK.
Andreas> On patch 3: In IMXTimerGState you're saving DeviceState
Andreas> *ccm. That should probably become a QOM link<> property,
Andreas> possibly after the initial submission. CC'ing Paolo.
I'm not sure what this means.
Andreas> What should be considered as a second step is factoring out
Andreas> all the devices individually added to the board into an
Andreas> i.MX31 SoC, which has implications on how the devices are
Andreas> initialized (compare my recent prep_pci and Anthony's i440fx
Andreas> patches). For a less sophisticated way using functions check
Andreas> out exynos4210. The way it's done right now there's no
Andreas> distinction of what is on the SoC and what is on the board,
Andreas> so it needs to be done by you.
Let's get the first round in first.
Andreas> Missing is an entry to MAINTAINERS for this board and its
Andreas> exclusive devices, naming who is to be cc'ed on patches.
OK.
--
Dr Peter Chubb peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
- [Qemu-devel] [patch v8 0/5] i.MX31 support, Peter Chubb, 2012/06/06
- [Qemu-devel] [patch v8 3/5] i.MX31 support: Timers, Peter Chubb, 2012/06/06
- [Qemu-devel] [patch v8 4/5] i.MX31 support: AVIC, Peter Chubb, 2012/06/06
- [Qemu-devel] [patch v8 5/5] i.MX31 support: KZM-ARM11-01 evaluation board, Peter Chubb, 2012/06/06
- [Qemu-devel] [patch v8 1/5] i.MX UART support, Peter Chubb, 2012/06/06
- [Qemu-devel] [patch v8 2/5] i.MX31 support: Clock Control Module, Peter Chubb, 2012/06/06
- Re: [Qemu-devel] [patch v8 0/5] i.MX31 support, Andreas Färber, 2012/06/06
- Re: [Qemu-devel] [patch v8 0/5] i.MX31 support,
Peter Chubb <=