Re: [Qemu-devel] [Qemu-arm] Changes to Broadcom(BCM) files and Raspberry Pi files. Addition of PanelEmu

2017-05-15 Thread Philippe Mathieu-Daudé 

Hi John,


That is going to be very difficult as a lot of the changes are
interlinked the vast majority of the patch is new files.


I rebased your branch on latest qemu/master here:

https://github.com/philmd/qemu/tree/flypie-GDummyPanel-rebased

It is much easier to follow now, the big XML files you added/removed 
also disappeared (gitk was crashing 'Out Of Memory' trying to look at 
your tree).


I hope it can help you to continue reordering in smaller patches.

Regards,

Phil.

On 05/15/2017 01:46 PM, Alistair Francis wrote:


Hey John,

Thanks for the patch!

Unfortunately this patch is too long to review, you need to split the
patch up into shorter more readable patches. Otherwise it's too hard
to people to understand what you are changing and why.

There are some details here:
http://wiki.qemu.org/Contribute/SubmitAPatch
about how to split up
patches. Each patch applied in order shouldn't break any compilation
or runtime. Generally the flow is to add the logic in earlier patches
and then connect it and switch it on in the later patches.

Try splitting up adding/editing each individual device and send that
our first. That is generally the easiest to review/accept.

Thanks,

Alistair


---
.gitignore  |  54 ++
hw/arm/Makefile.objs|2 +-
hw/arm/bcm2835.c|  114 
hw/arm/bcm2835_peripherals.c|  104 
hw/arm/bcm2836.c|3 +-
hw/arm/raspi.c  |  77 ++-
hw/gpio/bcm2835_gpio.c  |  333 ++-
hw/misc/Makefile.objs|2 +
hw/misc/bcm2835_mphi.c  |  163 ++
hw/misc/bcm2835_power.c  |  106 
hw/timer/Makefile.objs  |2 +
hw/timer/bcm2835_st.c|  202 +++
hw/timer/bcm2835_timer.c|  224 +++
hw/usb/Makefile.objs|4 +-
hw/usb/bcm2835_usb.c|  604 +++
hw/usb/bcm2835_usb_regs.h| 1061

++

hw/usb/dev-network.c|2 +-
include/hw/arm/bcm2835.h|  37 ++
include/hw/arm/bcm2835_peripherals.h |  10 +
include/hw/gpio/bcm2835_gpio.h  |5 +
include/hw/intc/bcm2835_control.h|  53 ++
include/hw/intc/bcm2836_control.h|2 +
include/hw/misc/bcm2835_mphi.h  |  28 +
include/hw/misc/bcm2835_power.h  |  22 +
include/hw/timer/bcm2835_st.h|  25 +
include/hw/timer/bcm2835_timer.h|  32 +
include/hw/usb/bcm2835_usb.h|  78 +++
include/qemu/PanelEmu.h  |  53 ++
util/Makefile.objs  |1 +
util/PanelEmu.c  |  293 ++
30 files changed, 3547 insertions(+), 149 deletions(-)
create mode 100644 hw/arm/bcm2835.c
create mode 100644 hw/misc/bcm2835_mphi.c
create mode 100644 hw/misc/bcm2835_power.c
create mode 100644 hw/timer/bcm2835_st.c
create mode 100644 hw/timer/bcm2835_timer.c
create mode 100644 hw/usb/bcm2835_usb.c
create mode 100644 hw/usb/bcm2835_usb_regs.h
create mode 100644 include/hw/arm/bcm2835.h
create mode 100644 include/hw/intc/bcm2835_control.h
create mode 100644 include/hw/misc/bcm2835_mphi.h
create mode 100644 include/hw/misc/bcm2835_power.h
create mode 100644 include/hw/timer/bcm2835_st.h
create mode 100644 include/hw/timer/bcm2835_timer.h
create mode 100644 include/hw/usb/bcm2835_usb.h
create mode 100644 include/qemu/PanelEmu.h
create mode 100644 util/PanelEmu.c

Re: [Qemu-devel] [Qemu-arm] Changes to Broadcom(BCM) files and Raspberry Pi files. Addition of PanelEmu

2017-05-15 Thread John Bradley via Qemu-devel 
Hi,
The XML files in the base are not in the patch. They where net beans files. I 
can easily get it into 3 files, one large at 91KB but contains only new files 
and so is easy to read. Could be smaller but seems pointless. 

another which is about 19KB of quite simple changes to mostly make files.
Then one 15KB which quite straight forward when you look at it. Applying them 
in that order should allow step wise addition. I've uploaded them to a share if 
anyone wants to comment.

The 3 files are new.patch
  
|  
|   
|   
|   ||

   |

  |
|  
||  
new.patch
 Shared with Dropbox  |   |

  |

  |

 
 mod1.patch

  
|  
|   
|   
|   ||

   |

  |
|  
||  
mod1.patch
 Shared with Dropbox  |   |

  |

  |

 

and 
 a_hw_gpio_bcm2835_gpio.c.patch
  
|  
|   
|   
|   ||

   |

  |
|  
||  
a_hw_gpio_bcm2835_gpio.c.patch
 Shared with Dropbox  |   |

  |

  |

 



John BradleyTel: 07896 839635Skype: flypie125 125B Grove StreetEdge Hill 
Liverpool L7 7AF 

On Tuesday, 16 May 2017, 0:20, Philippe Mathieu-Daudé  
wrote:
 

 Hi John,

> That is going to be very difficult as a lot of the changes are
> interlinked the vast majority of the patch is new files.

I rebased your branch on latest qemu/master here:

https://github.com/philmd/qemu/tree/flypie-GDummyPanel-rebased

It is much easier to follow now, the big XML files you added/removed 
also disappeared (gitk was crashing 'Out Of Memory' trying to look at 
your tree).

I hope it can help you to continue reordering in smaller patches.

Regards,

Phil.

On 05/15/2017 01:46 PM, Alistair Francis wrote:
>
> Hey John,
>
> Thanks for the patch!
>
> Unfortunately this patch is too long to review, you need to split the
> patch up into shorter more readable patches. Otherwise it's too hard
> to people to understand what you are changing and why.
>
> There are some details here:
> http://wiki.qemu.org/Contribute/SubmitAPatch
> about how to split up
> patches. Each patch applied in order shouldn't break any compilation
> or runtime. Generally the flow is to add the logic in earlier patches
> and then connect it and switch it on in the later patches.
>
> Try splitting up adding/editing each individual device and send that
> our first. That is generally the easiest to review/accept.
>
> Thanks,
>
> Alistair
>
>> ---
>> .gitignore                          |  54 ++
>> hw/arm/Makefile.objs                |    2 +-
>> hw/arm/bcm2835.c                    |  114 
>> hw/arm/bcm2835_peripherals.c        |  104 
>> hw/arm/bcm2836.c                    |    3 +-
>> hw/arm/raspi.c                      |  77 ++-
>> hw/gpio/bcm2835_gpio.c              |  333 ++-
>> hw/misc/Makefile.objs                |    2 +
>> hw/misc/bcm2835_mphi.c              |  163 ++
>> hw/misc/bcm2835_power.c              |  106 
>> hw/timer/Makefile.objs              |    2 +
>> hw/timer/bcm2835_st.c                |  202 +++
>> hw/timer/bcm2835_timer.c            |  224 +++
>> hw/usb/Makefile.objs                |    4 +-
>> hw/usb/bcm2835_usb.c                |  604 +++
>> hw/usb/bcm2835_usb_regs.h            | 1061
> ++
>> hw/usb/dev-network.c                |    2 +-
>> include/hw/arm/bcm2835.h            |  37 ++
>> include/hw/arm/bcm2835_peripherals.h |  10 +
>> include/hw/gpio/bcm2835_gpio.h      |    5 +
>> include/hw/intc/bcm2835_control.h    |  53 ++
>> include/hw/intc/bcm2836_control.h    |    2 +
>> include/hw/misc/bcm2835_mphi.h      |  28 +
>> include/hw/misc/bcm2835_power.h      |  22 +
>> include/hw/timer/bcm2835_st.h        |  25 +
>> include/hw/timer/bcm2835_timer.h    |  32 +
>> include/hw/usb/bcm2835_usb.h        |  78 +++
>> include/qemu/PanelEmu.h              |  53 ++
>> util/Makefile.objs                  |    1 +
>> util/PanelEmu.c                      |  293 ++
>> 30 files changed, 3547 insertions(+), 149 deletions(-)
>> create mode 100644 hw/arm/bcm2835.c
>> create mode 100644 hw/misc/bcm2835_mphi.c
>> create mode 100644 hw/misc/bcm2835_power.c
>> create mode 100644 hw/timer/bcm2835_st.c
>> create mode 100644 hw/timer/bcm2835_timer.c
>> create mode 100644 hw/usb/bcm2835_usb.c
>> create mode 100644 hw/usb/bcm2835_usb_regs.h
>> create mode 100644 include/hw/arm/bcm2835.h
>> create mode 100644 include/hw/intc/bcm2835_control.h
>> create mode 100644 include/hw/misc/bcm2835_mphi.h
>> create mode 100644 include/hw/misc/bcm2835_power.h
>> create mode 100644 include/hw/timer/bcm2835_st.h
>> create mode 100644 include/hw/timer/bcm2835_timer.h
>> create mode 100644 include/hw/usb/bcm2835_usb.h
>> create mode 100644 include/qemu/PanelEmu.h
>> create mode 100644 util/PanelEmu.c
>>
>
>

Re: [Qemu-devel] [Qemu-arm] Changes to Broadcom(BCM) files and Raspberry Pi files. Addition of PanelEmu

2017-05-16 Thread Geert Martin Ijewski 

Am 16.05.2017 um 02:01 schrieb John Bradley via Qemu-devel:

Hi,
The XML files in the base are not in the patch. They where net beans files. I 
can easily get it into 3 files, one large at 91KB but contains only new files 
and so is easy to read. Could be smaller but seems pointless.


I think what Alistar meant was something along the lines of:
patch/commit 1) add the remaining bcm2835 devices from Markus 
Armbruster's code to QEMU (btw: any reason why that hasn't been done? 
The USB stuff does seem nice to have)

-- with a more meaningful commit title
2) add utils/panelemu.c and the include
3) bcm2835 now uses the emupanel
and then your other commits e.g.
4) USB CDC Ethernet driver dropped packets
5) emupanel: fixed compilation errors on linux
6) emupanel: removed last blocking I/O

Along the way you also seem to fix indentation in other code a lot, that 
makes following the patches more confusing than they need to be.
It's harder to gauge at a quick glance whether you changed something 
meaingul or not.


Maybe even send a RFC just detailing the protocol, because it's probably 
important to get that right from the start.


Geert

another which is about 19KB of quite simple changes to mostly make files.
Then one 15KB which quite straight forward when you look at it. Applying them 
in that order should allow step wise addition. I've uploaded them to a share if 
anyone wants to comment.

The 3 files are new.patch

|
|
|
|   ||

   |

  |
|
||
new.patch
 Shared with Dropbox  |   |

  |

  |


 mod1.patch


|
|
|
|   ||

   |

  |
|
||
mod1.patch
 Shared with Dropbox  |   |

  |

  |



and
 a_hw_gpio_bcm2835_gpio.c.patch

|
|
|
|   ||

   |

  |
|
||
a_hw_gpio_bcm2835_gpio.c.patch
 Shared with Dropbox  |   |

  |

  |





John BradleyTel: 07896 839635Skype: flypie125 125B Grove StreetEdge Hill 
Liverpool L7 7AF

On Tuesday, 16 May 2017, 0:20, Philippe Mathieu-Daudé  
wrote:


 Hi John,


That is going to be very difficult as a lot of the changes are
interlinked the vast majority of the patch is new files.


I rebased your branch on latest qemu/master here:

https://github.com/philmd/qemu/tree/flypie-GDummyPanel-rebased

It is much easier to follow now, the big XML files you added/removed
also disappeared (gitk was crashing 'Out Of Memory' trying to look at
your tree).

I hope it can help you to continue reordering in smaller patches.

Regards,

Phil.

On 05/15/2017 01:46 PM, Alistair Francis wrote:


Hey John,

Thanks for the patch!

Unfortunately this patch is too long to review, you need to split the
patch up into shorter more readable patches. Otherwise it's too hard
to people to understand what you are changing and why.

There are some details here:
http://wiki.qemu.org/Contribute/SubmitAPatch
about how to split up
patches. Each patch applied in order shouldn't break any compilation
or runtime. Generally the flow is to add the logic in earlier patches
and then connect it and switch it on in the later patches.

Try splitting up adding/editing each individual device and send that
our first. That is generally the easiest to review/accept.

Thanks,

Alistair


---
.gitignore  |  54 ++
hw/arm/Makefile.objs|2 +-
hw/arm/bcm2835.c|  114 
hw/arm/bcm2835_peripherals.c|  104 
hw/arm/bcm2836.c|3 +-
hw/arm/raspi.c  |  77 ++-
hw/gpio/bcm2835_gpio.c  |  333 ++-
hw/misc/Makefile.objs|2 +
hw/misc/bcm2835_mphi.c  |  163 ++
hw/misc/bcm2835_power.c  |  106 
hw/timer/Makefile.objs  |2 +
hw/timer/bcm2835_st.c|  202 +++
hw/timer/bcm2835_timer.c|  224 +++
hw/usb/Makefile.objs|4 +-
hw/usb/bcm2835_usb.c|  604 +++
hw/usb/bcm2835_usb_regs.h| 1061

++

hw/usb/dev-network.c|2 +-
include/hw/arm/bcm2835.h|  37 ++
include/hw/arm/bcm2835_peripherals.h |  10 +
include/hw/gpio/bcm2835_gpio.h  |5 +
include/hw/intc/bcm2835_control.h|  53 ++
include/hw/intc/bcm2836_control.h|2 +
include/hw/misc/bcm2835_mphi.h  |  28 +
include/hw/misc/bcm2835_power.h  |  22 +
include/hw/timer/bcm2835_st.h|  25 +
include/hw/timer/bcm2835_timer.h|  32 +
include/hw/usb/bcm2835_usb.h|  78 +++
include/qemu/PanelEmu.h  |  53 ++
util/Makefile.objs  |1 +
util/PanelEmu.c  |  293 ++
30 files changed, 3547 insertions(+), 149 deletions(-)
create mode 100644 hw/arm/bcm2835.c
create mode 100644 hw/misc/bcm2835_mphi.c
create mode 100644 hw/misc/bcm2835_power.c
create mode 100644 hw/timer/bcm2835_st.c
create mode 100644 hw/timer/bcm2835_timer.c
create mode 100644 hw/usb/bcm2835_usb.c
create mode 100644 hw/usb/bcm2835_usb_regs

Re: [Qemu-devel] [Qemu-arm] Changes to Broadcom(BCM) files and Raspberry Pi files. Addition of PanelEmu

2017-05-16 Thread John Bradley via Qemu-devel 
The idea of an RFC is good, but with it I would like to have a demo. Something 
with no mass implementation to worry about and this would be it. 
I should also add a version number to the initialisation of the protocol to 
increase robustness, between versions. I'll call this one V 0.
 John BradleyTel: 07896 839635Skype: flypie125 125B Grove StreetEdge Hill 
Liverpool L7 7AF 

On Tuesday, 16 May 2017, 9:56, Geert Martin Ijewski  
wrote:
 

 Am 16.05.2017 um 02:01 schrieb John Bradley via Qemu-devel:
> Hi,
> The XML files in the base are not in the patch. They where net beans files. I 
> can easily get it into 3 files, one large at 91KB but contains only new files 
> and so is easy to read. Could be smaller but seems pointless.
>
I think what Alistar meant was something along the lines of:
patch/commit 1) add the remaining bcm2835 devices from Markus 
Armbruster's code to QEMU (btw: any reason why that hasn't been done? 
The USB stuff does seem nice to have)
-- with a more meaningful commit title
2) add utils/panelemu.c and the include
3) bcm2835 now uses the emupanel
and then your other commits e.g.
4) USB CDC Ethernet driver dropped packets
5) emupanel: fixed compilation errors on linux
6) emupanel: removed last blocking I/O

Along the way you also seem to fix indentation in other code a lot, that 
makes following the patches more confusing than they need to be.
It's harder to gauge at a quick glance whether you changed something 
meaingul or not.

Maybe even send a RFC just detailing the protocol, because it's probably 
important to get that right from the start.

Geert
> another which is about 19KB of quite simple changes to mostly make files.
> Then one 15KB which quite straight forward when you look at it. Applying them 
> in that order should allow step wise addition. I've uploaded them to a share 
> if anyone wants to comment.
>
> The 3 files are new.patch
>
> |
> |
> |
> |  |    |
>
>    |
>
>  |
> |
> |    |
> new.patch
>  Shared with Dropbox  |  |
>
>  |
>
>  |
>
>
>  mod1.patch
>
>
> |
> |
> |
> |  |    |
>
>    |
>
>  |
> |
> |    |
> mod1.patch
>  Shared with Dropbox  |  |
>
>  |
>
>  |
>
>
>
> and
>  a_hw_gpio_bcm2835_gpio.c.patch
>
> |
> |
> |
> |  |    |
>
>    |
>
>  |
> |
> |    |
> a_hw_gpio_bcm2835_gpio.c.patch
>  Shared with Dropbox  |  |
>
>  |
>
>  |
>
>
>
>
>
> John BradleyTel: 07896 839635Skype: flypie125 125B Grove StreetEdge Hill 
> Liverpool L7 7AF
>
>    On Tuesday, 16 May 2017, 0:20, Philippe Mathieu-Daudé  
>wrote:
>
>
>  Hi John,
>
>> That is going to be very difficult as a lot of the changes are
>> interlinked the vast majority of the patch is new files.
>
> I rebased your branch on latest qemu/master here:
>
> https://github.com/philmd/qemu/tree/flypie-GDummyPanel-rebased
>
> It is much easier to follow now, the big XML files you added/removed
> also disappeared (gitk was crashing 'Out Of Memory' trying to look at
> your tree).
>
> I hope it can help you to continue reordering in smaller patches.
>
> Regards,
>
> Phil.
>
> On 05/15/2017 01:46 PM, Alistair Francis wrote:
>>
>> Hey John,
>>
>> Thanks for the patch!
>>
>> Unfortunately this patch is too long to review, you need to split the
>> patch up into shorter more readable patches. Otherwise it's too hard
>> to people to understand what you are changing and why.
>>
>> There are some details here:
>> http://wiki.qemu.org/Contribute/SubmitAPatch
>> about how to split up
>> patches. Each patch applied in order shouldn't break any compilation
>> or runtime. Generally the flow is to add the logic in earlier patches
>> and then connect it and switch it on in the later patches.
>>
>> Try splitting up adding/editing each individual device and send that
>> our first. That is generally the easiest to review/accept.
>>
>> Thanks,
>>
>> Alistair
>>
>>> ---
>>> .gitignore                          |  54 ++
>>> hw/arm/Makefile.objs                |    2 +-
>>> hw/arm/bcm2835.c                    |  114 
>>> hw/arm/bcm2835_peripherals.c        |  104 
>>> hw/arm/bcm2836.c                    |    3 +-
>>> hw/arm/raspi.c                      |  77 ++-
>>> hw/gpio/bcm2835_gpio.c              |  333 ++-
>>> hw/misc/Makefile.objs                |    2 +
>>> hw/misc/bcm2835_mphi.c              |  163 ++
>>> hw/misc/bcm2835_power.c              |  106 
>>> hw/timer/Makefile.objs              |    2 +
>>> hw/timer/bcm2835_st.c                |  202 +++
>>> hw/timer/bcm2835_timer.c            |  224 +++
>>> hw/usb/Makefile.objs                |    4 +-
>>> hw/usb/bcm2835_usb.c                |  604 +++
>>> hw/usb/bcm2835_usb_regs.h            | 1061
>> ++
>>> hw/usb/dev-network.c                |    2 +-
>>> include/hw/arm/bcm2835.h            |  37 ++
>>> include/hw/arm/bcm2835_peripherals.h |  10 +
>>> include/hw/gpio/bcm2835_gpio.h      |    5 +
>>> include/hw/intc/bcm2835_control.h    |  53 ++
>>> include/hw/intc