Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

[Peter Maydell <peter.maydell@linaro.org>]

On 30 October 2018 at 11:28, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 30/10/18 10:36, Peter Maydell wrote:
>>
>> On 29 October 2018 at 23:20, Philippe Mathieu-Daudé <philmd@redhat.com>
>> wrote:
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>   MAINTAINERS            |  1 +
>>>   hw/arm/xilinx_zynq.c   | 18 ++----------------
>>>   hw/dma/pl330.c         |  2 +-
>>>   include/hw/dma/pl330.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 45 insertions(+), 17 deletions(-)
>>>   create mode 100644 include/hw/dma/pl330.h
>>
>>
>>> +static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
>>> +{
>>> +    SysBusDevice *busdev;
>>> +    DeviceState *dev;
>>> +
>>> +    dev = qdev_create(NULL, TYPE_PL330);
>>> +    qdev_prop_set_uint8(dev, "num_chnls", 8);
>>> +    qdev_prop_set_uint8(dev, "num_periph_req", nreq);
>>> +    qdev_prop_set_uint8(dev, "num_events", 16);
>>> +    qdev_prop_set_uint8(dev, "data_width", 64);
>>> +    qdev_prop_set_uint8(dev, "wr_cap", 8);
>>> +    qdev_prop_set_uint8(dev, "wr_q_dep", 16);
>>> +    qdev_prop_set_uint8(dev, "rd_cap", 8);
>>> +    qdev_prop_set_uint8(dev, "rd_q_dep", 16);
>>> +    qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
>>> +    qdev_init_nofail(dev);
>>
>>
>> These are the settings the Xilinx board uses, but are
>> they really the settings every SoC that has a PL330 will use ?
>
>
> Except "num_periph_req", all are pl330_properties defaults.

If they're all the device's defaults there's not much point
in setting them by hand. But my point is that the reason they're
properties is that in the real hardware these are configurable
values in the RTL. So any given SoC model needs to be able to
set them appropriately. Having a helper function that doesn't
let you set them makes it too easy for people modelling SoCs
not to think about the question, I think...

thanks
-- PMM