[v4] Xilinx Versal's PMC SLCR and OSPI support

[PATCH v4 02/11] hw/arm/xlnx-versal: Connect Versal's PMC SLCR

Posted by Francisco Iglesias 4 years, 2 months ago

Connect Versal's PMC SLCR (system-level control registers) model.

Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
---
 hw/arm/xlnx-versal.c         | 17 +++++++++++++++++
 include/hw/arm/xlnx-versal.h |  6 ++++++
 2 files changed, 23 insertions(+)

diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index b2705b6925..bb13b07e79 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -369,6 +369,22 @@ static void versal_create_efuse(Versal *s, qemu_irq *pic)
     sysbus_connect_irq(SYS_BUS_DEVICE(ctrl), 0, pic[VERSAL_EFUSE_IRQ]);
 }
 
+static void versal_create_pmc_iou_slcr(Versal *s, qemu_irq *pic)
+{
+    SysBusDevice *sbd;
+
+    object_initialize_child(OBJECT(s), "versal-pmc-iou-slcr", &s->pmc.iou.slcr,
+                            TYPE_XILINX_VERSAL_PMC_IOU_SLCR);
+
+    sbd = SYS_BUS_DEVICE(&s->pmc.iou.slcr);
+    sysbus_realize(sbd, &error_fatal);
+
+    memory_region_add_subregion(&s->mr_ps, MM_PMC_PMC_IOU_SLCR,
+                                sysbus_mmio_get_region(sbd, 0));
+
+    sysbus_connect_irq(sbd, 0, pic[VERSAL_PMC_IOU_SLCR_IRQ]);
+}
+
 /* This takes the board allocated linear DDR memory and creates aliases
  * for each split DDR range/aperture on the Versal address map.
  */
@@ -459,6 +475,7 @@ static void versal_realize(DeviceState *dev, Error **errp)
     versal_create_xrams(s, pic);
     versal_create_bbram(s, pic);
     versal_create_efuse(s, pic);
+    versal_create_pmc_iou_slcr(s, pic);
     versal_map_ddr(s);
     versal_unimp(s);
 
diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
index 895ba12c61..729c093dfc 100644
--- a/include/hw/arm/xlnx-versal.h
+++ b/include/hw/arm/xlnx-versal.h
@@ -26,6 +26,7 @@
 #include "hw/misc/xlnx-versal-xramc.h"
 #include "hw/nvram/xlnx-bbram.h"
 #include "hw/nvram/xlnx-versal-efuse.h"
+#include "hw/misc/xlnx-versal-pmc-iou-slcr.h"
 
 #define TYPE_XLNX_VERSAL "xlnx-versal"
 OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
@@ -78,6 +79,7 @@ struct Versal {
     struct {
         struct {
             SDHCIState sd[XLNX_VERSAL_NR_SDS];
+            XlnxVersalPmcIouSlcr slcr;
         } iou;
 
         XlnxZynqMPRTC rtc;
@@ -113,6 +115,7 @@ struct Versal {
 #define VERSAL_XRAM_IRQ_0          79
 #define VERSAL_BBRAM_APB_IRQ_0     121
 #define VERSAL_RTC_APB_ERR_IRQ     121
+#define VERSAL_PMC_IOU_SLCR_IRQ    121
 #define VERSAL_SD0_IRQ_0           126
 #define VERSAL_EFUSE_IRQ           139
 #define VERSAL_RTC_ALARM_IRQ       142
@@ -178,6 +181,9 @@ struct Versal {
 #define MM_FPD_FPD_APU              0xfd5c0000
 #define MM_FPD_FPD_APU_SIZE         0x100
 
+#define MM_PMC_PMC_IOU_SLCR         0xf1060000
+#define MM_PMC_PMC_IOU_SLCR_SIZE    0x10000
+
 #define MM_PMC_SD0                  0xf1040000U
 #define MM_PMC_SD0_SIZE             0x10000
 #define MM_PMC_BBRAM_CTRL           0xf11f0000
-- 
2.11.0

Re: [PATCH v4 02/11] hw/arm/xlnx-versal: Connect Versal's PMC SLCR

Posted by Peter Maydell 4 years, 2 months ago

On Wed, 1 Dec 2021 at 15:40, Francisco Iglesias
<francisco.iglesias@xilinx.com> wrote:
>
> Connect Versal's PMC SLCR (system-level control registers) model.
>
> Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 895ba12c61..729c093dfc 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -26,6 +26,7 @@
>  #include "hw/misc/xlnx-versal-xramc.h"
>  #include "hw/nvram/xlnx-bbram.h"
>  #include "hw/nvram/xlnx-versal-efuse.h"
> +#include "hw/misc/xlnx-versal-pmc-iou-slcr.h"
>
>  #define TYPE_XLNX_VERSAL "xlnx-versal"
>  OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
> @@ -78,6 +79,7 @@ struct Versal {
>      struct {
>          struct {
>              SDHCIState sd[XLNX_VERSAL_NR_SDS];
> +            XlnxVersalPmcIouSlcr slcr;
>          } iou;
>
>          XlnxZynqMPRTC rtc;
> @@ -113,6 +115,7 @@ struct Versal {
>  #define VERSAL_XRAM_IRQ_0          79
>  #define VERSAL_BBRAM_APB_IRQ_0     121
>  #define VERSAL_RTC_APB_ERR_IRQ     121
> +#define VERSAL_PMC_IOU_SLCR_IRQ    121

This looks weird -- are these devices really all the same
IRQ number ? If so, you need to create an OR gate and wire
the devices to the OR gate and the OR gate output to the
qemu irq in pic[121].

This seems to be a preexisting bug in this code, because
both VERSAL_BBRAM_APB_IRQ_0 and VERSAL_RTC_APB_ERR_IRQ are
being used despite being the same value. I would suggest
a patch before this one to fix that bug (either by correcting the
IRQ numbers if they're wrong, or by adding an initially 2-input
OR gate for them).

thanks
-- PMM

Re: [PATCH v4 02/11] hw/arm/xlnx-versal: Connect Versal's PMC SLCR

Posted by Francisco Iglesias 4 years, 1 month ago

On Fri, Dec 10, 2021 at 03:16:59PM +0000, Peter Maydell wrote:
> On Wed, 1 Dec 2021 at 15:40, Francisco Iglesias
> <francisco.iglesias@xilinx.com> wrote:
> >
> > Connect Versal's PMC SLCR (system-level control registers) model.
> >
> > Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> > diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> > index 895ba12c61..729c093dfc 100644
> > --- a/include/hw/arm/xlnx-versal.h
> > +++ b/include/hw/arm/xlnx-versal.h
> > @@ -26,6 +26,7 @@
> >  #include "hw/misc/xlnx-versal-xramc.h"
> >  #include "hw/nvram/xlnx-bbram.h"
> >  #include "hw/nvram/xlnx-versal-efuse.h"
> > +#include "hw/misc/xlnx-versal-pmc-iou-slcr.h"
> >
> >  #define TYPE_XLNX_VERSAL "xlnx-versal"
> >  OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
> > @@ -78,6 +79,7 @@ struct Versal {
> >      struct {
> >          struct {
> >              SDHCIState sd[XLNX_VERSAL_NR_SDS];
> > +            XlnxVersalPmcIouSlcr slcr;
> >          } iou;
> >
> >          XlnxZynqMPRTC rtc;
> > @@ -113,6 +115,7 @@ struct Versal {
> >  #define VERSAL_XRAM_IRQ_0          79
> >  #define VERSAL_BBRAM_APB_IRQ_0     121
> >  #define VERSAL_RTC_APB_ERR_IRQ     121
> > +#define VERSAL_PMC_IOU_SLCR_IRQ    121
> 
> This looks weird -- are these devices really all the same
> IRQ number ? If so, you need to create an OR gate and wire
> the devices to the OR gate and the OR gate output to the
> qemu irq in pic[121].
> 
> This seems to be a preexisting bug in this code, because
> both VERSAL_BBRAM_APB_IRQ_0 and VERSAL_RTC_APB_ERR_IRQ are
> being used despite being the same value. I would suggest
> a patch before this one to fix that bug (either by correcting the
> IRQ numbers if they're wrong, or by adding an initially 2-input
> OR gate for them).


Hi again Peter,

I added a new patch in v5 with a correction, the orgate you mentioned
above (the interrupt should be or'ed exactly as you assumed)!

Best regards,
Francisco

> 
> thanks
> -- PMM