Hi Peter, On 2015/5/8 0:34, Peter Maydell wrote: > Hi. I've had a look through these patches now, and sent some mails > for a couple of specific issues. For the rest, this is what I'd > like to see for me to pull this in to target-arm: >
Thanks for your review comments. > (1) acks or reviewed-by from one of the QEMU ACPI folk for any > patch that's touching generic ACPI code. Yeah, Igor said he will give reviewed-by for these patches. > (2) series should pass checkpatch.pl (you have a lot of overlong lines > warnings at the moment). Sorry, I will fix patch 07/22， but to patch 01/22， as I just move the file, maybe I don't need to deal the formats error of acpi-defs.h. Right? > (3) we shouldn't generate any ACPI tables or information where the > corresponding kernel code hasn't been accepted into mainline or > the arm64 tree; if there's anything here which isn't covered by > the accepted ACPI patches we should postpone that til the kernel > parts are in. > There are uart, virtio-mmio, PCIe whose kernel patches are not accepted into kernel mainline. For PCIe I think this is harmless. The PCIe ACPI table's format don't need to change even though the kernel code changes because it's defined by PCI firmware SPEC. For virtio-mmio the kernel code just add acpi_match_table for kernel driver to probe the device by ACPI. To be honestly the only thing will be changed is the _HID and this change is very small. It's same with uart. In a word, the only thing will be changed is the _HID of virtio-mmio and uart. But if we get these tables in, it's useful for people to test the corresponding kernel code. > If item (3) seems too awkward or harsh as a criterion I'm > open to discussion about it, but my feeling is that I'd > rather not start emitting tables until they're definitely > nailed down in format by being in the kernel. > > Finally, if you have some prebuilt images and command lines that > would be helpful, since I can put them into my local set of > things I run to check for breakage before making ARM pull requests. Ok. -- Shannon
