[PATCH v2 05/14] hw/dma/zynq: Ensure PCFG_DONE bit remains set to indicate PL is in user mode

Corvin Köhne posted 14 patches 3 months ago
There is a newer version of this series
[PATCH v2 05/14] hw/dma/zynq: Ensure PCFG_DONE bit remains set to indicate PL is in user mode
Posted by Corvin Köhne 3 months ago
From: YannickV <Y.Vossen@beckhoff.com>

All register bits are clear on write by writing 1s to those bits, however
the register bits will only be cleared if the condition that sets the
interrupt flag is no longer true. Since we can assume that programming
is always done, the `PCFG_DONE` flag is always set to 1, so it will not
never be cleared.

Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
---
 hw/dma/xlnx-zynq-devcfg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
index f28d0015e6..60ea351494 100644
--- a/hw/dma/xlnx-zynq-devcfg.c
+++ b/hw/dma/xlnx-zynq-devcfg.c
@@ -188,6 +188,8 @@ static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
 {
     XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
 
+    s->regs[R_INT_STS] |= R_INT_STS_PCFG_DONE_MASK;
+    
     xlnx_zynq_devcfg_update_ixr(s);
 }
 
-- 
2.50.1


Re: [PATCH v2 05/14] hw/dma/zynq: Ensure PCFG_DONE bit remains set to indicate PL is in user mode
Posted by Edgar E. Iglesias 2 months, 3 weeks ago
On Fri, Aug 15, 2025 at 11:01:03AM +0200, Corvin Köhne wrote:
> From: YannickV <Y.Vossen@beckhoff.com>
> 
> All register bits are clear on write by writing 1s to those bits, however
> the register bits will only be cleared if the condition that sets the
> interrupt flag is no longer true. Since we can assume that programming
> is always done, the `PCFG_DONE` flag is always set to 1, so it will not
> never be cleared.
> 
> Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
> ---
>  hw/dma/xlnx-zynq-devcfg.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
> index f28d0015e6..60ea351494 100644
> --- a/hw/dma/xlnx-zynq-devcfg.c
> +++ b/hw/dma/xlnx-zynq-devcfg.c
> @@ -188,6 +188,8 @@ static void r_ixr_post_write(RegisterInfo *reg, uint64_t val)
>  {
>      XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
>  
> +    s->regs[R_INT_STS] |= R_INT_STS_PCFG_DONE_MASK;
> +    

Looks like you've got some stray spaces in the empty line.

I'm fine with this but another way to handle PCFG_DONE could be to have
some state that goes true after the first programming. e.g:

s->regs[R_INT_STS] |= s->pcfg_done ? R_INT_STS_PCFG_DONE_MASK : 0;

On the other hand, for direct Linux boots we may want this to be always
one...

Anyway, with the whitespace fixes:

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



>      xlnx_zynq_devcfg_update_ixr(s);
>  }
>  
> -- 
> 2.50.1
>