On Mon, Jan 31, 2022 at 03:35:57PM +0000, Francisco Iglesias wrote:
> Hi Edgar,
>
> A couple of minor comments below, looks good to me otherwise!
>
> On Mon, Jan 31, 2022 at 12:12:03AM +0100, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Add a model of the Xilinx ZynqMP CRF. At the moment this
> > is mostly a stub model.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> > include/hw/misc/xlnx-zynqmp-crf.h | 209 +++++++++++++++++++++++
> > hw/misc/xlnx-zynqmp-crf.c | 270 ++++++++++++++++++++++++++++++
> > hw/misc/meson.build | 1 +
> > 3 files changed, 480 insertions(+)
> > create mode 100644 include/hw/misc/xlnx-zynqmp-crf.h
> > create mode 100644 hw/misc/xlnx-zynqmp-crf.c
> >
> > diff --git a/include/hw/misc/xlnx-zynqmp-crf.h
> > b/include/hw/misc/xlnx-zynqmp-crf.h
> > new file mode 100644
> > index 0000000000..b173ea4a08
> > --- /dev/null
> > +++ b/include/hw/misc/xlnx-zynqmp-crf.h
> > @@ -0,0 +1,209 @@
> > +/*
> > + * QEMU model of the CRF - Clock Reset FPD.
> > + *
> > + * Copyright (c) 2022 Xilinx Inc.
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + * Written by Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > + */
> > +
>
> Include guard seem to be missing:
>
> #ifndef XLNX_ZYNQMP_CRF_H
> #define XLNX_ZYNQMP_CRF_H
>
>
> > +#include "hw/sysbus.h"
> > +#include "hw/register.h"
> > +
> > +#define TYPE_XLNX_ZYNQMP_CRF "xlnx.zynqmp_crf"
> > +
> > +#define XILINX_CRF(obj) \
> > + OBJECT_CHECK(XlnxZynqMPCRF, (obj), TYPE_XLNX_ZYNQMP_CRF)
> > +
> > +REG32(ERR_CTRL, 0x0)
>
> (Optional but in case the these ones wont be used outside
> xlnx-zynqmp-crf.c we could consider placing them there).
Hi,
Yeah, I thought about that but the issue is with defining:
#define CRF_R_MAX (R_RST_DDR_SS + 1)
We could move these regdefs out and hardcode CRF_R_MAX, e.g:
#define CRF_R_MAX (0x108 / 4)
But that seems error-phrone.
The trade-off is keeping them here and pollute the name-space.
I already to rename one of these macros to avoid conflict...
Does someone have other ideas or preferences?
Cheers,
Edgar
....
> > +REG32(RST_DDR_SS, 0x108)
> > + FIELD(RST_DDR_SS, DDR_RESET, 3, 1)
> > + FIELD(RST_DDR_SS, APM_RESET, 2, 1)
> > +
> > +#define CRF_R_MAX (R_RST_DDR_SS + 1)
> > +
> > +typedef struct XlnxZynqMPCRF {
> > + SysBusDevice parent_obj;
> > + MemoryRegion iomem;
> > + qemu_irq irq_ir;
> > +
> > + RegisterInfoArray *reg_array;
> > + uint32_t regs[CRF_R_MAX];
> > + RegisterInfo regs_info[CRF_R_MAX];
> > +} XlnxZynqMPCRF;
