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

[Francisco Iglesias]

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