Hi Neil,

On Friday, 31 January 2025 at 10:42, Neil Armstrong <neil.armstr...@linaro.org> 
wrote:

> 
> 
> On 27/01/2025 08:49, Sam Day wrote:
> 
> > msm_rng_enable is supposed to skip writing to LFSR_CFG + CONFIG
> > registers in the PRNG_ block if PRNG_CONFIG_HW_ENABLE is already set.
> > The logic to test for this was inverted.
> > 
> > Without this fix, the driver was causing SError aborts on my MSM8916
> > device. Stephan Gerhold suggested this was probably because TZ has
> > marked this as a protected register, since it would also be using it for
> > RNG.
> > 
> > Fixes: 033ec636fcb ("rng: Add Qualcomm MSM PRNG driver")
> > Signed-off-by: Sam Day m...@samcday.com
> > ---
> > drivers/rng/msm_rng.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/rng/msm_rng.c b/drivers/rng/msm_rng.c
> > index 
> > f790d3b60f99bfc04ecd9acdd21405a8c38c257a..01505509103f28f7c91dd47edb83c23359163c82
> >  100644
> > --- a/drivers/rng/msm_rng.c
> > +++ b/drivers/rng/msm_rng.c
> > @@ -76,7 +76,7 @@ static int msm_rng_enable(struct msm_rng_priv priv, int 
> > enable)
> > if (enable) {
> > / Enable PRNG only if it is not already enabled */
> > val = readl_relaxed(priv->base + PRNG_CONFIG);
> > - if (val & PRNG_CONFIG_HW_ENABLE) {
> > + if (!(val & PRNG_CONFIG_HW_ENABLE)) {
> > val = readl_relaxed(priv->base + PRNG_LFSR_CFG);
> > val &= ~PRNG_LFSR_CFG_MASK;
> > val |= PRNG_LFSR_CFG_CLOCKS;
> 
> 
> Good catch !

I'd love to take credit, but actually it was Stephan Gerhold who spotted
this when we were debugging the issue together :)

Cheers,
-Sam


> 
> Reviewed-by: Neil Armstrong neil.armstr...@linaro.org

Reply via email to