linux-kernel - Re: [PATCH] hwrng: imx-rngc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wmegi4ys.fsf@kamlesh.mail-host-address-is-not-set>
Date: Mon, 27 Jan 2025 22:12:03 +0530
From: Kamlesh Gurudasani <kamlesh@...com>
To: Martin Kaiser <martin@...ser.cx>, Herbert Xu <herbert@...dor.apana.org.au>
CC: <linux-crypto@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, Martin Kaiser <martin@...ser.cx>
Subject: Re: [PATCH] hwrng: imx-rngc - add runtime pm

Martin Kaiser <martin@...ser.cx> writes:
...
> @@ -169,7 +178,11 @@ static int imx_rngc_init(struct hwrng *rng)
>  {
>  	struct imx_rngc *rngc = container_of(rng, struct imx_rngc, rng);
>  	u32 cmd, ctrl;
> -	int ret;
> +	int ret, err;
> +
> +	err = pm_runtime_resume_and_get(rngc->dev);
> +	if (err)
> +		return err;
>  
>  	/* clear error */
>  	cmd = readl(rngc->base + RNGC_COMMAND);
> @@ -186,15 +199,15 @@ static int imx_rngc_init(struct hwrng *rng)
>  		ret = wait_for_completion_timeout(&rngc->rng_op_done,
>  						  msecs_to_jiffies(RNGC_SEED_TIMEOUT));
>  		if (!ret) {
> -			ret = -ETIMEDOUT;
> -			goto err;
> +			err = -ETIMEDOUT;
> +			goto out;
>  		}
>  
>  	} while (rngc->err_reg == RNGC_ERROR_STATUS_STAT_ERR);
>  
>  	if (rngc->err_reg) {
> -		ret = -EIO;
> -		goto err;
> +		err = -EIO;
> +		goto out;
>  	}
>  
>  	/*
> @@ -205,23 +218,30 @@ static int imx_rngc_init(struct hwrng *rng)
>  	ctrl |= RNGC_CTRL_AUTO_SEED;
>  	writel(ctrl, rngc->base + RNGC_CONTROL);
>  
> +	err = 0;
is this really needed? The only time control reaches here when err = 0
in below equation
	err = pm_runtime_resume_and_get(rngc->dev);
	if (err)
		return err;

or am I missing something?

Regards,
Kamlesh

> +out:
>  	/*
>  	 * if initialisation was successful, we keep the interrupt
>  	 * unmasked until imx_rngc_cleanup is called
>  	 * we mask the interrupt ourselves if we return an error
>  	 */
> -	return 0;
> +	if (err)
> +		imx_rngc_irq_mask_clear(rngc);
>  
> -err: