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

[John Bradley <flypie1@yahoo.com>]

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 <gm.ijewski@web.de> 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é <f4bug@amsat.org> 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
>> <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
>>>
>>
>>
>
>
>
>