[PATCH v2 0/3] Venus remoteproc driver

[PATCH v2 0/3] Venus remoteproc driver

[Stanimir Varbanov]

Hi,

here is v2 of the Venus remoteproc driver, the changes are:
	- removed page table size and page table init SCM calls.

regards,
Stan

Stanimir Varbanov (3):
  firmware: qcom: scm: add a video command for state setting
  dt-binding: remoteproc: venus rproc dt binding document
  remoteproc: qcom: add Venus video core firmware loader driver

 .../devicetree/bindings/remoteproc/qcom,venus.txt  |  33 ++++
 drivers/firmware/qcom_scm-32.c                     |  18 ++
 drivers/firmware/qcom_scm-64.c                     |  16 ++
 drivers/firmware/qcom_scm.c                        |  16 ++
 drivers/firmware/qcom_scm.h                        |   2 +
 drivers/remoteproc/Kconfig                         |  12 ++
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/qcom_venus_pil.c                | 213 +++++++++++++++++++++
 include/linux/qcom_scm.h                           |   2 +
 9 files changed, 313 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
 create mode 100644 drivers/remoteproc/qcom_venus_pil.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

[Stanimir Varbanov]

This scm call is used to change the video core state, more
specifically it is used to suspend and resume the core.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/firmware/qcom_scm-32.c | 18 ++++++++++++++++++
 drivers/firmware/qcom_scm-64.c | 16 ++++++++++++++++
 drivers/firmware/qcom_scm.c    | 16 ++++++++++++++++
 drivers/firmware/qcom_scm.h    |  2 ++
 include/linux/qcom_scm.h       |  2 ++
 5 files changed, 54 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index c6aeedbdcbb0..82c1d8d0d36b 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -560,3 +560,21 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)
 
 	return ret ? : le32_to_cpu(out);
 }
+
+int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare)
+{
+	struct {
+		__le32 state;
+		__le32 spare;
+	} req;
+	__le32 scm_ret = 0;
+	int ret;
+
+	req.state = cpu_to_le32(state);
+	req.spare = cpu_to_le32(spare);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_VIDEO_SET_STATE,
+			    &req, sizeof(req), &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 4a0f5ead4fb5..68484ea2aa51 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -358,3 +358,19 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)
 
 	return ret ? : res.a1;
 }
+
+int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare)
+{
+	struct qcom_scm_desc desc = {0};
+	struct arm_smccc_res res;
+	int ret;
+
+	desc.args[0] = state;
+	desc.args[1] = spare;
+	desc.arginfo = QCOM_SCM_ARGS(2);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_VIDEO_SET_STATE,
+			    &desc, &res);
+
+	return ret ? : res.a1;
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d95c70227c05..7e364691a87c 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
 }
 EXPORT_SYMBOL(qcom_scm_is_available);
 
+int qcom_scm_video_set_state(u32 state, u32 spare)
+{
+	int ret;
+
+	ret = qcom_scm_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_video_set_state(__scm->dev, state, spare);
+
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_video_set_state);
+
 static int qcom_scm_probe(struct platform_device *pdev)
 {
 	struct qcom_scm *scm;
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 3584b00fe7e6..4830559b2639 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -15,6 +15,8 @@
 #define QCOM_SCM_SVC_BOOT		0x1
 #define QCOM_SCM_BOOT_ADDR		0x1
 #define QCOM_SCM_BOOT_ADDR_MC		0x11
+#define QCOM_SCM_VIDEO_SET_STATE	0xa
+extern int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare);
 
 #define QCOM_SCM_FLAG_HLOS		0x01
 #define QCOM_SCM_FLAG_COLDBOOT_MC	0x02
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index cc32ab852fbc..2ece81a6b078 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -46,4 +46,6 @@ extern void qcom_scm_cpu_power_down(u32 flags);
 
 extern u32 qcom_scm_get_version(void);
 
+extern int qcom_scm_video_set_state(u32 state, u32 spare);
+
 #endif
-- 
2.7.4

Re: [PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

[Stephen Boyd]

On 11/07, Stanimir Varbanov wrote:
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index d95c70227c05..7e364691a87c 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
>  }
>  EXPORT_SYMBOL(qcom_scm_is_available);
>  
> +int qcom_scm_video_set_state(u32 state, u32 spare)
> +{
> +	int ret;
> +
> +	ret = qcom_scm_clk_enable();

Do we need clk control for this? Usually it's only required for
crypto engine things, and turning on video doesn't sound like it
uses crypto. I don't think downstream android kernel does this.

> +	if (ret)
> +		return ret;
> +
> +	ret = __qcom_scm_video_set_state(__scm->dev, state, spare);
> +
> +	qcom_scm_clk_disable();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_video_set_state);
> +
>  static int qcom_scm_probe(struct platform_device *pdev)
>  {
>  	struct qcom_scm *scm;

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

[Stanimir Varbanov]

Hi,

On 11/10/2016 03:52 AM, Stephen Boyd wrote:
> On 11/07, Stanimir Varbanov wrote:
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index d95c70227c05..7e364691a87c 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
>>  }
>>  EXPORT_SYMBOL(qcom_scm_is_available);
>>  
>> +int qcom_scm_video_set_state(u32 state, u32 spare)
>> +{
>> +	int ret;
>> +
>> +	ret = qcom_scm_clk_enable();
> 
> Do we need clk control for this? Usually it's only required for
> crypto engine things, and turning on video doesn't sound like it
> uses crypto. I don't think downstream android kernel does this.

Correct, the crypto clk is not needed.

-- 
regards,
Stan

[PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

[Stanimir Varbanov]

Add devicetree binding document for Venus remote processor.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 .../devicetree/bindings/remoteproc/qcom,venus.txt  | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
new file mode 100644
index 000000000000..06a2db60fa38
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
@@ -0,0 +1,33 @@
+Qualcomm Venus Peripheral Image Loader
+
+This document defines the binding for a component that loads and boots firmware
+on the Qualcomm Venus remote processor core.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must contain "qcom,venus-pil"
+
+- memory-region:
+	Usage: required
+	Value type: <phandle>
+	Definition: a phandle to a node describing reserved memory
+
+* An example
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		venus_mem: venus@89900000 {
+			compatible = "shared-dma-pool";
+			reg = <0x0 0x89900000 0x0 0x800000>;
+			alignment = <0x1000>;
+			no-map;
+		};
+	};
+
+	rproc_venus@0 {
+		compatible = "qcom,venus-pil";
+		memory-region = <&venus_mem>;
+	};
-- 
2.7.4

Re: [PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

[Stephen Boyd]

On 11/07, Stanimir Varbanov wrote:
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> new file mode 100644
> index 000000000000..06a2db60fa38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Venus Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm Venus remote processor core.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must contain "qcom,venus-pil"
> +
> +- memory-region:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: a phandle to a node describing reserved memory
> +
> +* An example
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		venus_mem: venus@89900000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x89900000 0x0 0x800000>;
> +			alignment = <0x1000>;
> +			no-map;
> +		};
> +	};
> +
> +	rproc_venus@0 {

Not sure we need @0 here in the example if it doesn't have a reg
property.

> +		compatible = "qcom,venus-pil";
> +		memory-region = <&venus_mem>;
> +	};
> -- 
> 2.7.4
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

[Rob Herring]

On Mon, Nov 07, 2016 at 07:30:52PM +0200, Stanimir Varbanov wrote:
> Add devicetree binding document for Venus remote processor.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  .../devicetree/bindings/remoteproc/qcom,venus.txt  | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> new file mode 100644
> index 000000000000..06a2db60fa38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Venus Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm Venus remote processor core.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must contain "qcom,venus-pil"
> +
> +- memory-region:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: a phandle to a node describing reserved memory
> +
> +* An example
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		venus_mem: venus@89900000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x89900000 0x0 0x800000>;
> +			alignment = <0x1000>;
> +			no-map;
> +		};
> +	};
> +
> +	rproc_venus@0 {

s/_/-/

With that,

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

[Stanimir Varbanov]

This driver will load and authenticate the Venus firmware and
bringing it core out of reset. Those two functionalities are
via secure monitor calls to trusted environment.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/remoteproc/Kconfig          |  12 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/qcom_venus_pil.c | 213 ++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)
 create mode 100644 drivers/remoteproc/qcom_venus_pil.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index f396bfef5d42..0e90a2491873 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -107,6 +107,18 @@ config QCOM_WCNSS_PIL
 	  Say y here to support the Peripheral Image Loader for the Qualcomm
 	  Wireless Connectivity Subsystem.
 
+config QCOM_VENUS_PIL
+	tristate "Qualcomm Venus Peripheral Image Loader"
+	depends on OF && ARCH_QCOM
+	depends on QCOM_SCM
+	select QCOM_MDT_LOADER
+	select REMOTEPROC
+	help
+	  Say y here to support Qualcomm Peripherial Image Loader for the
+	  Venus remote processor. The Venus remote processor is a
+	  micro-controller plus dedicated hardware for video acceleration
+	  of video decoding and encoding operations.
+
 config ST_REMOTEPROC
 	tristate "ST remoteproc support"
 	depends on ARCH_STI
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 6dfb62ed643f..852711b89844 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -15,4 +15,5 @@ obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
 obj-$(CONFIG_QCOM_WCNSS_IRIS)		+= qcom_wcnss_iris.o
 obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss.o
+obj-$(CONFIG_QCOM_VENUS_PIL)            += qcom_venus_pil.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
diff --git a/drivers/remoteproc/qcom_venus_pil.c b/drivers/remoteproc/qcom_venus_pil.c
new file mode 100644
index 000000000000..6d4e55bffef5
--- /dev/null
+++ b/drivers/remoteproc/qcom_venus_pil.c
@@ -0,0 +1,213 @@
+/*
+ * Qualcomm Venus Peripheral Image Loader
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * 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/dma-mapping.h>
+#include <linux/firmware.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/qcom_scm.h>
+#include <linux/remoteproc.h>
+
+#include "qcom_mdt_loader.h"
+#include "remoteproc_internal.h"
+
+#define VENUS_CRASH_REASON_SMEM		425
+#define VENUS_FIRMWARE_NAME		"venus.mdt"
+#define VENUS_PAS_ID			9
+#define VENUS_FW_MEM_SIZE		SZ_8M
+
+struct qcom_venus {
+	struct device *dev;
+	struct rproc *rproc;
+	phys_addr_t fw_addr;
+	phys_addr_t mem_phys;
+	void *mem_va;
+	size_t mem_size;
+};
+
+static int venus_load(struct rproc *rproc, const struct firmware *fw)
+{
+	struct qcom_venus *venus = rproc->priv;
+	phys_addr_t pa;
+	size_t fw_size;
+	bool relocate;
+	int ret;
+
+	ret = qcom_scm_pas_init_image(VENUS_PAS_ID, fw->data, fw->size);
+	if (ret) {
+		dev_err(&rproc->dev, "invalid firmware metadata (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	ret = qcom_mdt_parse(fw, &venus->fw_addr, &fw_size, &relocate);
+	if (ret) {
+		dev_err(&rproc->dev, "failed to parse mdt header (%d)\n", ret);
+		return ret;
+	}
+
+	if (fw_size > venus->mem_size)
+		return -ENOMEM;
+
+	pa = relocate ? venus->mem_phys : venus->fw_addr;
+
+	ret = qcom_scm_pas_mem_setup(VENUS_PAS_ID, pa, fw_size);
+	if (ret) {
+		dev_err(&rproc->dev, "unable to setup memory (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	return qcom_mdt_load(rproc, fw, VENUS_FIRMWARE_NAME);
+}
+
+static const struct rproc_fw_ops venus_fw_ops = {
+	.find_rsc_table = qcom_mdt_find_rsc_table,
+	.load = venus_load,
+};
+
+static int venus_start(struct rproc *rproc)
+{
+	struct qcom_venus *venus = rproc->priv;
+	int ret;
+
+	ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
+	if (ret)
+		dev_err(venus->dev,
+			"authentication image and release reset failed (%d)\n",
+			ret);
+
+	return ret;
+}
+
+static int venus_stop(struct rproc *rproc)
+{
+	struct qcom_venus *venus = rproc->priv;
+	int ret;
+
+	ret = qcom_scm_pas_shutdown(VENUS_PAS_ID);
+	if (ret)
+		dev_err(venus->dev, "failed to shutdown: %d\n", ret);
+
+	return ret;
+}
+
+static void *venus_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct qcom_venus *venus = rproc->priv;
+	s64 offset;
+
+	offset = da - venus->fw_addr;
+
+	if (offset < 0 || offset + len > venus->mem_size)
+		return NULL;
+
+	return venus->mem_va + offset;
+}
+
+static const struct rproc_ops venus_ops = {
+	.start = venus_start,
+	.stop = venus_stop,
+	.da_to_va = venus_da_to_va,
+};
+
+static int venus_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct qcom_venus *venus;
+	struct rproc *rproc;
+	int ret;
+
+	if (!qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
+	if (!qcom_scm_pas_supported(VENUS_PAS_ID)) {
+		dev_err(dev, "PAS is not available for venus\n");
+		return -ENXIO;
+	}
+
+	ret = of_reserved_mem_device_init(dev);
+	if (ret)
+		return ret;
+
+	rproc = rproc_alloc(dev, pdev->name, &venus_ops, VENUS_FIRMWARE_NAME,
+			    sizeof(*venus));
+	if (!rproc) {
+		dev_err(dev, "unable to allocate remoteproc\n");
+		ret = -ENOMEM;
+		goto release_mem;
+	}
+
+	rproc->fw_ops = &venus_fw_ops;
+	venus = rproc->priv;
+	venus->dev = dev;
+	venus->rproc = rproc;
+	venus->mem_size = VENUS_FW_MEM_SIZE;
+
+	platform_set_drvdata(pdev, venus);
+
+	venus->mem_va = dma_alloc_coherent(dev, venus->mem_size,
+					   &venus->mem_phys, GFP_KERNEL);
+	if (!venus->mem_va) {
+		ret = -ENOMEM;
+		goto free_rproc;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret)
+		goto free_mem;
+
+	return 0;
+
+free_mem:
+	dma_free_coherent(dev, venus->mem_size, venus->mem_va, venus->mem_phys);
+free_rproc:
+	rproc_put(rproc);
+release_mem:
+	of_reserved_mem_device_release(dev);
+
+	return ret;
+}
+
+static int venus_remove(struct platform_device *pdev)
+{
+	struct qcom_venus *venus = platform_get_drvdata(pdev);
+	struct device *dev = venus->dev;
+
+	rproc_del(venus->rproc);
+	rproc_put(venus->rproc);
+	dma_free_coherent(dev, venus->mem_size, venus->mem_va, venus->mem_phys);
+	of_reserved_mem_device_release(dev);
+
+	return 0;
+}
+
+static const struct of_device_id venus_of_match[] = {
+	{ .compatible = "qcom,venus-pil" },
+	{ },
+};
+
+static struct platform_driver venus_driver = {
+	.probe = venus_probe,
+	.remove = venus_remove,
+	.driver = {
+		.name = "qcom-venus-pil",
+		.of_match_table = venus_of_match,
+	},
+};
+
+module_platform_driver(venus_driver);
+MODULE_DESCRIPTION("Peripheral Image Loader for Venus");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

[Stephen Boyd]

On 11/07, Stanimir Varbanov wrote:
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/qcom_scm.h>
> +#include <linux/remoteproc.h>
> +
> +#include "qcom_mdt_loader.h"
> +#include "remoteproc_internal.h"
> +
> +#define VENUS_CRASH_REASON_SMEM		425

This is unused. Is there going to be some common smem API to get
the crash reason?
> +
> +static const struct of_device_id venus_of_match[] = {
> +	{ .compatible = "qcom,venus-pil" },
> +	{ },
> +};

Add a MODULE_DEVICE_TABLE?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

[Stanimir Varbanov]

Hi,

On 11/14/2016 09:16 PM, Stephen Boyd wrote:
> On 11/07, Stanimir Varbanov wrote:
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/qcom_scm.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "qcom_mdt_loader.h"
>> +#include "remoteproc_internal.h"
>> +
>> +#define VENUS_CRASH_REASON_SMEM		425
> 
> This is unused. Is there going to be some common smem API to get
> the crash reason?

This is leftover and never used, so I will delete it. About smem maybe
Bjorn have some idea?

>> +
>> +static const struct of_device_id venus_of_match[] = {
>> +	{ .compatible = "qcom,venus-pil" },
>> +	{ },
>> +};
> 
> Add a MODULE_DEVICE_TABLE?
> 

OK.

-- 
regards,
Stan

Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

[Bjorn Andersson]

On Mon 14 Nov 11:16 PST 2016, Stephen Boyd wrote:

> On 11/07, Stanimir Varbanov wrote:
> > +#include <linux/module.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/qcom_scm.h>
> > +#include <linux/remoteproc.h>
> > +
> > +#include "qcom_mdt_loader.h"
> > +#include "remoteproc_internal.h"
> > +
> > +#define VENUS_CRASH_REASON_SMEM		425
> 
> This is unused. Is there going to be some common smem API to get
> the crash reason?

There are a couple of these operations that are good candidates to be
shared between all Qualcomm remoteproc drivers. I just haven't spent the
time to figure out how to properly splice it.

For the crash reason specifically, there are three operations done in
the other drivers; read-and-null-terminate, then this is outputed to the
log, also indicating what type of event we had. And finally the
remoteproc core is informed about the crash, with a event-type passed
and we get another line in the log based on that, i.e. there's room for
a few improvements in this area.

Regards,
Bjorn