[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz
From: |
Antony Pavlov |
Subject: |
Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz. |
Date: |
Tue, 7 Jul 2015 10:30:57 +0300 |
On Mon, 6 Jul 2015 11:58:54 -0700
Serge Vakulenko <address@hidden> wrote:
> On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov <address@hidden> wrote:
> > On Sun, 5 Jul 2015 21:18:11 -0700
> > Serge Vakulenko <address@hidden> wrote:
> >
> >> On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno <address@hidden> wrote:
> >> > On 2015-06-30 21:12, Serge Vakulenko wrote:
> >> >> Signed-off-by: Serge Vakulenko <address@hidden>
> >> >> ---
> >> >> 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.
>
> 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:
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
self-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 it
needs.
Also please see this Aurilien's comment:
http://lists.nongnu.org/archive/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 into
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 target.
So make your UART C code compilation dependendent on a Makefile option,
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
This UART support patch has to be submitted __before__ a patch with
SoC/board code that use UART.
As Peter suggests please use 'Netduino 2 Machine Model' patchseries as a model,
see http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg03398.html
--
Best regards,
Antony Pavlov
Message not available
- [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Serge Vakulenko, 2015/07/01
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Aurelien Jarno, 2015/07/01
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Serge Vakulenko, 2015/07/06
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Antony Pavlov, 2015/07/06
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Serge Vakulenko, 2015/07/06
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Peter Crosthwaite, 2015/07/06
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.,
Antony Pavlov <=
- Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Aurelien Jarno, 2015/07/07
Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Antony Pavlov, 2015/07/02
Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Serge Vakulenko, 2015/07/06
Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz., Antony Pavlov, 2015/07/06
Re: [Qemu-devel] [PATCH pic32 v2 0/5] Support for Microchip pic32mx7 and pic32mz microcontrollers, Antony Pavlov, 2015/07/02