linux-kernel - Re: [PATCH V6 3/5] remoteproc: qcom: Split the head and tail of the q5v5-pil rproc reset function

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]

Date:   Fri, 18 May 2018 17:52:33 +0530
From:   Vinod <vkoul@...nel.org>
To:     Sricharan R <sricharan@...eaurora.org>
Cc:     bjorn.andersson@...aro.org, ohad@...ery.com, robh+dt@...nel.org,
        mark.rutland@....com, andy.gross@...aro.org,
        david.brown@...aro.org, linux-remoteproc@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        sibis@...eaurora.org
Subject: Re: [PATCH V6 3/5] remoteproc: qcom: Split the head and tail of the
 q5v5-pil rproc reset function

On 14-05-18, 16:16, Sricharan R wrote:

> +static int q6v5_reset(struct q6v5 *qproc)
> +{
> +	u32 ret;
> +	int val, i;
> +
> +	/* Assert resets, stop core */
> +	val = readl(qproc->reg_base + QDSP6SS_RESET_REG);
> +	val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
> +	writel(val, qproc->reg_base + QDSP6SS_RESET_REG);
> +
> +	/* BHS require xo cbcr to be enabled */
> +	val = readl(qproc->reg_base + QDSP6SS_XO_CBCR);
> +	val |= 0x1;
> +	writel(val, qproc->reg_base + QDSP6SS_XO_CBCR);

consider adding a updatel macro which does read, update and write for you...

> +
> +	/* Read CLKOFF bit to go low indicating CLK is enabled */
> +	ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_XO_CBCR,
> +				 val, !(val & BIT(31)), 1,
> +				 HALT_CHECK_MAX_LOOPS);
> +	if (ret) {
> +		dev_err(qproc->dev,
> +			"xo cbcr enabling timed out (rc:%d)\n", ret);
> +		return ret;
> +	}
> +	/* Enable power block headswitch and wait for it to stabilize */
> +	val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +	val |= QDSP6v56_BHS_ON;
> +	writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +	val |= readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);

why is this read required

> +	udelay(1);
> +
> +	/* Put LDO in bypass mode */
> +	val |= QDSP6v56_LDO_BYP;
> +	writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +
> +	/* Deassert QDSP6 compiler memory clamp */
> +	val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +	val &= ~QDSP6v56_CLAMP_QMC_MEM;
> +	writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +
> +	/* Deassert memory peripheral sleep and L2 memory standby */
> +	val |= Q6SS_L2DATA_STBY_N | Q6SS_SLP_RET_N;
> +	writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +
> +	/* Turn on L1, L2, ETB and JU memories 1 at a time */
> +	val = readl(qproc->reg_base + QDSP6SS_MEM_PWR_CTL);
> +	for (i = 19; i >= 0; i--) {

where is the magic number 19 coming from?
-- 
~Vinod