LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sricharan R <sricharan@codeaurora.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Ohad Ben-Cohen <ohad@wizery.com>,
	Sibi Sankar <sibis@codeaurora.org>,
	Rohit kumar <rohitkr@codeaurora.org>
Cc: Andy Gross <andy.gross@linaro.org>,
	linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling
Date: Fri, 1 Jun 2018 11:46:13 +0530	[thread overview]
Message-ID: <8c2acd62-0f3b-9167-b028-1f5149bd7937@codeaurora.org> (raw)
In-Reply-To: <20180523052054.19025-3-bjorn.andersson@linaro.org>

Hi Bjorn,
  Thanks for this much needed consolidation.

On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Shared between all Hexagon V5 based remoteprocs is the handling of the 5
> interrupts and the SMP2P stop request, so break this out into a separate
> function in order to allow these drivers to be cleaned up.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/remoteproc/Kconfig     |   5 +
>  drivers/remoteproc/Makefile    |   1 +
>  drivers/remoteproc/qcom_q6v5.c | 243 +++++++++++++++++++++++++++++++++
>  drivers/remoteproc/qcom_q6v5.h |  46 +++++++
>  4 files changed, 295 insertions(+)
>  create mode 100644 drivers/remoteproc/qcom_q6v5.c
>  create mode 100644 drivers/remoteproc/qcom_q6v5.h
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index cd1c168fd188..63b79ea91a21 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -102,6 +102,11 @@ config QCOM_ADSP_PIL
>  config QCOM_RPROC_COMMON
>  	tristate
>  
> +config QCOM_Q6V5_COMMON
> +	tristate
> +	depends on ARCH_QCOM
> +	depends on QCOM_SMEM
> +
>  config QCOM_Q6V5_PIL
>  	tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
>  	depends on OF && ARCH_QCOM
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 02627ede8d4a..5dd0249cf76a 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
>  obj-$(CONFIG_KEYSTONE_REMOTEPROC)	+= keystone_remoteproc.o
>  obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
>  obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
> +obj-$(CONFIG_QCOM_Q6V5_COMMON)		+= qcom_q6v5.o
>  obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
>  obj-$(CONFIG_QCOM_SYSMON)		+= qcom_sysmon.o
>  obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> new file mode 100644
> index 000000000000..9076537a1671
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm Peripheral Image Loader for Q6V5 WCSS
> + *

  Probably just Q6V5, QCSS not needed.

> + * Copyright (C) 2016-2018 Linaro Ltd.
> + * Copyright (C) 2014 Sony Mobile Communications AB
> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/smem_state.h>
> +#include <linux/remoteproc.h>
> +#include "qcom_q6v5.h"
> +
> +/**
> + * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
> + * @q6v5:	reference to qcom_q6v5 context to be reinitialized
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
> +{
> +	reinit_completion(&q6v5->start_done);
> +	reinit_completion(&q6v5->stop_done);
> +
> +	q6v5->running = true;
> +	q6v5->handover_issued = false;
> +
> +	enable_irq(q6v5->handover_irq);
> +
> +	return 0;
> +}
> +
> +/**
> + * qcom_q6v5_unprepare() - unprepare the qcom_q6v5 context after stop
> + * @q6v5:	reference to qcom_q6v5 context to be unprepared
> + *
> + * Return: 0 on success, 1 if handover hasn't yet been called
> + */
> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
> +{
> +	disable_irq(q6v5->handover_irq);
> +
> +	return !q6v5->handover_issued;
> +}
> +
> +static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
> +{
> +	struct qcom_q6v5 *q6v5 = data;
> +	size_t len;
> +	char *msg;
> +
> +	/* Sometimes the stop triggers a watchdog rather than a stop-ack */
> +	if (!q6v5->running) {
> +		complete(&q6v5->stop_done);
> +		return IRQ_HANDLED;
> +	}
> +

   Does this change the behavior for adsp pil, which was unconditionally
   doing a rproc_report_crash before, but now checks for the running flag 
   or probably this is the correct sequence ?

> +	msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
> +	if (!IS_ERR(msg) && len > 0 && msg[0])
> +		dev_err(q6v5->dev, "watchdog received: %s\n", msg);
> +	else
> +		dev_err(q6v5->dev, "watchdog without message\n");
> +
> +	rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
> +

   Should be rproc_report_crash(q6v5->rproc, RPROC_WATCHDOG);

> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
> +{
> +	struct qcom_q6v5 *q6v5 = data;
> +	size_t len;
> +	char *msg;
> +
> +	msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
> +	if (!IS_ERR(msg) && len > 0 && msg[0])
> +		dev_err(q6v5->dev, "fatal error received: %s\n", msg);
> +	else
> +		dev_err(q6v5->dev, "fatal error without message\n");
> +
> +	rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_ready_interrupt(int irq, void *data)
> +{
> +	struct qcom_q6v5 *q6v5 = data;
> +
> +	complete(&q6v5->start_done);
> +
> +	return IRQ_HANDLED;
> +}

  For adsp, previously start_done completion was done as a part of
  handover interrupt, now its done in ready. Does it mean that the
  entries in DT should be changed etc ?
> +
> +/**
> + * qcom_q6v5_wait_for_start() - wait for remote processor start signal
> + * @q6v5:	reference to qcom_q6v5 context
> + * @timeout:	timeout to wait for the event, in jiffies
> + *
> + * qcom_q6v5_unprepare() should not be called when this function fails.
> + *
> + * Return: 0 on success, -ETIMEDOUT on timeout
> + */
> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout)
> +{
> +	int ret;
> +
> +	ret = wait_for_completion_timeout(&q6v5->start_done, timeout);
> +	if (!ret)
> +		disable_irq(q6v5->handover_irq);
> +
> +	return !ret ? -ETIMEDOUT : 0;
> +}
> +
> +static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
> +{
> +	struct qcom_q6v5 *q6v5 = data;
> +
> +	if (q6v5->handover)
> +		q6v5->handover(q6v5);
> +
> +	q6v5->handover_issued = true;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_stop_interrupt(int irq, void *data)
> +{
> +	struct qcom_q6v5 *q6v5 = data;
> +
> +	complete(&q6v5->stop_done);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * qcom_q6v5_request_stop() - request the remote processor to stop
> + * @q6v5:	reference to qcom_q6v5 context
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
> +{
> +	int ret;
> +
> +	q6v5->running = false;
> +
> +	qcom_smem_state_update_bits(q6v5->state,
> +				    BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
> +
> +	ret = wait_for_completion_timeout(&q6v5->stop_done, 5 * HZ);
> +
> +	qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), 0);
> +
> +	return ret == 0 ? -ETIMEDOUT : 0;
> +}
> +
> +/**
> + * qcom_q6v5_init() - initializer of the q6v5 common struct
> + * @q6v5:	handle to be initialized
> + * @pdev:	platform_device reference for acquiring resources
> + * @rproc:	associated remoteproc instance
> + * @crash_reason: SMEM id for crash reason string, or 0 if none
> + * @handover:	function to be called when proxy resources should be released
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> +		   struct rproc *rproc, int crash_reason,
> +		   void (*handover)(struct qcom_q6v5 *q6v5))
> +{
> +	int ret;
> +
> +	q6v5->rproc = rproc;
> +	q6v5->dev = &pdev->dev;
> +	q6v5->crash_reason = crash_reason;
> +	q6v5->handover = handover;
> +
> +	init_completion(&q6v5->start_done);
> +	init_completion(&q6v5->stop_done);
> +
> +	q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
> +	ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
> +					NULL, q6v5_wdog_interrupt,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"q6v5 wdog", q6v5);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
> +		return ret;
> +	}
> +
> +	q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
> +	ret = devm_request_threaded_irq(&pdev->dev, q6v5->fatal_irq,
> +					NULL, q6v5_fatal_interrupt,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"q6v5 fatal", q6v5);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to acquire fatal IRQ\n");
> +		return ret;
> +	}
> +
> +	q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
> +	ret = devm_request_threaded_irq(&pdev->dev, q6v5->ready_irq,
> +					NULL, q6v5_ready_interrupt,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"q6v5 ready", q6v5);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to acquire ready IRQ\n");
> +		return ret;
> +	}
> +
> +	q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
> +	ret = devm_request_threaded_irq(&pdev->dev, q6v5->handover_irq,
> +					NULL, q6v5_handover_interrupt,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"q6v5 handover", q6v5);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to acquire handover IRQ\n");
> +		return ret;
> +	}
> +	disable_irq(q6v5->handover_irq);
> +
> +	q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
> +	ret = devm_request_threaded_irq(&pdev->dev, q6v5->stop_irq,
> +					NULL, q6v5_stop_interrupt,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"q6v5 stop", q6v5);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to acquire stop-ack IRQ\n");
> +		return ret;
> +	}
> +
> +	q6v5->state = qcom_smem_state_get(&pdev->dev, "stop", &q6v5->stop_bit);
> +	if (IS_ERR(q6v5->state)) {
> +		dev_err(&pdev->dev, "failed to acquire stop state\n");
> +		return PTR_ERR(q6v5->state);
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
> new file mode 100644
> index 000000000000..7ac92c1e0f49
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __QCOM_Q6V5_H__
> +#define __QCOM_Q6V5_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/completion.h>
> +
> +struct rproc;
> +struct qcom_smem_state;
> +
> +struct qcom_q6v5 {
> +	struct device *dev;
> +	struct rproc *rproc;
> +
> +	struct qcom_smem_state *state;
> +	unsigned stop_bit;
> +
> +	int wdog_irq;
> +	int fatal_irq;
> +	int ready_irq;
> +	int handover_irq;
> +	int stop_irq;
> +
> +	bool handover_issued;
> +
> +	struct completion start_done;
> +	struct completion stop_done;
> +
> +	int crash_reason;
> +
> +	bool running;
> +
> +	void (*handover)(struct qcom_q6v5 *q6v5);
> +};
> +
> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> +		   struct rproc *rproc, int crash_reason,
> +		   void (*handover)(struct qcom_q6v5 *q6v5));
> +
> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
> +
> +#endif
> 

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

  reply	other threads:[~2018-06-01  6:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-23  5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
2018-05-23  5:20 ` [RFC PATCH 1/5] remoteproc: qcom: mdt_loader: Make the firmware authentication optional Bjorn Andersson
2018-05-23  5:20 ` [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling Bjorn Andersson
2018-06-01  6:16   ` Sricharan R [this message]
2018-06-01 15:18     ` Sibi S
2018-06-01 16:39       ` Sricharan R
2018-05-23  5:20 ` [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers Bjorn Andersson
2018-06-01  6:25   ` Sricharan R
2018-06-01 13:35   ` Rohit Kumar
2018-05-23  5:20 ` [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: " Bjorn Andersson
2018-06-01  6:42   ` Sricharan R
2018-05-23  5:20 ` [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver Bjorn Andersson
2018-05-23  6:05   ` Vinod
2018-05-23  6:58     ` Bjorn Andersson
2018-05-23  7:37       ` Vinod
2018-05-23 14:48         ` Sricharan R
2018-05-29  4:07           ` Bjorn Andersson
2018-05-29  8:32             ` Sricharan R
2018-06-01 13:33 ` [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Rohit Kumar

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=8c2acd62-0f3b-9167-b028-1f5149bd7937@codeaurora.org \
    --to=sricharan@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=ohad@wizery.com \
    --cc=rohitkr@codeaurora.org \
    --cc=sibis@codeaurora.org \
    --subject='Re: [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling' \
    /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).