Re: [PATCH] crypto: atmel-sha204a - drop hwrng quality reduction for ATSHA204A

[Thorsten Blum <thorsten.blum@linux.dev>]

On Tue, Apr 28, 2026 at 07:58:39AM +0200, Ard Biesheuvel wrote:
> Hi Thorsten,
> 
> On Mon, 27 Apr 2026, at 14:40, Thorsten Blum wrote:
> > Commit 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to
> > lowest possible") reduced the hwrng quality to 1 based on a review by
> > Bill Cox [1]. However, despite its title, the review only tested the
> > ATSHA204, not the ATSHA204A.
> >
> > In the same thread, Atmel engineer Landon Cox wrote "this behavior has
> > been eliminated entirely"[2] in the ATSHA204A and "this problem does not
> > affect the ATECC108 or the ATECC108A (or the ATSHA204A)"[3].
> >
> > According to the official ATSHA204A datasheet [4], the device contains a
> > high-quality hardware RNG that combines its output with an internal seed
> > value stored in EEPROM or SRAM to generate random numbers. The device
> > also implements all security functions using SHA-256, and the driver
> > uses the chip's Random command in seed-update mode.
> >
> > Keep 'quality = 1' for ATSHA204, but drop the explicit hwrng quality
> > reduction for ATSHA204A and fall back to the hwrng core default.
> >
> 
> Interesting! Thanks for digging this up.
> 
> > [1] 
> > https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
> > [2] 
> > https://www.metzdowd.com/pipermail/cryptography/2014-December/023852.html
> > [3] 
> > https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html
> > [4] 
> > https://ww1.microchip.com/downloads/en/DeviceDoc/ATSHA204A-Data-Sheet-40002025A.pdf
> >
> > Fixes: 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to 
> > lowest possible")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  drivers/crypto/atmel-sha204a.c | 40 ++++++++++++++++++----------------
> >  1 file changed, 21 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index dbb39ed0cea1..df69fb190e52 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -19,6 +19,25 @@
> >  #include <linux/workqueue.h>
> >  #include "atmel-i2c.h"
> > 
> > +enum atmel_sha204a_variant {
> > +	ATSHA204 = 1,
> > +	ATSHA204A,
> > +};
> > +
> 
> I agree that setting quality to '1' is only appropriate for the ATSHA204
> but this looks a bit clunky to me.

Moving the declarations to the top was also a leftover from a previous
approach and not necessary anymore. I'll send a v2.

Thanks,
Thorsten