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

[Peter Maydell]

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