[PATCH 05/21] hw/dma/zynq: Notify devcfg on FPGA reset via SLCR control

Corvin Köhne posted 21 patches 10 months, 4 weeks ago
There is a newer version of this series
[PATCH 05/21] hw/dma/zynq: Notify devcfg on FPGA reset via SLCR control
Posted by Corvin Köhne 10 months, 4 weeks ago
From: YannickV <Y.Vossen@beckhoff.com>

When the FPGA_RST_CTRL register in the SLCR (System Level Control
Register) is written to, the devcfg (Device Configuration) should
indicate the finished reset.

Problems occure when Loaders trigger a reset via SLCR and poll for
the done flag in devcfg. Since the flag will never be set, this can
result in an endless loop.

A callback function `slcr_reset_handler` is added to the
`XlnxZynqDevcfg` structure. The `slcr_reset` function sets the
`PCFG_DONE` flag when triggered by an FPGA reset in the SLCR.
The SLCR write handler calls the `slcr_reset` function when the
FPGA reset control register (`R_FPGA_RST_CTRL`) is written with
the reset value.

Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
---
 hw/dma/xlnx-zynq-devcfg.c         |  7 +++++++
 hw/misc/zynq_slcr.c               | 16 ++++++++++++++++
 include/hw/dma/xlnx-zynq-devcfg.h |  1 +
 3 files changed, 24 insertions(+)

diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
index 03b5280228..611a57b4d4 100644
--- a/hw/dma/xlnx-zynq-devcfg.c
+++ b/hw/dma/xlnx-zynq-devcfg.c
@@ -138,6 +138,11 @@ static void xlnx_zynq_devcfg_update_ixr(XlnxZynqDevcfg *s)
     qemu_set_irq(s->irq, ~s->regs[R_INT_MASK] & s->regs[R_INT_STS]);
 }
 
+static void slcr_reset (DeviceState *dev) {
+    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
+    s->regs[R_INT_STS] |= R_INT_STS_PCFG_DONE_MASK;
+}
+
 static void xlnx_zynq_devcfg_reset(DeviceState *dev)
 {
     XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
@@ -374,6 +379,8 @@ static void xlnx_zynq_devcfg_init(Object *obj)
     XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(obj);
     RegisterInfoArray *reg_array;
 
+    s->slcr_reset_handler = slcr_reset;
+
     sysbus_init_irq(sbd, &s->irq);
 
     memory_region_init(&s->iomem, obj, "devcfg", XLNX_ZYNQ_DEVCFG_R_MAX * 4);
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index a766bab182..9b3220f354 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -26,6 +26,7 @@
 #include "qom/object.h"
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
+#include "hw/dma/xlnx-zynq-devcfg.h"
 
 #ifndef ZYNQ_SLCR_ERR_DEBUG
 #define ZYNQ_SLCR_ERR_DEBUG 0
@@ -576,6 +577,21 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
         zynq_slcr_compute_clocks(s);
         zynq_slcr_propagate_clocks(s);
         break;
+    case R_FPGA_RST_CTRL:
+        if (val == 0) {
+            Object *devcfgObject =
+                    object_resolve_type_unambiguous("xlnx.ps7-dev-cfg", NULL);
+            if (!devcfgObject) {
+                break;
+            }
+            DeviceState *devcfg = OBJECT_CHECK(DeviceState, devcfgObject,
+                                               "xlnx.ps7-dev-cfg");
+            XlnxZynqDevcfg *zynqdevcfg = XLNX_ZYNQ_DEVCFG(devcfg);
+            if (zynqdevcfg) {
+                zynqdevcfg->slcr_reset_handler(devcfg);
+            }
+        }
+        break;
     }
 }
 
diff --git a/include/hw/dma/xlnx-zynq-devcfg.h b/include/hw/dma/xlnx-zynq-devcfg.h
index 2ab054e598..f48a630c5a 100644
--- a/include/hw/dma/xlnx-zynq-devcfg.h
+++ b/include/hw/dma/xlnx-zynq-devcfg.h
@@ -56,6 +56,7 @@ struct XlnxZynqDevcfg {
     uint8_t dma_cmd_fifo_num;
 
     bool is_initialized;
+    void (*slcr_reset_handler) (DeviceState *dev);
 
     uint32_t regs[XLNX_ZYNQ_DEVCFG_R_MAX];
     RegisterInfo regs_info[XLNX_ZYNQ_DEVCFG_R_MAX];
-- 
2.49.0


Re: [PATCH 05/21] hw/dma/zynq: Notify devcfg on FPGA reset via SLCR control
Posted by Edgar E. Iglesias 9 months, 2 weeks ago
On Tue, Mar 18, 2025 at 02:07:56PM +0100, Corvin Köhne wrote:
> From: YannickV <Y.Vossen@beckhoff.com>
> 
> When the FPGA_RST_CTRL register in the SLCR (System Level Control
> Register) is written to, the devcfg (Device Configuration) should
> indicate the finished reset.
> 
> Problems occure when Loaders trigger a reset via SLCR and poll for
> the done flag in devcfg. Since the flag will never be set, this can
> result in an endless loop.
> 
> A callback function `slcr_reset_handler` is added to the
> `XlnxZynqDevcfg` structure. The `slcr_reset` function sets the
> `PCFG_DONE` flag when triggered by an FPGA reset in the SLCR.
> The SLCR write handler calls the `slcr_reset` function when the
> FPGA reset control register (`R_FPGA_RST_CTRL`) is written with
> the reset value.

Could you please refer to the specs where this is described?
I couldn't find it...



> 
> Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
> ---
>  hw/dma/xlnx-zynq-devcfg.c         |  7 +++++++
>  hw/misc/zynq_slcr.c               | 16 ++++++++++++++++
>  include/hw/dma/xlnx-zynq-devcfg.h |  1 +
>  3 files changed, 24 insertions(+)
> 
> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
> index 03b5280228..611a57b4d4 100644
> --- a/hw/dma/xlnx-zynq-devcfg.c
> +++ b/hw/dma/xlnx-zynq-devcfg.c
> @@ -138,6 +138,11 @@ static void xlnx_zynq_devcfg_update_ixr(XlnxZynqDevcfg *s)
>      qemu_set_irq(s->irq, ~s->regs[R_INT_MASK] & s->regs[R_INT_STS]);
>  }
>  
> +static void slcr_reset (DeviceState *dev) {
> +    XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
> +    s->regs[R_INT_STS] |= R_INT_STS_PCFG_DONE_MASK;
> +}
> +
>  static void xlnx_zynq_devcfg_reset(DeviceState *dev)
>  {
>      XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
> @@ -374,6 +379,8 @@ static void xlnx_zynq_devcfg_init(Object *obj)
>      XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(obj);
>      RegisterInfoArray *reg_array;
>  
> +    s->slcr_reset_handler = slcr_reset;
> +
>      sysbus_init_irq(sbd, &s->irq);
>  
>      memory_region_init(&s->iomem, obj, "devcfg", XLNX_ZYNQ_DEVCFG_R_MAX * 4);
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index a766bab182..9b3220f354 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -26,6 +26,7 @@
>  #include "qom/object.h"
>  #include "hw/qdev-properties.h"
>  #include "qapi/error.h"
> +#include "hw/dma/xlnx-zynq-devcfg.h"
>  
>  #ifndef ZYNQ_SLCR_ERR_DEBUG
>  #define ZYNQ_SLCR_ERR_DEBUG 0
> @@ -576,6 +577,21 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
>          zynq_slcr_compute_clocks(s);
>          zynq_slcr_propagate_clocks(s);
>          break;
> +    case R_FPGA_RST_CTRL:
> +        if (val == 0) {
> +            Object *devcfgObject =
> +                    object_resolve_type_unambiguous("xlnx.ps7-dev-cfg", NULL);
> +            if (!devcfgObject) {
> +                break;
> +            }
> +            DeviceState *devcfg = OBJECT_CHECK(DeviceState, devcfgObject,
> +                                               "xlnx.ps7-dev-cfg");
> +            XlnxZynqDevcfg *zynqdevcfg = XLNX_ZYNQ_DEVCFG(devcfg);
> +            if (zynqdevcfg) {
> +                zynqdevcfg->slcr_reset_handler(devcfg);
> +            }
> +        }
> +        break;
>      }
>  }
>  
> diff --git a/include/hw/dma/xlnx-zynq-devcfg.h b/include/hw/dma/xlnx-zynq-devcfg.h
> index 2ab054e598..f48a630c5a 100644
> --- a/include/hw/dma/xlnx-zynq-devcfg.h
> +++ b/include/hw/dma/xlnx-zynq-devcfg.h
> @@ -56,6 +56,7 @@ struct XlnxZynqDevcfg {
>      uint8_t dma_cmd_fifo_num;
>  
>      bool is_initialized;
> +    void (*slcr_reset_handler) (DeviceState *dev);
>  
>      uint32_t regs[XLNX_ZYNQ_DEVCFG_R_MAX];
>      RegisterInfo regs_info[XLNX_ZYNQ_DEVCFG_R_MAX];
> -- 
> 2.49.0
> 
Re: [PATCH 05/21] hw/dma/zynq: Notify devcfg on FPGA reset via SLCR control
Posted by Corvin Köhne 9 months ago
On Fri, 2025-04-25 at 18:11 +0200, Edgar E. Iglesias wrote:
> CAUTION: External Email!!
> On Tue, Mar 18, 2025 at 02:07:56PM +0100, Corvin Köhne wrote:
> > From: YannickV <Y.Vossen@beckhoff.com>
> > 
> > When the FPGA_RST_CTRL register in the SLCR (System Level Control
> > Register) is written to, the devcfg (Device Configuration) should
> > indicate the finished reset.
> > 
> > Problems occure when Loaders trigger a reset via SLCR and poll for
> > the done flag in devcfg. Since the flag will never be set, this can
> > result in an endless loop.
> > 
> > A callback function `slcr_reset_handler` is added to the
> > `XlnxZynqDevcfg` structure. The `slcr_reset` function sets the
> > `PCFG_DONE` flag when triggered by an FPGA reset in the SLCR.
> > The SLCR write handler calls the `slcr_reset` function when the
> > FPGA reset control register (`R_FPGA_RST_CTRL`) is written with
> > the reset value.
> 
> Could you please refer to the specs where this is described?
> I couldn't find it...
> 
> 

Looks like we've misread the specs and our loader code. Our loader writes a one
to PCFG_DONE and FPGA_RST_CTRL and then polls PCFG_DONE, so we thought that it's
related. However, we've rechecked it and on hardware PCFG_DONE isn't reset on
this write. According to the spec, PCFG_DONE is a Write 1 to Clear register but
it won't reset when the condition for setting PCFG_DONE is still true. We're
going to fix this in v2, thanks.


-- 
Kind regards,
Corvin