From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCNEn-0004qO-KH for qemu-devel@nongnu.org; Tue, 07 Jul 2015 03:24:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCNEj-0001xF-8T for qemu-devel@nongnu.org; Tue, 07 Jul 2015 03:24:17 -0400 Received: from mail-la0-x232.google.com ([2a00:1450:4010:c03::232]:36588) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCNEi-0001wl-Qy for qemu-devel@nongnu.org; Tue, 07 Jul 2015 03:24:13 -0400 Received: by lagc2 with SMTP id c2so182876496lag.3 for ; Tue, 07 Jul 2015 00:24:12 -0700 (PDT) Date: Tue, 7 Jul 2015 10:30:57 +0300 From: Antony Pavlov Message-Id: <20150707103057.bba8fe37471c75ddc2712c0f@gmail.com> In-Reply-To: References: <16270e45922ea6a8c8622e76f702c46ce7ce15ac.1435723168.git.serge.vakulenko@gmail.com> <20150701134103.GB8793@aurel32.net> <20150706103304.8eef043a9e27e0d7b514061c@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Vakulenko Cc: Peter Crosthwaite , Leon Alrae , qemu-devel@nongnu.org, Aurelien Jarno On Mon, 6 Jul 2015 11:58:54 -0700 Serge Vakulenko wrote: > On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov = wrote: > > On Sun, 5 Jul 2015 21:18:11 -0700 > > Serge Vakulenko wrote: > > > >> On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno = wrote: > >> > On 2015-06-30 21:12, Serge Vakulenko wrote: > >> >> Signed-off-by: Serge Vakulenko > >> >> --- > >> >> hw/mips/Makefile.objs | 3 + > >> >> hw/mips/mips_pic32mx7.c | 1652 +++++++++++++++++++++++++ > >> >> hw/mips/mips_pic32mz.c | 2840 +++++++++++++++++++++++++++++++= ++++++++++++ > >> >> hw/mips/pic32_ethernet.c | 557 +++++++++ > >> >> hw/mips/pic32_gpio.c | 39 + > >> >> hw/mips/pic32_load_hex.c | 238 ++++ > >> >> hw/mips/pic32_peripherals.h | 210 ++++ > >> >> hw/mips/pic32_sdcard.c | 428 +++++++ > >> >> hw/mips/pic32_spi.c | 121 ++ > >> >> hw/mips/pic32_uart.c | 228 ++++ > >> >> hw/mips/pic32mx.h | 1290 ++++++++++++++++++++ > >> >> hw/mips/pic32mz.h | 2093 +++++++++++++++++++++++++++++++ > >> >> 12 files changed, 9699 insertions(+) > >> >> create mode 100644 hw/mips/mips_pic32mx7.c > >> >> create mode 100644 hw/mips/mips_pic32mz.c > >> >> create mode 100644 hw/mips/pic32_ethernet.c > >> >> create mode 100644 hw/mips/pic32_gpio.c > >> >> create mode 100644 hw/mips/pic32_load_hex.c > >> >> create mode 100644 hw/mips/pic32_peripherals.h > >> >> create mode 100644 hw/mips/pic32_sdcard.c > >> >> create mode 100644 hw/mips/pic32_spi.c > >> >> create mode 100644 hw/mips/pic32_uart.c > >> >> create mode 100644 hw/mips/pic32mx.h > >> >> create mode 100644 hw/mips/pic32mz.h > >> > > >> > This patch is huge, and needs to be splitted to ease the review. > >> > >> I'll prepare a new patch set, with every new file put into a separate > >> message. Other issues fixed as well. > > > > Putting every new file into a separate message is a nonsense. > > Please separate __logical changes__ into a single patch. >=20 > Aurelien Jarno asked to split this patch to ease the review. IMHO he meant something very different. Please reread the qemu submitting patch manual carefully (see http://wiki.qemu.org/Contribute/SubmitAPatch). Here is a quote: Split up longer patches into a patch series of logical code changes. Each change should compile and execute successfully. For instance, don't add a file to the makefile in patch one and then add the file itself in patch two. (This rule is here so that people can later use tools like git bisect without hitting points in the commit history where QEMU doesn't work for reasons unrelated to the bug they're chasing.) Also please reread this Peter's comment very very carefully: =20 http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg01430.html Peter asks you to rework your device support code: every device should be s= elf-contained. E.g. for UART support code this means that: 0. Object model is used. Your UART code implements operation of one UART= instance. private structure is used for storing UART instance's current state. The SoC code (or even board code) creates as many UART instances as i= t needs. Also please see this Aurilien's comment: http://lists.nongnu.org/arch= ive/html/qemu-devel/2015-07/msg01242.html 1. UART C code go to qemu.git/hw/char/; 2. externally visible UART stuff (header file) go to qemu.git/include/hw= /char/; Pay attention that there is no need to put all UART related macro int= o header file. If nobody outside your UART C code use these macros then you can keep= their definition in the C code. 3. UART C code compilation has to be enabled only for mips-softmmu targe= t. So make your UART C code compilation dependendent on a Makefile optio= n, enable this option only in qemu.git/default-configs/mips-softmmu.mak. 4. UART support have to be added in a separate patch. So this patch have= to contain changes in these files: default-configs/mips-softmmu.mak hw/char/Makefile.objs hw/char/pic32_uart.c include/hw/char/pic32_uart.h =20 This UART support patch has to be submitted __before__ a patch with S= oC/board code that use UART. As Peter suggests please use 'Netduino 2 Machine Model' patchseries as a mo= del, see http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg03398.html --=A0 Best regards, =A0 Antony Pavlov