Re: [Qemu-devel] [PATCH target-arm v1 00/15] Next Generation Xilinx Zynq SoC

[Alistair Francis]

On Tue, Mar 3, 2015 at 6:06 AM, Peter Crosthwaite
<address@hidden> wrote:
> On Thu, Feb 26, 2015 at 7:38 PM, Alistair Francis
> <address@hidden> wrote:
>> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
>> <address@hidden> wrote:
>>> Hi Peter and all,
>>>
>>> Xilinx's next gen SoC has been announced. This series adds a SoC and
>>> machine model.
>>>
>>> Series start with addition of ARM cortex A53 support (P1 and P2). The
>>> Soc skeleton is then added with GIC, EMACs and UARTs are added. The
>>> pre-existing models for GEM and UART are not SoC friendly (no visible
>>> state struct), so those are refactored for SoC.
>>>
>>> Create a generic machine model that exposes just the RAW SoC itself. The
>>> only external device modelled is DDR RAM, as driven by the -m option.
>>> The standard bootloader and PSCI support is used.
>>>
>>> Regards,
>>> Peter
>>>
>>>
>>> Peter Crosthwaite (15):
>>>   target-arm: cpu64: Factor out ARM cortex init
>>>   target-arm: cpu64: Add support for cortex-a53
>>>   arm: Introduce Xilinx Zynq MPSoC
>>>   arm: xlnx-zynq-mp: Add GIC
>>>   arm: xlnx-zynq-mp: Connect CPU Timers to GIC
>>>   net: cadence_gem: Clean up variable names
>>>   net: cadence_gem: Split state struct and type into header
>>>   arm: xilinx-zynq-mp: Add GEM support
>>>   char: cadence_uart: Clean up variable names
>>>   char: cadence_uart: Split state struct and type into header
>>>   arm: xilinx-zynq-mp: Add UART support
>>>   arm: Add xilinx-zynq-mp-generic machine
>>>   arm: xilinx-zynq-mp-generic: Add external RAM
>>>   arm: xilinx-zynq-mp-generic: Add bootloading
>>>   arm: xlnx-zynq-mp: Add PSCI setup
>>>
>>>  default-configs/aarch64-softmmu.mak |   2 +-
>>>  hw/arm/Makefile.objs                |   1 +
>>>  hw/arm/xlnx-zynq-mp-generic.c       |  67 +++++++++++++++
>>>  hw/arm/xlnx-zynq-mp.c               | 167 
>>> ++++++++++++++++++++++++++++++++++++
>>
>> These two files need to be renamed. You can' tell what each one is
>> from the title.

Just re-read this, sorry about the 'need to be'.

>>
>
> So the unqualified "zynqmp" should imply the SoC. This currently
> applies to allwinner a10 and digic so Im trying to follow that
> convention. When we start using specific boards, the zynqmp will be

I see your point, but Allwinner only have one file in the hw/arm/ folder
that starts with 'allwinner', making it much clearer what it is. Digic and
Exynos also have the '_boards' suffix on their boards, which separates
the SoC from the board/boards.

With that logic the 'xlnx-zynq-mp-generic.c' probably should have a
'board' in there somewhere.

> dropped from the name completely. We could instead use the specific
> board name ep108 but I wanted that to be additive. The board would be
> called:
>
> hw/arm/xlnx-ep108.c
>
> by current conventions. One alternative would be just go out with
> ep108 board only and no generic machine if you think that works
> better?

That naming is clearer, but only if the user knows what the EP108 is.
If they are just looking to 'boot Xilinx's ZynqMP chip' then it might be a
bit confusing. In saying that, I don't think it's a big issue and I would
be fine with having a 'xlnx-ep108.c' board

>
>> It should be something like 'xilinx_zynqmp_soc.c' and
>> 'xilinx_zynqmp_machine.c' (or maybe just 'xilinx_zynqmp.c').
>>
>
> I'm trying to start using the truncated vendor name xlnx instead of
> xilinx going forward (there is already in-tree precedent) as the 3
> saved chars does save on a few 80 char line wraps. This is also the
> convention used in device tree compatible strings.

Ok, makes sense. Is it possible to change the current 'xilinx_zynq.c'
file then, that way everything matches? I guess that could break current
implementations though (epically if the actual machine name changes).

Thanks,

Alistair

>
> Regards,
> Peter
>
>> That way it also matches the same style as the current Zynq machine.
>>
>> Thanks,
>>
>> Alistair
>>
>>>  hw/char/cadence_uart.c              | 113 ++++++++++--------------
>>>  hw/net/cadence_gem.c                |  95 ++++++--------------
>>>  include/hw/arm/xlnx-zynq-mp.h       |  29 +++++++
>>>  include/hw/char/cadence_uart.h      |  35 ++++++++
>>>  include/hw/net/cadence_gem.h        |  49 +++++++++++
>>>  target-arm/cpu64.c                  |  47 +++++++---
>>>  10 files changed, 456 insertions(+), 149 deletions(-)
>>>  create mode 100644 hw/arm/xlnx-zynq-mp-generic.c
>>>  create mode 100644 hw/arm/xlnx-zynq-mp.c
>>>  create mode 100644 include/hw/arm/xlnx-zynq-mp.h
>>>  create mode 100644 include/hw/char/cadence_uart.h
>>>  create mode 100644 include/hw/net/cadence_gem.h
>>>
>>> --
>>> 2.3.0.1.g27a12f1
>>>
>>>
>>
>