LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Sibi S <sibis@codeaurora.org>
Cc: p.zabel@pengutronix.de, robh+dt@kernel.org,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, georgi.djakov@linaro.org,
	jassisinghbrar@gmail.com, ohad@wizery.com, mark.rutland@arm.com,
	kyan@codeaurora.org, sricharan@codeaurora.org,
	akdwived@codeaurora.org, linux-arm-msm@vger.kernel.org,
	tsoni@codeaurora.org
Subject: Re: [PATCH v3 7/7] remoteproc: qcom: Always assert and deassert reset signals in SDM845
Date: Sun, 18 Mar 2018 15:46:31 -0700	[thread overview]
Message-ID: <20180318224631.GH5626@tuxbook-pro> (raw)
In-Reply-To: <1521019283-32212-8-git-send-email-sibis@codeaurora.org>

On Wed 14 Mar 02:21 PDT 2018, Sibi S wrote:
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
[..]
> +struct q6v5_reset_ops {
> +	int (*reset_start)(struct q6v5 *qproc);
> +	int (*reset_stop)(struct q6v5 *qproc);
> +};

Please add these two ops directly in q6v5 instead and please keep the
naming "reset_assert" and "reset_deassert".

> +
>  enum {
>  	MSS_MSM8916,
>  	MSS_MSM8974,
> @@ -343,6 +354,52 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
>  	return 0;
>  }
>  
> +static void alt_reset_restart(struct q6v5 *qproc, u32 restart)
> +{
> +	writel(restart, qproc->rmb_base + RMB_MBA_ALT_RESET);

Just move this write into q6v5_sdm_reset_start()

> +}
> +
> +static int q6v5_msm_reset_stop(struct q6v5 *qproc)
> +{
> +	return reset_control_assert(qproc->mss_restart);
> +}
> +
> +static int q6v5_msm_reset_start(struct q6v5 *qproc)
> +{
> +	return reset_control_deassert(qproc->mss_restart);
> +}
> +
> +static int q6v5_sdm_reset_stop(struct q6v5 *qproc)
> +{
> +	return reset_control_reset(qproc->mss_restart);
> +}
> +
> +static int q6v5_sdm_reset_start(struct q6v5 *qproc)
> +{
> +	int ret;
> +
> +	alt_reset_restart(qproc, 1);
> +	/* Ensure alt reset is written before restart reg */

That's not what udelay does ;)

If you want to make sure that the register is written and then give it
100us delay until you reset the reset you have to readl() the same
register back after the writel()

I think the delay deserves a comment on what we're waiting for.

> +	udelay(100);

Use usleep_range() for delays longer than 10us.

> +
> +	ret = reset_control_reset(qproc->mss_restart);
> +
> +	udelay(100);

Same as the delay above, what are we waiting for?

> +	alt_reset_restart(qproc, 0);
> +
> +	return ret;
> +}
> +
[..]
> @@ -1179,6 +1251,12 @@ static int q6v5_probe(struct platform_device *pdev)
>  	qproc->dev = &pdev->dev;
>  	qproc->rproc = rproc;
>  	platform_set_drvdata(pdev, qproc);
> +	qproc->version = desc->version;
> +
> +	if (qproc->version == MSS_SDM845)
> +		qproc->ops = &q6v5_sdm_ops;
> +	else
> +		qproc->ops = &q6v5_msm_ops;

Can we carry a boolean for "has_alt_reset" or something that picks the
new reset ops, rather than picking by version?

Regards,
Bjorn

  reply	other threads:[~2018-03-18 22:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-14  9:21 [PATCH v3 0/7] Add support for remoteproc modem-pil on SDM845 SoCs Sibi S
2018-03-14  9:21 ` [PATCH v3 1/7] dt-bindings: reset: Add AOSS reset bindings for " Sibi S
2018-03-18 12:49   ` Rob Herring
2018-03-18 22:44   ` Bjorn Andersson
2018-03-21  6:29     ` Sibi S
2018-03-14  9:21 ` [PATCH v3 2/7] reset: qcom: AOSS (always on subsystem) reset controller Sibi S
2018-03-14 10:48   ` Vivek Gautam
2018-03-18 22:45   ` Bjorn Andersson
2018-03-22 22:32     ` Sibi S
2018-03-14  9:21 ` [PATCH v3 3/7] dt-bindings: mailbox: Add APCS global binding for SDM845 SoCs Sibi S
2018-03-18 22:45   ` Bjorn Andersson
2018-03-14  9:21 ` [PATCH v3 4/7] mailbox: Add support for Qualcomm " Sibi S
2018-03-18 22:45   ` Bjorn Andersson
2018-04-19 17:22   ` Bjorn Andersson
2018-03-14  9:21 ` [PATCH v3 5/7] dt-bindings: remoteproc: Add Q6v5 Modem PIL binding for SDM845 Sibi S
2018-03-18 22:46   ` Bjorn Andersson
2018-03-14  9:21 ` [PATCH v3 6/7] remoteproc: qcom: Add support for mss remoteproc on SDM845 Sibi S
2018-03-14  9:21 ` [PATCH v3 7/7] remoteproc: qcom: Always assert and deassert reset signals in SDM845 Sibi S
2018-03-18 22:46   ` Bjorn Andersson [this message]
2018-03-22 22:10     ` Sibi S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180318224631.GH5626@tuxbook-pro \
    --to=bjorn.andersson@linaro.org \
    --cc=akdwived@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=georgi.djakov@linaro.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=kyan@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ohad@wizery.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --cc=sibis@codeaurora.org \
    --cc=sricharan@codeaurora.org \
    --cc=tsoni@codeaurora.org \
    --subject='Re: [PATCH v3 7/7] remoteproc: qcom: Always assert and deassert reset signals in SDM845' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).