Re: [PATCH v2] caam: bugfix CAAM error on startup

[Tim Harvey]

On Tue, May 20, 2025 at 5:29 AM Olaf Baehring <olaf.baehr...@draeger.com> wrote:
>
> In rare cases U-Boot returns an error message when intantiating the RNG
> of the CAAM device:
> “SEC0:  RNG4 SH0 instantiation failed with error 0xffffffff”
> This  means, that even when the CAAM device reports a finished
> descriptor, none is found in the output ring.
> This might be caused by a missing cache invalidation before
> reading the memory of the output ring
> This patch moves the cache invalidation of the output ring from start of
> the job to immediately after the notification from hardware where the
> output ring will be read.
>
> Signed-off-by: Olaf Baehring <olaf.baehr...@draeger.com>
> ---
>
> (no changes since v1)
>
>  drivers/crypto/fsl/jr.c | 30 +++++++++++++++++-------------
>  drivers/crypto/fsl/jr.h |  4 ----
>  2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 8f7a821ebf3..edf678d363e 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -217,13 +217,6 @@ static int jr_enqueue(uint32_t *desc_addr,
>
>         jr->head = (head + 1) & (jr->size - 1);
>
> -       /* Invalidate output ring */
> -       start = (unsigned long)jr->output_ring &
> -                                       ~(ARCH_DMA_MINALIGN - 1);
> -       end = ALIGN((unsigned long)jr->output_ring + jr->op_size,
> -                   ARCH_DMA_MINALIGN);
> -       invalidate_dcache_range(start, end);
> -
>         sec_out32(&regs->irja, 1);
>
>         return 0;
> @@ -243,6 +236,7 @@ static int jr_dequeue(int sec_idx, struct caam_regs *caam)
>  #else
>         uint32_t *addr;
>  #endif
> +       unsigned long start, end;
>
>         while (sec_in32(&regs->orsf) && CIRC_CNT(jr->head, jr->tail,
>                                                  jr->size)) {
> @@ -250,6 +244,14 @@ static int jr_dequeue(int sec_idx, struct caam_regs 
> *caam)
>                 found = 0;
>
>                 caam_dma_addr_t op_desc;
> +
> +               /* Invalidate output ring */
> +               start = (unsigned long)jr->output_ring &
> +                                               ~(ARCH_DMA_MINALIGN - 1);
> +               end = ALIGN((unsigned long)jr->output_ring
> +                                       + jr->op_size, ARCH_DMA_MINALIGN);
> +               invalidate_dcache_range(start, end);
> +
>         #ifdef CONFIG_CAAM_64BIT
>                 /* Read the 64 bit Descriptor address from Output Ring.
>                  * The 32 bit hign and low part of the address will
> @@ -283,28 +285,31 @@ static int jr_dequeue(int sec_idx, struct caam_regs 
> *caam)
>                 }
>
>                 /* Error condition if match not found */
> -               if (!found)
> +               if (!found) {
> +                       int slots_full = sec_in32(&regs->orsf);
> +
> +                       jr->tail = (jr->tail + slots_full) & (jr->size - 1);
> +                       sec_out32(&regs->orjr, slots_full);
>                         return -1;
> +               }
>

Hi Olaf,

I could not find v3 of your patch in my mailbox, which I think was the
version that was accepted by Fabio,

I've found that this patch causes a regression on an imx8mm board
(imx8mm_venice_defconfig) where the first call to caam_rng_read fails
here in jr_dequeue but if you call it again it works. With some
debugging added:
SEC0:  RNG instantiated
...
Hit any key to stop autoboot:  0
u-boot=> rng list
RNG #0 - caam-rng
u-boot=> rng 0 10
caam_rng_read caam-rng len=16
run_descriptor_jr_idx idx=0
Error in SEC deq: -1
caam_rng_read_one run_descriptor_jr failed: -1
caam_rng_read caam-rng caam_rng_read_one failed: -5
Reading RNG failed
u-boot=> rng 0 10
caam_rng_read caam-rng len=16
run_descriptor_jr_idx idx=0
00000000: ad 2e ad c0 2a 12 27 c4 65 82 66 19 be ef f6 07  ....*.'.e.f.....

If I revert your patch caam_rng_read works initially and on subsequent calls.

Thoughts?

Best Regards,

Tim