[PATCH v2 1/4] dt-binding: remoteproc: Introduce ADSP loader binding

[PATCH v2 1/4] dt-binding: remoteproc: Introduce ADSP loader binding

[Bjorn Andersson]

This document defines the binding for a component that loads firmware
and control the life cycle of the Qualcomm ADSP core.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Added platform names to compatibility

 .../devicetree/bindings/remoteproc/qcom,adsp.txt   | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
new file mode 100644
index 000000000000..3820151ce3e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
@@ -0,0 +1,70 @@
+Qualcomm ADSP Peripheral Image Loader
+
+This document defines the binding for a component that loads and boots firmware
+on the Qualcomm ADSP core.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,msm8974-adsp-pil"
+		    "qcom,msm8996-adsp-pil"
+
+- interrupts-extended:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: must list the watchdog, fatal IRQs ready, handover and
+		    stop-ack IRQs
+
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
+
+- cx-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: reference to the regulator to be held on behalf of the
+		    booting of the Hexagon core
+
+- memory-region:
+	Usage: required
+	Value type: <phandle>
+	Definition: reference to the reserved-memory for the ADSP
+
+- qcom,smem-states:
+	Usage: required
+	Value type: <phandle>
+	Definition: reference to the smem state for requesting the ADSP to
+		    shut down
+
+- qcom,smem-state-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must be "stop"
+
+= EXAMPLE
+The following example describes the resources needed to boot control the
+ADSP, as it is found on MSM8974 boards.
+
+	adsp {
+		compatible = "qcom,adsp-pil";
+
+		interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "wdog",
+				  "fatal",
+				  "ready",
+				  "handover",
+				  "stop-ack";
+
+		cx-supply = <&pm8841_s2>;
+
+		memory-region = <&adsp_region>;
+
+		qcom,smem-states = <&modem_smp2p_out 0>;
+		qcom,smem-state-names = "stop";
+	};
-- 
2.5.0

[PATCH v2 2/4] remoteproc: Introduce Qualcomm ADSP PIL

[Bjorn Andersson]

From: Bjorn Andersson <bjorn.andersson@sonymobile.com>

The Qualcomm ADSP Peripheral Image Loader is used on a variety of
different Qualcomm platforms for loading firmware into and controlling
the Hexagon based ADSP.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Added platform names to compatibility
- Removed some incorrect quirks that tried to handle older platforms

 drivers/remoteproc/Kconfig         |  11 +
 drivers/remoteproc/Makefile        |   1 +
 drivers/remoteproc/qcom_adsp_pil.c | 399 +++++++++++++++++++++++++++++++++++++
 3 files changed, 411 insertions(+)
 create mode 100644 drivers/remoteproc/qcom_adsp_pil.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 1a8bf76a925f..919741674686 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -77,6 +77,17 @@ config DA8XX_REMOTEPROC
 	  It's safe to say n here if you're not interested in multimedia
 	  offloading.
 
+config QCOM_ADSP_PIL
+	tristate "Qualcomm ADSP Peripheral Image Loader"
+	depends on OF && ARCH_QCOM
+	depends on QCOM_SMEM
+	select MFD_SYSCON
+	select QCOM_MDT_LOADER
+	select REMOTEPROC
+	help
+	  Say y here to support the TrustZone based Peripherial Image Loader
+	  for the Qualcomm ADSP remote processors.
+
 config QCOM_MDT_LOADER
 	tristate
 
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 92d3758bd15c..877a4c3b554d 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_STE_MODEM_RPROC)	 	+= ste_modem_rproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
+obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
new file mode 100644
index 000000000000..914163315091
--- /dev/null
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -0,0 +1,399 @@
+/*
+ * Qualcomm ADSP Peripheral Image Loader for MSM8974 and MSM8996
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ * Copyright (C) 2014 Sony Mobile Communications AB
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/firmware.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/qcom_scm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/remoteproc.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/smem_state.h>
+
+#include "qcom_mdt_loader.h"
+#include "remoteproc_internal.h"
+
+#define ADSP_CRASH_REASON_SMEM		423
+#define ADSP_FIRMWARE_NAME		"adsp.mdt"
+#define ADSP_PAS_ID			1
+
+struct qcom_adsp {
+	struct device *dev;
+	struct rproc *rproc;
+
+	int wdog_irq;
+	int fatal_irq;
+	int ready_irq;
+	int handover_irq;
+	int stop_ack_irq;
+
+	struct qcom_smem_state *state;
+	unsigned stop_bit;
+
+	struct regulator *cx_supply;
+
+	struct completion start_done;
+	struct completion stop_done;
+
+	phys_addr_t mem_phys;
+	phys_addr_t mem_reloc;
+	void *mem_region;
+	size_t mem_size;
+};
+
+static int adsp_load(struct rproc *rproc, const struct firmware *fw)
+{
+	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+	phys_addr_t fw_addr;
+	size_t fw_size;
+	bool relocate;
+	int ret;
+
+	ret = qcom_scm_pas_init_image(ADSP_PAS_ID, fw->data, fw->size);
+	if (ret) {
+		dev_err(&rproc->dev, "invalid firmware metadata\n");
+		return ret;
+	}
+
+	ret = qcom_mdt_parse(fw, &fw_addr, &fw_size, &relocate);
+	if (ret) {
+		dev_err(&rproc->dev, "failed to parse mdt header\n");
+		return ret;
+	}
+
+	if (relocate) {
+		adsp->mem_reloc = fw_addr;
+
+		ret = qcom_scm_pas_mem_setup(ADSP_PAS_ID, adsp->mem_phys, fw_size);
+		if (ret) {
+			dev_err(&rproc->dev, "unable to setup memory for image\n");
+			return ret;
+		}
+	}
+
+	return qcom_mdt_load(rproc, fw, rproc->firmware);
+}
+
+static const struct rproc_fw_ops adsp_fw_ops = {
+	.find_rsc_table = qcom_mdt_find_rsc_table,
+	.load = adsp_load,
+};
+
+static int adsp_start(struct rproc *rproc)
+{
+	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+	int ret;
+
+	ret = regulator_enable(adsp->cx_supply);
+	if (ret)
+		return ret;
+
+	ret = qcom_scm_pas_auth_and_reset(ADSP_PAS_ID);
+	if (ret) {
+		dev_err(adsp->dev,
+			"failed to authenticate image and release reset\n");
+		goto disable_regulators;
+	}
+
+	ret = wait_for_completion_timeout(&adsp->start_done,
+					  msecs_to_jiffies(5000));
+	if (!ret) {
+		dev_err(adsp->dev, "start timed out\n");
+		qcom_scm_pas_shutdown(ADSP_PAS_ID);
+		ret = -ETIMEDOUT;
+		goto disable_regulators;
+	}
+
+	ret = 0;
+
+disable_regulators:
+	regulator_disable(adsp->cx_supply);
+
+	return ret;
+}
+
+static int adsp_stop(struct rproc *rproc)
+{
+	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+	int ret;
+
+	qcom_smem_state_update_bits(adsp->state,
+				    BIT(adsp->stop_bit),
+				    BIT(adsp->stop_bit));
+
+	ret = wait_for_completion_timeout(&adsp->stop_done,
+					  msecs_to_jiffies(5000));
+	if (ret == 0)
+		dev_err(adsp->dev, "timed out on wait\n");
+
+	qcom_smem_state_update_bits(adsp->state,
+				    BIT(adsp->stop_bit),
+				    0);
+
+	ret = qcom_scm_pas_shutdown(ADSP_PAS_ID);
+	if (ret)
+		dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
+
+	return ret;
+}
+
+static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+	int offset;
+
+	offset = da - adsp->mem_reloc;
+	if (offset < 0 || offset + len > adsp->mem_size)
+		return NULL;
+
+	return adsp->mem_region + offset;
+}
+
+static const struct rproc_ops adsp_ops = {
+	.start = adsp_start,
+	.stop = adsp_stop,
+	.da_to_va = adsp_da_to_va,
+};
+
+static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
+{
+	struct qcom_adsp *adsp = dev;
+
+	rproc_report_crash(adsp->rproc, RPROC_WATCHDOG);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adsp_fatal_interrupt(int irq, void *dev)
+{
+	struct qcom_adsp *adsp = dev;
+	size_t len;
+	char *msg;
+
+	msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, ADSP_CRASH_REASON_SMEM, &len);
+	if (!IS_ERR(msg) && len > 0 && msg[0])
+		dev_err(adsp->dev, "fatal error received: %s\n", msg);
+
+	rproc_report_crash(adsp->rproc, RPROC_FATAL_ERROR);
+
+	if (!IS_ERR(msg))
+		msg[0] = '\0';
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adsp_ready_interrupt(int irq, void *dev)
+{
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adsp_handover_interrupt(int irq, void *dev)
+{
+	struct qcom_adsp *adsp = dev;
+
+	complete(&adsp->start_done);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
+{
+	struct qcom_adsp *adsp = dev;
+
+	complete(&adsp->stop_done);
+
+	return IRQ_HANDLED;
+}
+
+static int adsp_init_regulator(struct qcom_adsp *adsp)
+{
+	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");
+	if (IS_ERR(adsp->cx_supply))
+		return PTR_ERR(adsp->cx_supply);
+
+	regulator_set_load(adsp->cx_supply, 100000);
+
+	return 0;
+}
+
+static int adsp_request_irq(struct qcom_adsp *adsp,
+			     struct platform_device *pdev,
+			     const char *name,
+			     irq_handler_t thread_fn)
+{
+	int ret;
+
+	ret = platform_get_irq_byname(pdev, name);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "no %s IRQ defined\n", name);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, ret,
+					NULL, thread_fn,
+					IRQF_ONESHOT,
+					"adsp", adsp);
+	if (ret)
+		dev_err(&pdev->dev, "request %s IRQ failed\n", name);
+
+	return ret;
+}
+
+static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
+{
+	struct device_node *node;
+	struct resource r;
+	int ret;
+
+	node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
+	if (!node) {
+		dev_err(adsp->dev, "no memory-region specified\n");
+		return -EINVAL;
+	}
+
+	ret = of_address_to_resource(node, 0, &r);
+	if (ret)
+		return ret;
+
+	adsp->mem_phys = adsp->mem_reloc = r.start;
+	adsp->mem_size = resource_size(&r);
+	adsp->mem_region = devm_ioremap_wc(adsp->dev, adsp->mem_phys, adsp->mem_size);
+	if (!adsp->mem_region) {
+		dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
+			&r.start, adsp->mem_size);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static int adsp_probe(struct platform_device *pdev)
+{
+	struct qcom_adsp *adsp;
+	struct rproc *rproc;
+	int ret;
+
+	if (!qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
+	if (!qcom_scm_pas_supported(ADSP_PAS_ID)) {
+		dev_err(&pdev->dev, "PAS is not available for ADSP\n");
+		return -ENXIO;
+	}
+
+	rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
+			    ADSP_FIRMWARE_NAME, sizeof(*adsp));
+	if (!rproc) {
+		dev_err(&pdev->dev, "unable to allocate remoteproc\n");
+		return -ENOMEM;
+	}
+
+	rproc->fw_ops = &adsp_fw_ops;
+
+	adsp = (struct qcom_adsp *)rproc->priv;
+	adsp->dev = &pdev->dev;
+	adsp->rproc = rproc;
+	platform_set_drvdata(pdev, adsp);
+
+	init_completion(&adsp->start_done);
+	init_completion(&adsp->stop_done);
+
+	ret = adsp_alloc_memory_region(adsp);
+	if (ret)
+		goto free_rproc;
+
+	ret = adsp_init_regulator(adsp);
+	if (ret)
+		goto free_rproc;
+
+	ret = adsp_request_irq(adsp, pdev, "wdog", adsp_wdog_interrupt);
+	if (ret < 0)
+		goto free_rproc;
+	adsp->wdog_irq = ret;
+
+	ret = adsp_request_irq(adsp, pdev, "fatal", adsp_fatal_interrupt);
+	if (ret < 0)
+		goto free_rproc;
+	adsp->fatal_irq = ret;
+
+	ret = adsp_request_irq(adsp, pdev, "ready", adsp_ready_interrupt);
+	if (ret < 0)
+		goto free_rproc;
+	adsp->ready_irq = ret;
+
+	ret = adsp_request_irq(adsp, pdev, "handover", adsp_handover_interrupt);
+	if (ret < 0)
+		goto free_rproc;
+	adsp->handover_irq = ret;
+
+	ret = adsp_request_irq(adsp, pdev, "stop-ack", adsp_stop_ack_interrupt);
+	if (ret < 0)
+		goto free_rproc;
+	adsp->stop_ack_irq = ret;
+
+	adsp->state = qcom_smem_state_get(&pdev->dev, "stop",
+					  &adsp->stop_bit);
+	if (IS_ERR(adsp->state)) {
+		ret = PTR_ERR(adsp->state);
+		goto free_rproc;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret)
+		goto free_rproc;
+
+	return 0;
+
+free_rproc:
+	rproc_put(rproc);
+
+	return ret;
+}
+
+static int adsp_remove(struct platform_device *pdev)
+{
+	struct qcom_adsp *adsp = platform_get_drvdata(pdev);
+
+	qcom_smem_state_put(adsp->state);
+	rproc_del(adsp->rproc);
+	rproc_put(adsp->rproc);
+
+	return 0;
+}
+
+static const struct of_device_id adsp_of_match[] = {
+	{ .compatible = "qcom,msm8974-adsp-pil" },
+	{ .compatible = "qcom,msm8996-adsp-pil" },
+	{ },
+};
+
+static struct platform_driver adsp_driver = {
+	.probe = adsp_probe,
+	.remove = adsp_remove,
+	.driver = {
+		.name = "qcom_adsp_pil",
+		.of_match_table = adsp_of_match,
+	},
+};
+
+module_platform_driver(adsp_driver);
+MODULE_DESCRIPTION("Qualcomm MSM8974/MSM8996 ADSP Peripherial Image Loader");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

Re: [PATCH v2 2/4] remoteproc: Introduce Qualcomm ADSP PIL

[Stanimir Varbanov]

Hi Bjorn,

On 08/23/2016 08:57 AM, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> 
> The Qualcomm ADSP Peripheral Image Loader is used on a variety of
> different Qualcomm platforms for loading firmware into and controlling
> the Hexagon based ADSP.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v1:
> - Added platform names to compatibility
> - Removed some incorrect quirks that tried to handle older platforms
> 

<cut>

> +
> +static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
> +{
> +	struct device_node *node;
> +	struct resource r;
> +	int ret;
> +
> +	node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
> +	if (!node) {
> +		dev_err(adsp->dev, "no memory-region specified\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_address_to_resource(node, 0, &r);
> +	if (ret)
> +		return ret;
> +

I think that the parsing of memory-region and subsequent
address_to_resource is not so good. I had the same issue with Venus
remoteproc driver and come to the more standard way by using
of_reserved_mem_device_init() and dma_alloc_coherent.

Could you review "[PATCH 0/4] Venus remoteproc driver" patchset in this
regard?

> +	adsp->mem_phys = adsp->mem_reloc = r.start;
> +	adsp->mem_size = resource_size(&r);
> +	adsp->mem_region = devm_ioremap_wc(adsp->dev, adsp->mem_phys, adsp->mem_size);
> +	if (!adsp->mem_region) {
> +		dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
> +			&r.start, adsp->mem_size);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +

-- 
regards,
Stan

Re: [PATCH v2 2/4] remoteproc: Introduce Qualcomm ADSP PIL

[Bjorn Andersson]

On Tue 23 Aug 08:14 PDT 2016, Stanimir Varbanov wrote:

> Hi Bjorn,
> 
> On 08/23/2016 08:57 AM, Bjorn Andersson wrote:
> > From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > 
> > The Qualcomm ADSP Peripheral Image Loader is used on a variety of
> > different Qualcomm platforms for loading firmware into and controlling
> > the Hexagon based ADSP.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > Changes since v1:
> > - Added platform names to compatibility
> > - Removed some incorrect quirks that tried to handle older platforms
> > 
> 
> <cut>
> 
> > +
> > +static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
> > +{
> > +	struct device_node *node;
> > +	struct resource r;
> > +	int ret;
> > +
> > +	node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
> > +	if (!node) {
> > +		dev_err(adsp->dev, "no memory-region specified\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = of_address_to_resource(node, 0, &r);
> > +	if (ret)
> > +		return ret;
> > +
> 
> I think that the parsing of memory-region and subsequent
> address_to_resource is not so good. I had the same issue with Venus
> remoteproc driver and come to the more standard way by using
> of_reserved_mem_device_init() and dma_alloc_coherent.
> 

I tried this in an earlier version. But on 8974 the adsp is supposed to
be loaded at 0xdc00000, where we have a chunk that's 0x1900000 (25.6MB).

The problem is that dma_alloc_coherent(25.6MB) will fail, because there
is not 32MB free in that 25.6MB region. This stems from how
dma_alloc_coherent() is implemented, so some work would need to be done
there.


Further more, in remoteproc you are expected to provide a resource table
detailing carveout regions like this - a table that's supposed to come
from the ELF. We are currently looking into ways of injecting individual
resource requests from a remoteproc driver, so I hope to replace this
with a "rproc_add_carveout()".

But then this shows another issue with using dma_alloc_coherent(), it
suddenly requires us to create dummy devices for each memory region; to
be able to control which dma_mem should be used for each operation.


So, I intend to move most of this handling into the remoteproc core, but
I'm still uncertain if I we will not just have the same logic, but
shared in the core...

> Could you review "[PATCH 0/4] Venus remoteproc driver" patchset in this
> regard?
> 

I will, sorry for the delays.

> > +	adsp->mem_phys = adsp->mem_reloc = r.start;
> > +	adsp->mem_size = resource_size(&r);
> > +	adsp->mem_region = devm_ioremap_wc(adsp->dev, adsp->mem_phys, adsp->mem_size);
> > +	if (!adsp->mem_region) {
> > +		dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
> > +			&r.start, adsp->mem_size);
> > +		return -EBUSY;
> > +	}
> > +
> > +	return 0;
> > +}
> > +

Regards,
Bjorn

[PATCH v2 3/4] ARM: dts: msm8974: Add ADSP smp2p and smd nodes

[Bjorn Andersson]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Added this patch

 arch/arm/boot/dts/qcom-msm8974.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 561d4d136762..eac24cbe3c87 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -168,6 +168,31 @@
 		hwlocks = <&tcsr_mutex 3>;
 	};
 
+	smp2p-adsp {
+		compatible = "qcom,smp2p";
+		qcom,smem = <443>, <429>;
+
+		interrupt-parent = <&intc>;
+		interrupts = <0 158 IRQ_TYPE_EDGE_RISING>;
+
+		qcom,ipc = <&apcs 8 10>;
+
+		qcom,local-pid = <0>;
+		qcom,remote-pid = <2>;
+
+		adsp_smp2p_out: master-kernel {
+			qcom,entry-name = "master-kernel";
+			#qcom,smem-state-cells = <1>;
+		};
+
+		adsp_smp2p_in: slave-kernel {
+			qcom,entry-name = "slave-kernel";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
 	smp2p-modem {
 		compatible = "qcom,smp2p";
 		qcom,smem = <435>, <428>;
@@ -543,6 +568,13 @@
 	smd {
 		compatible = "qcom,smd";
 
+		adsp {
+			interrupts = <0 156 IRQ_TYPE_EDGE_RISING>;
+
+			qcom,ipc = <&apcs 8 8>;
+			qcom,smd-edge = <1>;
+		};
+
 		modem {
 			interrupts = <0 25 IRQ_TYPE_EDGE_RISING>;
 
-- 
2.5.0

[PATCH v2 4/4] ARM: dts: msm8974: Add ADSP PIL node

[Bjorn Andersson]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Added this patch

 arch/arm/boot/dts/qcom-msm8974.dtsi | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index eac24cbe3c87..4fa630cc7e4a 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -29,7 +29,7 @@
 			no-map;
 		};
 
-		adsp@0dc00000 {
+		adsp_region: adsp@0dc00000 {
 			reg = <0x0dc00000 0x1900000>;
 			no-map;
 		};
@@ -159,6 +159,24 @@
 		clock-frequency = <19200000>;
 	};
 
+	adsp-pil {
+		compatible = "qcom,msm8974-adsp-pil";
+
+		interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "wdog", "fatal", "ready", "handover", "stop-ack";
+
+		cx-supply = <&pm8841_s2>;
+
+		memory-region = <&adsp_region>;
+
+		qcom,smem-states = <&adsp_smp2p_out 0>;
+		qcom,smem-state-names = "stop";
+	};
+
 	smem {
 		compatible = "qcom,smem";
 
-- 
2.5.0

Re: [PATCH v2 1/4] dt-binding: remoteproc: Introduce ADSP loader binding

[Rob Herring]

On Mon, Aug 22, 2016 at 10:57:43PM -0700, Bjorn Andersson wrote:
> This document defines the binding for a component that loads firmware
> and control the life cycle of the Qualcomm ADSP core.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v1:
> - Added platform names to compatibility
> 
>  .../devicetree/bindings/remoteproc/qcom,adsp.txt   | 70 ++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> new file mode 100644
> index 000000000000..3820151ce3e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> @@ -0,0 +1,70 @@
> +Qualcomm ADSP Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm ADSP core.

ADSP is for Audio DSP? So there is another driver for the audio 
functions? Why doesn't it do the firmware loading? I'm still confused 
why this binding is separate? If you had one common interface (a rproc) 
to load and boot various other blocks like ADSP and Venus, then this 
would make sense. Or does every accel block have some separate control 
uC associated with it?

> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,msm8974-adsp-pil"
> +		    "qcom,msm8996-adsp-pil"
> +
> +- interrupts-extended:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: must list the watchdog, fatal IRQs ready, handover and
> +		    stop-ack IRQs
> +
> +- interrupt-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +
> +- cx-supply:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: reference to the regulator to be held on behalf of the
> +		    booting of the Hexagon core
> +
> +- memory-region:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: reference to the reserved-memory for the ADSP
> +
> +- qcom,smem-states:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: reference to the smem state for requesting the ADSP to
> +		    shut down
> +
> +- qcom,smem-state-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be "stop"
> +
> += EXAMPLE
> +The following example describes the resources needed to boot control the
> +ADSP, as it is found on MSM8974 boards.
> +
> +	adsp {
> +		compatible = "qcom,adsp-pil";
> +
> +		interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>,
> +				      <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> +				      <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> +				      <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> +				      <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> +		interrupt-names = "wdog",
> +				  "fatal",
> +				  "ready",
> +				  "handover",
> +				  "stop-ack";
> +
> +		cx-supply = <&pm8841_s2>;
> +
> +		memory-region = <&adsp_region>;
> +
> +		qcom,smem-states = <&modem_smp2p_out 0>;
> +		qcom,smem-state-names = "stop";
> +	};
> -- 
> 2.5.0
> 

Re: [PATCH v2 1/4] dt-binding: remoteproc: Introduce ADSP loader binding

[Bjorn Andersson]

On Tue 23 Aug 11:31 PDT 2016, Rob Herring wrote:

> On Mon, Aug 22, 2016 at 10:57:43PM -0700, Bjorn Andersson wrote:
> > This document defines the binding for a component that loads firmware
> > and control the life cycle of the Qualcomm ADSP core.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > Changes since v1:
> > - Added platform names to compatibility
> > 
> >  .../devicetree/bindings/remoteproc/qcom,adsp.txt   | 70 ++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > new file mode 100644
> > index 000000000000..3820151ce3e9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > @@ -0,0 +1,70 @@
> > +Qualcomm ADSP Peripheral Image Loader
> > +
> > +This document defines the binding for a component that loads and boots firmware
> > +on the Qualcomm ADSP core.
> 
> ADSP is for Audio DSP? So there is another driver for the audio 
> functions? Why doesn't it do the firmware loading? I'm still confused 
> why this binding is separate? If you had one common interface (a rproc) 
> to load and boot various other blocks like ADSP and Venus, then this 
> would make sense. Or does every accel block have some separate control 
> uC associated with it?
> 

The ADSP is a general purpose CPU [1] mainly running services related to
audio handling - including controlling audio paths, driving the audio
blocks, audio effects, audio codec decoding.

On some platforms it also sports services for sensor batch offloading
(or whatever Google calls it) and video decoding for certain codecs.

All these services show up in a semi-probable fashion on other buses;
often on SMD, APR, QRTR.


There are a few blocks that share mechanism with the remoteproc, that
does not have a separate uC - with a destinct life cycle - I'm still
investigating how to describe these, but most likely those cases will
not show up in DT at all.

On msm8916 you have the following additional uCs; RPM, Hexagon, Wireless
and Venus; the RPM is always-on.

On msm8960 we have the following uCs; RPM, Hexagon for audio, DSPS (ARM
for sensor processing), two(?) Hexagons for modem, WCNSS (ARM core for
wireless), Venus (seems to be another ARM core) and an optional ARM core
for GPS if you don't have the modem Hexagons.

So, we have between 4 and 8 extra uCs in these SoCs; most are controlled
in a very similar fashion, but requires different resources and some
tweaks to the steps of bringing them up, down and handling crashes.

Downstream this is handled by having a "rproc" driver that's completely
generic, DT provides lists of resources controlling each step and a
callback mechanism is used to extend the rproc drivers with specific
functionality - it took me months to figure out how to boot the WCNSS
because the logic and resources are scattered throughout.

[1] https://en.wikipedia.org/wiki/Qualcomm_Hexagon

Regards,
Bjorn

Re: [PATCH v2 1/4] dt-binding: remoteproc: Introduce ADSP loader binding

[Bjorn Andersson]

On Tue 23 Aug 11:57 PDT 2016, Bjorn Andersson wrote:

> On Tue 23 Aug 11:31 PDT 2016, Rob Herring wrote:
> 
> > On Mon, Aug 22, 2016 at 10:57:43PM -0700, Bjorn Andersson wrote:
> > > This document defines the binding for a component that loads firmware
> > > and control the life cycle of the Qualcomm ADSP core.
> > > 
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > > 
> > > Changes since v1:
> > > - Added platform names to compatibility
> > > 
> > >  .../devicetree/bindings/remoteproc/qcom,adsp.txt   | 70 ++++++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > > new file mode 100644
> > > index 000000000000..3820151ce3e9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > > @@ -0,0 +1,70 @@
> > > +Qualcomm ADSP Peripheral Image Loader
> > > +
> > > +This document defines the binding for a component that loads and boots firmware
> > > +on the Qualcomm ADSP core.
> > 
> > ADSP is for Audio DSP? So there is another driver for the audio 
> > functions? Why doesn't it do the firmware loading? I'm still confused 
> > why this binding is separate? If you had one common interface (a rproc) 
> > to load and boot various other blocks like ADSP and Venus, then this 
> > would make sense.

> > Or does every accel block have some separate control 
> > uC associated with it?

Sorry for the lengthy explanation below, in case you rather want a
TL;DR:

This is not an accel block, it's a general purpose CPU exposing among
other thing audio related services.

> > 
> 
> The ADSP is a general purpose CPU [1] mainly running services related to
> audio handling - including controlling audio paths, driving the audio
> blocks, audio effects, audio codec decoding.
> 
> On some platforms it also sports services for sensor batch offloading
> (or whatever Google calls it) and video decoding for certain codecs.
> 
> All these services show up in a semi-probable fashion on other buses;
> often on SMD, APR, QRTR.
> 
> 
> There are a few blocks that share mechanism with the remoteproc, that
> does not have a separate uC - with a destinct life cycle - I'm still
> investigating how to describe these, but most likely those cases will
> not show up in DT at all.
> 
> On msm8916 you have the following additional uCs; RPM, Hexagon, Wireless
> and Venus; the RPM is always-on.
> 
> On msm8960 we have the following uCs; RPM, Hexagon for audio, DSPS (ARM
> for sensor processing), two(?) Hexagons for modem, WCNSS (ARM core for
> wireless), Venus (seems to be another ARM core) and an optional ARM core
> for GPS if you don't have the modem Hexagons.
> 
> So, we have between 4 and 8 extra uCs in these SoCs; most are controlled
> in a very similar fashion, but requires different resources and some
> tweaks to the steps of bringing them up, down and handling crashes.
> 
> Downstream this is handled by having a "rproc" driver that's completely
> generic, DT provides lists of resources controlling each step and a
> callback mechanism is used to extend the rproc drivers with specific
> functionality - it took me months to figure out how to boot the WCNSS
> because the logic and resources are scattered throughout.
> 
> [1] https://en.wikipedia.org/wiki/Qualcomm_Hexagon
> 

Rob, did this answer your questions, do you find this acceptable or do
you have any suggestion to how I should model this?

Regards,
Bjorn