Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-07 Thread Antony Pavlov
On Mon, 6 Jul 2015 11:58:54 -0700
Serge Vakulenko serge.vakule...@gmail.com wrote:

 On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov antonynpav...@gmail.com 
 wrote:
  On Sun, 5 Jul 2015 21:18:11 -0700
  Serge Vakulenko serge.vakule...@gmail.com wrote:
 
  On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net 
  wrote:
   On 2015-06-30 21:12, Serge Vakulenko wrote:
   Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
   ---
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



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-07 Thread Aurelien Jarno
On 2015-07-07 10:30, Antony Pavlov wrote:
 On Mon, 6 Jul 2015 11:58:54 -0700
 Serge Vakulenko serge.vakule...@gmail.com wrote:
 
  On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov antonynpav...@gmail.com 
  wrote:
   On Sun, 5 Jul 2015 21:18:11 -0700
   Serge Vakulenko serge.vakule...@gmail.com wrote:
  
   On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net 
   wrote:
On 2015-06-30 21:12, Serge Vakulenko wrote:
Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
---
 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.

I confirm I wanted basically a changeset progressively adding devices,
like one patch per devices.

 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

Thanks for the detailed guidelines, it's quite useful.

Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-06 Thread Antony Pavlov
On Sun, 5 Jul 2015 21:18:11 -0700
Serge Vakulenko serge.vakule...@gmail.com wrote:

 On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net wrote:
  On 2015-06-30 21:12, Serge Vakulenko wrote:
  Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
  ---
   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.

-- 
Best regards,
  Antony Pavlov



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-06 Thread Antony Pavlov
On Sun, 5 Jul 2015 21:27:04 -0700
Serge Vakulenko serge.vakule...@gmail.com wrote:

 On Wed, Jul 1, 2015 at 10:56 PM, Antony Pavlov antonynpav...@gmail.com 
 wrote:
  On Tue, 30 Jun 2015 21:12:34 -0700
  Serge Vakulenko serge.vakule...@gmail.com wrote:
 
  Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
  ---
   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
 
  diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
  index 9633f3a..dcbaec9 100644
  --- a/hw/mips/Makefile.objs
  +++ b/hw/mips/Makefile.objs
  @@ -3,3 +3,6 @@ obj-y += addr.o cputimer.o mips_int.o
   obj-$(CONFIG_JAZZ) += mips_jazz.o
   obj-$(CONFIG_FULONG) += mips_fulong2e.o
   obj-y += gt64xxx_pci.o
  +obj-y += mips_pic32mz.o mips_pic32mx7.o
  +obj-y += pic32_load_hex.o pic32_sdcard.o pic32_spi.o pic32_uart.o 
  pic32_gpio.o
  +obj-y += pic32_ethernet.o
 
  Can we move mips-unrelated stuff to the appropriate dirs?
  E.g. pic32_gpio.c can to go to hw/gpio.
 
 All these files are pic32-related. They depend on pic32_t data
 structure declared in pic32_peripherals.h and register definitions in
 pic32mx.h and pic32mz.h. I see no point to move them around.

But why hw/gpio/pic32_gpio.c can't include pic32_peripherals.h?
You can see that mainline qemu.git/hw/mips/ does not contain any header file,
so just put your header files into the proper dir (I suppose 
qemu.git/include/hw/mips).


  Also please use separate patch for every peripheral controller (see 
  Aurelien's comment).
 
 Agreed.
 
  diff --git a/hw/mips/mips_pic32mx7.c b/hw/mips/mips_pic32mx7.c
  new file mode 100644
  index 000..1d8ffb5
  --- /dev/null
  +++ b/hw/mips/mips_pic32mx7.c
  @@ -0,0 +1,1652 @@
  +/*
  + * QEMU support for Microchip PIC32MX7 microcontroller.
  + *
  + * Copyright (c) 2015 Serge Vakulenko
  + *
  + * Permission is hereby granted, free of charge, to any person obtaining 
  a copy
  + * of this software and associated documentation files (the Software), 
  to deal
  + * in the Software without restriction, including without limitation the 
  rights
  + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
  sell
  + * copies of the Software, and to permit persons to whom the Software is
  + * furnished to do so, subject to the following conditions:
  + *
  + * The above copyright notice and this permission notice shall be 
  included in
  + * all copies or substantial portions of the Software.
  + *
  + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, 
  EXPRESS OR
  + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
  MERCHANTABILITY,
  + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
  + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
  OTHER
  + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
  ARISING FROM,
  + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
  IN
  + * THE SOFTWARE.
  + */
  +
  +/* Only 32-bit little endian mode supported. */
  +#include config.h
  +#if !defined TARGET_MIPS64  !defined TARGET_WORDS_BIGENDIAN
 
  Please don't use C preprocessor directive for conditional compilation of 
  the whole file.
  Use Makefile instead. See CONFIG_FULONG for example (fulong2e is 
  mips64le-only).
 
 Makes sense.
 
 Thanks,
 --Serge
 
  --
  Best regards,
Antony Pavlov


-- 
-- 
Best regards,
  Antony Pavlov



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-06 Thread Serge Vakulenko
On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov antonynpav...@gmail.com wrote:
 On Sun, 5 Jul 2015 21:18:11 -0700
 Serge Vakulenko serge.vakule...@gmail.com wrote:

 On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net wrote:
  On 2015-06-30 21:12, Serge Vakulenko wrote:
  Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
  ---
   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.

 --
 Best regards,
   Antony Pavlov



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-06 Thread Peter Crosthwaite
On Mon, Jul 6, 2015 at 11:58 AM, Serge Vakulenko
serge.vakule...@gmail.com wrote:
 On Mon, Jul 6, 2015 at 12:33 AM, Antony Pavlov antonynpav...@gmail.com 
 wrote:
 On Sun, 5 Jul 2015 21:18:11 -0700
 Serge Vakulenko serge.vakule...@gmail.com wrote:

 On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net wrote:
  On 2015-06-30 21:12, Serge Vakulenko wrote:
  Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
  ---
   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.


There are better ways to split the patch other than straight per-file
though. For example, header patches (e.g. for your structs) should go
with their accompanying C code changes.

git add -p is the interactive utility for selecting specific changes
to be included in a commit.

Regards,
Peter

 --
 Best regards,
   Antony Pavlov




Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-05 Thread Serge Vakulenko
On Wed, Jul 1, 2015 at 6:41 AM, Aurelien Jarno aurel...@aurel32.net wrote:
 On 2015-06-30 21:12, Serge Vakulenko wrote:
 Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
 ---
  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.

 --
 Aurelien Jarno  GPG: 4096R/1DDD8C9B
 aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-05 Thread Serge Vakulenko
On Wed, Jul 1, 2015 at 10:56 PM, Antony Pavlov antonynpav...@gmail.com wrote:
 On Tue, 30 Jun 2015 21:12:34 -0700
 Serge Vakulenko serge.vakule...@gmail.com wrote:

 Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
 ---
  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

 diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
 index 9633f3a..dcbaec9 100644
 --- a/hw/mips/Makefile.objs
 +++ b/hw/mips/Makefile.objs
 @@ -3,3 +3,6 @@ obj-y += addr.o cputimer.o mips_int.o
  obj-$(CONFIG_JAZZ) += mips_jazz.o
  obj-$(CONFIG_FULONG) += mips_fulong2e.o
  obj-y += gt64xxx_pci.o
 +obj-y += mips_pic32mz.o mips_pic32mx7.o
 +obj-y += pic32_load_hex.o pic32_sdcard.o pic32_spi.o pic32_uart.o 
 pic32_gpio.o
 +obj-y += pic32_ethernet.o

 Can we move mips-unrelated stuff to the appropriate dirs?
 E.g. pic32_gpio.c can to go to hw/gpio.

All these files are pic32-related. They depend on pic32_t data
structure declared in pic32_peripherals.h and register definitions in
pic32mx.h and pic32mz.h. I see no point to move them around.

 Also please use separate patch for every peripheral controller (see 
 Aurelien's comment).

Agreed.

 diff --git a/hw/mips/mips_pic32mx7.c b/hw/mips/mips_pic32mx7.c
 new file mode 100644
 index 000..1d8ffb5
 --- /dev/null
 +++ b/hw/mips/mips_pic32mx7.c
 @@ -0,0 +1,1652 @@
 +/*
 + * QEMU support for Microchip PIC32MX7 microcontroller.
 + *
 + * Copyright (c) 2015 Serge Vakulenko
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 + * of this software and associated documentation files (the Software), to 
 deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included 
 in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS 
 OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
 OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 + * THE SOFTWARE.
 + */
 +
 +/* Only 32-bit little endian mode supported. */
 +#include config.h
 +#if !defined TARGET_MIPS64  !defined TARGET_WORDS_BIGENDIAN

 Please don't use C preprocessor directive for conditional compilation of the 
 whole file.
 Use Makefile instead. See CONFIG_FULONG for example (fulong2e is 
 mips64le-only).

Makes sense.

Thanks,
--Serge

 --
 Best regards,
   Antony Pavlov



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-01 Thread Aurelien Jarno
On 2015-06-30 21:12, Serge Vakulenko wrote:
 Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
 ---
  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.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH pic32 v2 5/5] Two new machine platforms: pic32mz7 and pic32mz.

2015-07-01 Thread Antony Pavlov
On Tue, 30 Jun 2015 21:12:34 -0700
Serge Vakulenko serge.vakule...@gmail.com wrote:

 Signed-off-by: Serge Vakulenko serge.vakule...@gmail.com
 ---
  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
 
 diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
 index 9633f3a..dcbaec9 100644
 --- a/hw/mips/Makefile.objs
 +++ b/hw/mips/Makefile.objs
 @@ -3,3 +3,6 @@ obj-y += addr.o cputimer.o mips_int.o
  obj-$(CONFIG_JAZZ) += mips_jazz.o
  obj-$(CONFIG_FULONG) += mips_fulong2e.o
  obj-y += gt64xxx_pci.o
 +obj-y += mips_pic32mz.o mips_pic32mx7.o
 +obj-y += pic32_load_hex.o pic32_sdcard.o pic32_spi.o pic32_uart.o 
 pic32_gpio.o
 +obj-y += pic32_ethernet.o

Can we move mips-unrelated stuff to the appropriate dirs?
E.g. pic32_gpio.c can to go to hw/gpio.
Also please use separate patch for every peripheral controller (see Aurelien's 
comment).


 diff --git a/hw/mips/mips_pic32mx7.c b/hw/mips/mips_pic32mx7.c
 new file mode 100644
 index 000..1d8ffb5
 --- /dev/null
 +++ b/hw/mips/mips_pic32mx7.c
 @@ -0,0 +1,1652 @@
 +/*
 + * QEMU support for Microchip PIC32MX7 microcontroller.
 + *
 + * Copyright (c) 2015 Serge Vakulenko
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 + * of this software and associated documentation files (the Software), to 
 deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 + * THE SOFTWARE.
 + */
 +
 +/* Only 32-bit little endian mode supported. */
 +#include config.h
 +#if !defined TARGET_MIPS64  !defined TARGET_WORDS_BIGENDIAN

Please don't use C preprocessor directive for conditional compilation of the 
whole file.
Use Makefile instead. See CONFIG_FULONG for example (fulong2e is mips64le-only).


-- 
Best regards,
  Antony Pavlov