Re: fs/proc: Crash observed in next_tgid (fs/proc/base.c)

2019-04-17 Thread Jitendra Sharma

Thanks Marc and Kees for replying.
Answer to your queries:
Kernel version: 4.14.83
Test case: Not some specific test case. Issue reproduced while doing 
monkey testing for very long hours.


Thanks,
Jitendra

On 4/17/2019 5:41 PM, Marc Gonzalez wrote:

On 15/04/2019 14:58, Jitendra Sharma wrote:


We are observing one kernel crash in next_tgid function through
getdents64 path. Call stack is as shown below:


It might help if you specify the exact kernel version you are discussing,
as in which tag or commit hash you are running.

Also, what are you doing to trigger the issue?

Regards.



--

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


fs/proc: Crash observed in next_tgid (fs/proc/base.c)

2019-04-15 Thread Jitendra Sharma

Hi Kees Cook/Luis,

We are observing one kernel crash in next_tgid function through 
getdents64 path. Call stack is as shown below:


-000|has_group_leader_pid(inline)
-000|next_tgid(
| [X20] ns = 0xFF87CABB1AC0,
| [locdesc] iter = (
| [locdesc] tgid = 424,
| [locdesc] task = ?))
| [X21] p = 0xFFD0F948
| [X21] task = 0xFFD0F948
-001|proc_pid_readdir(
| [X20] file = 0xFFD1AC60FC40,
| [X19] ctx = 0xFF8027363E40)
| [X21] ns = 0xFF87CABB1AC0
-002|proc_root_readdir(
| [X20] file = 0xFFD1AC60FC40,
| [X19] ctx = 0xFF8027363E40)
-003|iterate_dir(
| [X19] file = 0xFFD1AC60FC40,
| [X22] ctx = 0xFF8027363E40)
| [X23] inode = 0xFFD1F20246D0
-004|SYSC_getdents64(inline)
-004|sys_getdents64(
| ?,
| ?,
| [X19] count = 4200)
| [X19] count = 4200
| [X20] f = ([X20] file = 0xAC60FC43AC60FC40, [X20] flags = 1207898624)
| [X0] error = -1720
-005|el0_svc_naked(asm)
-->|exception
-006|NUX:0x78C5AD7D38(asm)
---|end of frame


From this call stack,task: 0xFFD0F948, seems to be invalid. 
As(from ramdumps) it doesn't have any valid fields. And while trying to 
access the fields of this task struct in has_group_leader_pid, abort is 
happening.


From the dumps, its not clear why the task struct is coming to be some 
invalid (Possibly task has already exited).  This issue is observed 
during normal monkey testing for long hours.


Could you please provide some pointers which could help in debugging 
this issue further.



Thanks,

Jitendra

--

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


Re: [PATCH v5 4/5] remoteproc: qcom: Introduce sysmon

2018-03-02 Thread Jitendra Sharma

Hi Bjorn,


On 12/5/2017 11:13 PM, Bjorn Andersson wrote:

The sysmon client communicates either via a dedicated SMD/GLINK channel
or via QMI encoded messages over IPCROUTER with remote processors in
order to perform graceful shutdown and inform about other remote
processors shutting down.

Acked-by: Chris Lew 
Signed-off-by: Bjorn Andersson 
---

Changes since v4:
- SPDX header

Changes since v3:
- Kerneldoc updates
- Style fixes

Changes since v2:
- Checkpatch fixes

Changes since v1:
- New patch

  drivers/remoteproc/Kconfig |  17 ++
  drivers/remoteproc/Makefile|   1 +
  drivers/remoteproc/qcom_adsp_pil.c |  12 +
  drivers/remoteproc/qcom_common.h   |  21 ++
  drivers/remoteproc/qcom_q6v5_pil.c |   3 +
  drivers/remoteproc/qcom_sysmon.c   | 579 +
  drivers/remoteproc/qcom_wcnss.c|   4 +
  7 files changed, 637 insertions(+)
  create mode 100644 drivers/remoteproc/qcom_sysmon.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index b609e1d3654b..6556df21318e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -90,6 +90,7 @@ config QCOM_ADSP_PIL
depends on QCOM_SMEM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
select QCOM_MDT_LOADER
select QCOM_RPROC_COMMON
@@ -107,6 +108,7 @@ config QCOM_Q6V5_PIL
depends on QCOM_SMEM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
select QCOM_RPROC_COMMON
select QCOM_SCM
@@ -114,12 +116,27 @@ config QCOM_Q6V5_PIL
  Say y here to support the Qualcomm Peripherial Image Loader for the
  Hexagon V5 based remote processors.
  
+config QCOM_SYSMON

+   tristate "Qualcomm sysmon driver"
+   depends on RPMSG
+   depends on ARCH_QCOM
+   select QCOM_QMI_HELPERS
+   help
+ The sysmon driver implements a sysmon QMI client and a handler for
+ the sys_mon SMD and GLINK channel, which are used for graceful
+ shutdown, retrieving failure information and propagating information
+ about other subsystems being shut down.
+
+ Say y here if your system runs firmware on any other subsystems, e.g.
+ modem or DSP.
+
  config QCOM_WCNSS_PIL
tristate "Qualcomm WCNSS Peripheral Image Loader"
depends on OF && ARCH_QCOM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
depends on QCOM_SMEM
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select QCOM_MDT_LOADER
select QCOM_RPROC_COMMON
select QCOM_SCM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 6e16450ce11f..02627ede8d4a 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -17,6 +17,7 @@ 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_PIL)   += qcom_q6v5_pil.o
+obj-$(CONFIG_QCOM_SYSMON)  += qcom_sysmon.o
  obj-$(CONFIG_QCOM_WCNSS_PIL)  += qcom_wcnss_pil.o
  qcom_wcnss_pil-y  += qcom_wcnss.o
  qcom_wcnss_pil-y  += qcom_wcnss_iris.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c 
b/drivers/remoteproc/qcom_adsp_pil.c
index 3f6af54dbc96..45e7e66604d4 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -38,7 +38,10 @@ struct adsp_data {
const char *firmware_name;
int pas_id;
bool has_aggre2_clk;
+
const char *ssr_name;
+   const char *sysmon_name;
+   int ssctl_id;
  };
  
  struct qcom_adsp {

@@ -75,6 +78,7 @@ struct qcom_adsp {
struct qcom_rproc_glink glink_subdev;
struct qcom_rproc_subdev smd_subdev;
struct qcom_rproc_ssr ssr_subdev;
+   struct qcom_sysmon *sysmon;
  };
  
  static int adsp_load(struct rproc *rproc, const struct firmware *fw)

@@ -404,6 +408,9 @@ static int adsp_probe(struct platform_device *pdev)
qcom_add_glink_subdev(rproc, >glink_subdev);
qcom_add_smd_subdev(rproc, >smd_subdev);
qcom_add_ssr_subdev(rproc, >ssr_subdev, desc->ssr_name);
+   adsp->sysmon = qcom_add_sysmon_subdev(rproc,
+ desc->sysmon_name,
+ desc->ssctl_id);
  
  	ret = rproc_add(rproc);

if (ret)
@@ -425,6 +432,7 @@ static int adsp_remove(struct platform_device *pdev)
rproc_del(adsp->rproc);
  
  	

Re: [PATCH v5 4/5] remoteproc: qcom: Introduce sysmon

2018-03-02 Thread Jitendra Sharma

Hi Bjorn,


On 12/5/2017 11:13 PM, Bjorn Andersson wrote:

The sysmon client communicates either via a dedicated SMD/GLINK channel
or via QMI encoded messages over IPCROUTER with remote processors in
order to perform graceful shutdown and inform about other remote
processors shutting down.

Acked-by: Chris Lew 
Signed-off-by: Bjorn Andersson 
---

Changes since v4:
- SPDX header

Changes since v3:
- Kerneldoc updates
- Style fixes

Changes since v2:
- Checkpatch fixes

Changes since v1:
- New patch

  drivers/remoteproc/Kconfig |  17 ++
  drivers/remoteproc/Makefile|   1 +
  drivers/remoteproc/qcom_adsp_pil.c |  12 +
  drivers/remoteproc/qcom_common.h   |  21 ++
  drivers/remoteproc/qcom_q6v5_pil.c |   3 +
  drivers/remoteproc/qcom_sysmon.c   | 579 +
  drivers/remoteproc/qcom_wcnss.c|   4 +
  7 files changed, 637 insertions(+)
  create mode 100644 drivers/remoteproc/qcom_sysmon.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index b609e1d3654b..6556df21318e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -90,6 +90,7 @@ config QCOM_ADSP_PIL
depends on QCOM_SMEM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
select QCOM_MDT_LOADER
select QCOM_RPROC_COMMON
@@ -107,6 +108,7 @@ config QCOM_Q6V5_PIL
depends on QCOM_SMEM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
select QCOM_RPROC_COMMON
select QCOM_SCM
@@ -114,12 +116,27 @@ config QCOM_Q6V5_PIL
  Say y here to support the Qualcomm Peripherial Image Loader for the
  Hexagon V5 based remote processors.
  
+config QCOM_SYSMON

+   tristate "Qualcomm sysmon driver"
+   depends on RPMSG
+   depends on ARCH_QCOM
+   select QCOM_QMI_HELPERS
+   help
+ The sysmon driver implements a sysmon QMI client and a handler for
+ the sys_mon SMD and GLINK channel, which are used for graceful
+ shutdown, retrieving failure information and propagating information
+ about other subsystems being shut down.
+
+ Say y here if your system runs firmware on any other subsystems, e.g.
+ modem or DSP.
+
  config QCOM_WCNSS_PIL
tristate "Qualcomm WCNSS Peripheral Image Loader"
depends on OF && ARCH_QCOM
depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
depends on QCOM_SMEM
+   depends on QCOM_SYSMON || QCOM_SYSMON=n
select QCOM_MDT_LOADER
select QCOM_RPROC_COMMON
select QCOM_SCM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 6e16450ce11f..02627ede8d4a 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -17,6 +17,7 @@ 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_PIL)   += qcom_q6v5_pil.o
+obj-$(CONFIG_QCOM_SYSMON)  += qcom_sysmon.o
  obj-$(CONFIG_QCOM_WCNSS_PIL)  += qcom_wcnss_pil.o
  qcom_wcnss_pil-y  += qcom_wcnss.o
  qcom_wcnss_pil-y  += qcom_wcnss_iris.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c 
b/drivers/remoteproc/qcom_adsp_pil.c
index 3f6af54dbc96..45e7e66604d4 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -38,7 +38,10 @@ struct adsp_data {
const char *firmware_name;
int pas_id;
bool has_aggre2_clk;
+
const char *ssr_name;
+   const char *sysmon_name;
+   int ssctl_id;
  };
  
  struct qcom_adsp {

@@ -75,6 +78,7 @@ struct qcom_adsp {
struct qcom_rproc_glink glink_subdev;
struct qcom_rproc_subdev smd_subdev;
struct qcom_rproc_ssr ssr_subdev;
+   struct qcom_sysmon *sysmon;
  };
  
  static int adsp_load(struct rproc *rproc, const struct firmware *fw)

@@ -404,6 +408,9 @@ static int adsp_probe(struct platform_device *pdev)
qcom_add_glink_subdev(rproc, >glink_subdev);
qcom_add_smd_subdev(rproc, >smd_subdev);
qcom_add_ssr_subdev(rproc, >ssr_subdev, desc->ssr_name);
+   adsp->sysmon = qcom_add_sysmon_subdev(rproc,
+ desc->sysmon_name,
+ desc->ssctl_id);
  
  	ret = rproc_add(rproc);

if (ret)
@@ -425,6 +432,7 @@ static int adsp_remove(struct platform_device *pdev)
rproc_del(adsp->rproc);
  
  	qcom_remove_glink_subdev(adsp->rproc, >glink_subdev);


Re: [PATCH 1/2] soc: qcom: rmtfs-mem: Add support for assigning memory to remote

2018-02-21 Thread Jitendra Sharma

Hi Bjorn,


On 2/13/2018 7:07 AM, Bjorn Andersson wrote:

On some platform the remote processor's memory map is not statically
configured in TrustZone, so each memory region that is to be accessed by
the remote needs a call into TrustZone to set up the remote's
permissions.

Implement this for the rmtfs memory driver, to give the modem on 8996
access to the shared file system buffers.

Signed-off-by: Bjorn Andersson 
---
  drivers/soc/qcom/Kconfig |  1 +
  drivers/soc/qcom/rmtfs_mem.c | 34 ++
  2 files changed, 35 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb83341d..a993d19fa562 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -47,6 +47,7 @@ config QCOM_QMI_HELPERS
  config QCOM_RMTFS_MEM
tristate "Qualcomm Remote Filesystem memory driver"
depends on ARCH_QCOM
+   select QCOM_SCM
help
  The Qualcomm remote filesystem memory driver is used for allocating
  and exposing regions of shared memory with remote processors for the
diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
index 0a43b2e8906f..c8999e38b005 100644
--- a/drivers/soc/qcom/rmtfs_mem.c
+++ b/drivers/soc/qcom/rmtfs_mem.c
@@ -37,6 +37,8 @@ struct qcom_rmtfs_mem {
phys_addr_t size;
  
  	unsigned int client_id;

+
+   unsigned int perms;
  };
  
  static ssize_t qcom_rmtfs_mem_show(struct device *dev,

@@ -151,9 +153,11 @@ static void qcom_rmtfs_mem_release_device(struct device 
*dev)
  static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
  {
struct device_node *node = pdev->dev.of_node;
+   struct qcom_scm_vmperm perms[2];
struct reserved_mem *rmem;
struct qcom_rmtfs_mem *rmtfs_mem;
u32 client_id;
+   u32 vmid;
int ret;
  
  	rmem = of_reserved_mem_lookup(node);

@@ -204,10 +208,31 @@ static int qcom_rmtfs_mem_probe(struct platform_device 
*pdev)
  
  	rmtfs_mem->dev.release = qcom_rmtfs_mem_release_device;
  
+	ret = of_property_read_u32(node, "qcom,vmid", );

+   if (ret < 0 && ret != -EINVAL) {
+   dev_err(>dev, "failed to parse qcom,vmid\n");
+   goto remove_cdev;
+   } else if (!ret) {
+   perms[0].vmid = QCOM_SCM_VMID_HLOS;
+   perms[0].perm = QCOM_SCM_PERM_RW;
+   perms[1].vmid = vmid;
+   perms[1].perm = QCOM_SCM_PERM_RW;
+
+   rmtfs_mem->perms = BIT(QCOM_SCM_VMID_HLOS);
+   ret = qcom_scm_assign_mem(rmtfs_mem->addr, rmtfs_mem->size,
+ _mem->perms, perms, 2);
+   if (ret < 0) {
+   dev_err(>dev, "assign memory failed\n");
+   goto remove_cdev;
+   }
+   }
+

I have a query here,
We assigned memory ownership to modem at boot up. In case of errors in 
other parts of driver, where we are returning, have we assigned memory 
back to HLOS

dev_set_drvdata(>dev, rmtfs_mem);
  
  	return 0;
  
+remove_cdev:

+   cdev_device_del(_mem->cdev, _mem->dev);
  put_device:
put_device(_mem->dev);
  
@@ -217,6 +242,15 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)

  static int qcom_rmtfs_mem_remove(struct platform_device *pdev)
  {
struct qcom_rmtfs_mem *rmtfs_mem = dev_get_drvdata(>dev);
+   struct qcom_scm_vmperm perm;
+
+   if (rmtfs_mem->perms) {
+   perm.vmid = QCOM_SCM_VMID_HLOS;
+   perm.perm = QCOM_SCM_PERM_RW;
+
+   qcom_scm_assign_mem(rmtfs_mem->addr, rmtfs_mem->size,
+   _mem->perms, , 1);
+   }
  
  	cdev_device_del(_mem->cdev, _mem->dev);

put_device(_mem->dev);




Re: [PATCH 1/2] soc: qcom: rmtfs-mem: Add support for assigning memory to remote

2018-02-21 Thread Jitendra Sharma

Hi Bjorn,


On 2/13/2018 7:07 AM, Bjorn Andersson wrote:

On some platform the remote processor's memory map is not statically
configured in TrustZone, so each memory region that is to be accessed by
the remote needs a call into TrustZone to set up the remote's
permissions.

Implement this for the rmtfs memory driver, to give the modem on 8996
access to the shared file system buffers.

Signed-off-by: Bjorn Andersson 
---
  drivers/soc/qcom/Kconfig |  1 +
  drivers/soc/qcom/rmtfs_mem.c | 34 ++
  2 files changed, 35 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb83341d..a993d19fa562 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -47,6 +47,7 @@ config QCOM_QMI_HELPERS
  config QCOM_RMTFS_MEM
tristate "Qualcomm Remote Filesystem memory driver"
depends on ARCH_QCOM
+   select QCOM_SCM
help
  The Qualcomm remote filesystem memory driver is used for allocating
  and exposing regions of shared memory with remote processors for the
diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
index 0a43b2e8906f..c8999e38b005 100644
--- a/drivers/soc/qcom/rmtfs_mem.c
+++ b/drivers/soc/qcom/rmtfs_mem.c
@@ -37,6 +37,8 @@ struct qcom_rmtfs_mem {
phys_addr_t size;
  
  	unsigned int client_id;

+
+   unsigned int perms;
  };
  
  static ssize_t qcom_rmtfs_mem_show(struct device *dev,

@@ -151,9 +153,11 @@ static void qcom_rmtfs_mem_release_device(struct device 
*dev)
  static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
  {
struct device_node *node = pdev->dev.of_node;
+   struct qcom_scm_vmperm perms[2];
struct reserved_mem *rmem;
struct qcom_rmtfs_mem *rmtfs_mem;
u32 client_id;
+   u32 vmid;
int ret;
  
  	rmem = of_reserved_mem_lookup(node);

@@ -204,10 +208,31 @@ static int qcom_rmtfs_mem_probe(struct platform_device 
*pdev)
  
  	rmtfs_mem->dev.release = qcom_rmtfs_mem_release_device;
  
+	ret = of_property_read_u32(node, "qcom,vmid", );

+   if (ret < 0 && ret != -EINVAL) {
+   dev_err(>dev, "failed to parse qcom,vmid\n");
+   goto remove_cdev;
+   } else if (!ret) {
+   perms[0].vmid = QCOM_SCM_VMID_HLOS;
+   perms[0].perm = QCOM_SCM_PERM_RW;
+   perms[1].vmid = vmid;
+   perms[1].perm = QCOM_SCM_PERM_RW;
+
+   rmtfs_mem->perms = BIT(QCOM_SCM_VMID_HLOS);
+   ret = qcom_scm_assign_mem(rmtfs_mem->addr, rmtfs_mem->size,
+ _mem->perms, perms, 2);
+   if (ret < 0) {
+   dev_err(>dev, "assign memory failed\n");
+   goto remove_cdev;
+   }
+   }
+

I have a query here,
We assigned memory ownership to modem at boot up. In case of errors in 
other parts of driver, where we are returning, have we assigned memory 
back to HLOS

dev_set_drvdata(>dev, rmtfs_mem);
  
  	return 0;
  
+remove_cdev:

+   cdev_device_del(_mem->cdev, _mem->dev);
  put_device:
put_device(_mem->dev);
  
@@ -217,6 +242,15 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)

  static int qcom_rmtfs_mem_remove(struct platform_device *pdev)
  {
struct qcom_rmtfs_mem *rmtfs_mem = dev_get_drvdata(>dev);
+   struct qcom_scm_vmperm perm;
+
+   if (rmtfs_mem->perms) {
+   perm.vmid = QCOM_SCM_VMID_HLOS;
+   perm.perm = QCOM_SCM_PERM_RW;
+
+   qcom_scm_assign_mem(rmtfs_mem->addr, rmtfs_mem->size,
+   _mem->perms, , 1);
+   }
  
  	cdev_device_del(_mem->cdev, _mem->dev);

put_device(_mem->dev);




Re: [PATCH] rpmsg: qcom_smd: Access APCS through mailbox framework

2017-12-06 Thread Jitendra Sharma

Hi Bjorn,


On 11/16/2017 12:38 PM, Bjorn Andersson wrote:

Attempt to acquire the APCS IPC through the mailbox framework and fall
back to the old syscon based approach, to allow us to move away from
using the syscon.

Signed-off-by: Bjorn Andersson 
---
  drivers/rpmsg/qcom_smd.c | 62 +---
  1 file changed, 43 insertions(+), 19 deletions(-)

diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index b01774e9fac0..ef2a526ebc8f 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -14,6 +14,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -107,6 +108,8 @@ static const struct {
   * @ipc_regmap:   regmap handle holding the outgoing ipc register
   * @ipc_offset:   offset within @ipc_regmap of the register for 
ipc
   * @ipc_bit:  bit in the register at @ipc_offset of @ipc_regmap
+ * @mbox_client:   mailbox client handle
+ * @mbox_chan: apcs ipc mailbox channel handle
   * @channels: list of all channels detected on this edge
   * @channels_lock:guard for modifications of @channels
   * @allocated:array of bitmaps representing already allocated 
channels
@@ -129,6 +132,9 @@ struct qcom_smd_edge {
int ipc_offset;
int ipc_bit;
  
+	struct mbox_client mbox_client;

+   struct mbox_chan *mbox_chan;
+
struct list_head channels;
spinlock_t channels_lock;
  
@@ -365,7 +371,12 @@ static void qcom_smd_signal_channel(struct qcom_smd_channel *channel)

  {
struct qcom_smd_edge *edge = channel->edge;
  
-	regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));

+   if (edge->mbox_chan) {
+   mbox_send_message(edge->mbox_chan, NULL);

mbox_send_message could fail. So return value should be checked

+   mbox_client_txdone(edge->mbox_chan, 0);
+   } else {
+   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
BIT(edge->ipc_bit));
+   }
  }
  
  /*

@@ -1268,27 +1279,37 @@ static int qcom_smd_parse_edge(struct device *dev,
key = "qcom,remote-pid";
of_property_read_u32(node, key, >remote_pid);
  
-	syscon_np = of_parse_phandle(node, "qcom,ipc", 0);

-   if (!syscon_np) {
-   dev_err(dev, "no qcom,ipc node\n");
-   return -ENODEV;
-   }
+   edge->mbox_client.dev = dev;
+   edge->mbox_client.knows_txdone = true;
+   edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
+   if (IS_ERR(edge->mbox_chan)) {
+   if (PTR_ERR(edge->mbox_chan) != -ENODEV)
+   return PTR_ERR(edge->mbox_chan);
  
-	edge->ipc_regmap = syscon_node_to_regmap(syscon_np);

-   if (IS_ERR(edge->ipc_regmap))
-   return PTR_ERR(edge->ipc_regmap);
+   edge->mbox_chan = NULL;
  
-	key = "qcom,ipc";

-   ret = of_property_read_u32_index(node, key, 1, >ipc_offset);
-   if (ret < 0) {
-   dev_err(dev, "no offset in %s\n", key);
-   return -EINVAL;
-   }
+   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
+   if (!syscon_np) {
+   dev_err(dev, "no qcom,ipc node\n");
+   return -ENODEV;
+   }
  
-	ret = of_property_read_u32_index(node, key, 2, >ipc_bit);

-   if (ret < 0) {
-   dev_err(dev, "no bit in %s\n", key);
-   return -EINVAL;
+   edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
+   if (IS_ERR(edge->ipc_regmap))
+   return PTR_ERR(edge->ipc_regmap);
+
+   key = "qcom,ipc";
+   ret = of_property_read_u32_index(node, key, 1, 
>ipc_offset);
+   if (ret < 0) {
+   dev_err(dev, "no offset in %s\n", key);
+   return -EINVAL;
+   }
+
+   ret = of_property_read_u32_index(node, key, 2, >ipc_bit);
+   if (ret < 0) {
+   dev_err(dev, "no bit in %s\n", key);
+   return -EINVAL;
+   }
}
  
  	ret = of_property_read_string(node, "label", >name);

@@ -1394,6 +1415,8 @@ struct qcom_smd_edge *qcom_smd_register_edge(struct 
device *parent,
return edge;
  
  unregister_dev:

+   if (!IS_ERR_OR_NULL(edge->mbox_chan))
+   mbox_free_channel(edge->mbox_chan);
put_device(>dev);
return ERR_PTR(ret);
  }
@@ -1422,6 +1445,7 @@ int qcom_smd_unregister_edge(struct qcom_smd_edge *edge)
if (ret)
dev_warn(>dev, "can't remove smd device: %d\n", ret);
  
+	mbox_free_channel(edge->mbox_chan);

device_unregister(>dev);
  
  	return 0;




Re: [PATCH] rpmsg: qcom_smd: Access APCS through mailbox framework

2017-12-06 Thread Jitendra Sharma

Hi Bjorn,


On 11/16/2017 12:38 PM, Bjorn Andersson wrote:

Attempt to acquire the APCS IPC through the mailbox framework and fall
back to the old syscon based approach, to allow us to move away from
using the syscon.

Signed-off-by: Bjorn Andersson 
---
  drivers/rpmsg/qcom_smd.c | 62 +---
  1 file changed, 43 insertions(+), 19 deletions(-)

diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index b01774e9fac0..ef2a526ebc8f 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -14,6 +14,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -107,6 +108,8 @@ static const struct {
   * @ipc_regmap:   regmap handle holding the outgoing ipc register
   * @ipc_offset:   offset within @ipc_regmap of the register for 
ipc
   * @ipc_bit:  bit in the register at @ipc_offset of @ipc_regmap
+ * @mbox_client:   mailbox client handle
+ * @mbox_chan: apcs ipc mailbox channel handle
   * @channels: list of all channels detected on this edge
   * @channels_lock:guard for modifications of @channels
   * @allocated:array of bitmaps representing already allocated 
channels
@@ -129,6 +132,9 @@ struct qcom_smd_edge {
int ipc_offset;
int ipc_bit;
  
+	struct mbox_client mbox_client;

+   struct mbox_chan *mbox_chan;
+
struct list_head channels;
spinlock_t channels_lock;
  
@@ -365,7 +371,12 @@ static void qcom_smd_signal_channel(struct qcom_smd_channel *channel)

  {
struct qcom_smd_edge *edge = channel->edge;
  
-	regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));

+   if (edge->mbox_chan) {
+   mbox_send_message(edge->mbox_chan, NULL);

mbox_send_message could fail. So return value should be checked

+   mbox_client_txdone(edge->mbox_chan, 0);
+   } else {
+   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
BIT(edge->ipc_bit));
+   }
  }
  
  /*

@@ -1268,27 +1279,37 @@ static int qcom_smd_parse_edge(struct device *dev,
key = "qcom,remote-pid";
of_property_read_u32(node, key, >remote_pid);
  
-	syscon_np = of_parse_phandle(node, "qcom,ipc", 0);

-   if (!syscon_np) {
-   dev_err(dev, "no qcom,ipc node\n");
-   return -ENODEV;
-   }
+   edge->mbox_client.dev = dev;
+   edge->mbox_client.knows_txdone = true;
+   edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
+   if (IS_ERR(edge->mbox_chan)) {
+   if (PTR_ERR(edge->mbox_chan) != -ENODEV)
+   return PTR_ERR(edge->mbox_chan);
  
-	edge->ipc_regmap = syscon_node_to_regmap(syscon_np);

-   if (IS_ERR(edge->ipc_regmap))
-   return PTR_ERR(edge->ipc_regmap);
+   edge->mbox_chan = NULL;
  
-	key = "qcom,ipc";

-   ret = of_property_read_u32_index(node, key, 1, >ipc_offset);
-   if (ret < 0) {
-   dev_err(dev, "no offset in %s\n", key);
-   return -EINVAL;
-   }
+   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
+   if (!syscon_np) {
+   dev_err(dev, "no qcom,ipc node\n");
+   return -ENODEV;
+   }
  
-	ret = of_property_read_u32_index(node, key, 2, >ipc_bit);

-   if (ret < 0) {
-   dev_err(dev, "no bit in %s\n", key);
-   return -EINVAL;
+   edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
+   if (IS_ERR(edge->ipc_regmap))
+   return PTR_ERR(edge->ipc_regmap);
+
+   key = "qcom,ipc";
+   ret = of_property_read_u32_index(node, key, 1, 
>ipc_offset);
+   if (ret < 0) {
+   dev_err(dev, "no offset in %s\n", key);
+   return -EINVAL;
+   }
+
+   ret = of_property_read_u32_index(node, key, 2, >ipc_bit);
+   if (ret < 0) {
+   dev_err(dev, "no bit in %s\n", key);
+   return -EINVAL;
+   }
}
  
  	ret = of_property_read_string(node, "label", >name);

@@ -1394,6 +1415,8 @@ struct qcom_smd_edge *qcom_smd_register_edge(struct 
device *parent,
return edge;
  
  unregister_dev:

+   if (!IS_ERR_OR_NULL(edge->mbox_chan))
+   mbox_free_channel(edge->mbox_chan);
put_device(>dev);
return ERR_PTR(ret);
  }
@@ -1422,6 +1445,7 @@ int qcom_smd_unregister_edge(struct qcom_smd_edge *edge)
if (ret)
dev_warn(>dev, "can't remove smd device: %d\n", ret);
  
+	mbox_free_channel(edge->mbox_chan);

device_unregister(>dev);
  
  	return 0;




Re: [PATCH v4 1/5] soc: qcom: Introduce QMI encoder/decoder

2017-12-01 Thread Jitendra Sharma

Hi Bjorn,

Few minor comments ..


On 11/30/2017 6:46 AM, Bjorn Andersson wrote:

Add the helper library for encoding and decoding QMI encoded messages.
The implementation is taken from lib/qmi_encdec.c of the Qualcomm kernel
(msm-3.18).

Modifications has been made to the public API, source buffers has been
made const and the debug-logging part was omitted, for now.

Tested-By: Chris Lew 
Tested-By: Srinivas Kandagatla 
Signed-off-by: Bjorn Andersson 
---

Changes since v3:
- Moved depends on ARCH_QCOM from patch 2
- Kerneldoc updates
- Style updates
- Dropped qrtr.h include from header file
- Rename is_array to array_type

Changes since v2:
- Checkpatch fixes

Changes since v1:
- None

  drivers/soc/qcom/Kconfig  |   9 +
  drivers/soc/qcom/Makefile |   2 +
  drivers/soc/qcom/qmi_encdec.c | 826 ++
  include/linux/soc/qcom/qmi.h  | 114 ++
  4 files changed, 951 insertions(+)
  create mode 100644 drivers/soc/qcom/qmi_encdec.c
  create mode 100644 include/linux/soc/qcom/qmi.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b81374bb6713..2411df0427d9 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -35,6 +35,15 @@ config QCOM_PM
  modes. It interface with various system drivers to put the cores in
  low power modes.
  
+config QCOM_QMI_HELPERS

+   tristate
+   depends on ARCH_QCOM
+   help
+ Helper library for handling QMI encoded messages.  QMI encoded
+ messages are used in communication between the majority of QRTR
+ clients and this helpers provide the common functionality needed for
+ doing this from a kernel driver.
+
  config QCOM_RMTFS_MEM
tristate "Qualcomm Remote Filesystem memory driver"
depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 40c56f67e94a..37f85b45d0a1 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -3,6 +3,8 @@ obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o
  obj-$(CONFIG_QCOM_GSBI)   +=  qcom_gsbi.o
  obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
  obj-$(CONFIG_QCOM_PM) +=  spm.o
+obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
+qmi_helpers-y  += qmi_encdec.o
  obj-$(CONFIG_QCOM_RMTFS_MEM)  += rmtfs_mem.o
  obj-$(CONFIG_QCOM_SMD_RPM)+= smd-rpm.o
  obj-$(CONFIG_QCOM_SMEM) +=smem.o
diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
new file mode 100644
index ..a197fc0114c3
--- /dev/null
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -0,0 +1,826 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 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 and
+ * only 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define QMI_ENCDEC_ENCODE_TLV(type, length, p_dst) do { \
+   *p_dst++ = type; \
+   *p_dst++ = ((u8)((length) & 0xFF)); \
+   *p_dst++ = ((u8)(((length) >> 8) & 0xFF)); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_TLV(p_type, p_length, p_src) do { \
+   *p_type = (u8)*p_src++; \
+   *p_length = (u8)*p_src++; \
+   *p_length |= ((u8)*p_src) << 8; \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_N_BYTES(p_dst, p_src, size) \
+do { \
+   memcpy(p_dst, p_src, size); \
+   p_dst = (u8 *)p_dst + size; \
+   p_src = (u8 *)p_src + size; \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
+do { \
+   memcpy(p_dst, p_src, size); \
+   p_dst = (u8 *)p_dst + size; \
+   p_src = (u8 *)p_src + size; \
+} while (0)
+
+#define UPDATE_ENCODE_VARIABLES(temp_si, buf_dst, \
+   encoded_bytes, tlv_len, encode_tlv, rc) \
+do { \
+   buf_dst = (u8 *)buf_dst + rc; \
+   encoded_bytes += rc; \
+   tlv_len += rc; \
+   temp_si = temp_si + 1; \
+   encode_tlv = 1; \
+} while (0)
+
+#define UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc) \
+do { \
+   buf_src = (u8 *)buf_src + rc; \
+   decoded_bytes += rc; \
+} while (0)
+
+#define TLV_LEN_SIZE sizeof(u16)
+#define TLV_TYPE_SIZE sizeof(u8)
+#define OPTIONAL_TLV_TYPE_START 0x10
+
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+ const void *in_c_struct, u32 out_buf_len,
+ int enc_level);
+
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+ const void *in_buf, u32 

Re: [PATCH v4 1/5] soc: qcom: Introduce QMI encoder/decoder

2017-12-01 Thread Jitendra Sharma

Hi Bjorn,

Few minor comments ..


On 11/30/2017 6:46 AM, Bjorn Andersson wrote:

Add the helper library for encoding and decoding QMI encoded messages.
The implementation is taken from lib/qmi_encdec.c of the Qualcomm kernel
(msm-3.18).

Modifications has been made to the public API, source buffers has been
made const and the debug-logging part was omitted, for now.

Tested-By: Chris Lew 
Tested-By: Srinivas Kandagatla 
Signed-off-by: Bjorn Andersson 
---

Changes since v3:
- Moved depends on ARCH_QCOM from patch 2
- Kerneldoc updates
- Style updates
- Dropped qrtr.h include from header file
- Rename is_array to array_type

Changes since v2:
- Checkpatch fixes

Changes since v1:
- None

  drivers/soc/qcom/Kconfig  |   9 +
  drivers/soc/qcom/Makefile |   2 +
  drivers/soc/qcom/qmi_encdec.c | 826 ++
  include/linux/soc/qcom/qmi.h  | 114 ++
  4 files changed, 951 insertions(+)
  create mode 100644 drivers/soc/qcom/qmi_encdec.c
  create mode 100644 include/linux/soc/qcom/qmi.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b81374bb6713..2411df0427d9 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -35,6 +35,15 @@ config QCOM_PM
  modes. It interface with various system drivers to put the cores in
  low power modes.
  
+config QCOM_QMI_HELPERS

+   tristate
+   depends on ARCH_QCOM
+   help
+ Helper library for handling QMI encoded messages.  QMI encoded
+ messages are used in communication between the majority of QRTR
+ clients and this helpers provide the common functionality needed for
+ doing this from a kernel driver.
+
  config QCOM_RMTFS_MEM
tristate "Qualcomm Remote Filesystem memory driver"
depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 40c56f67e94a..37f85b45d0a1 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -3,6 +3,8 @@ obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o
  obj-$(CONFIG_QCOM_GSBI)   +=  qcom_gsbi.o
  obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
  obj-$(CONFIG_QCOM_PM) +=  spm.o
+obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
+qmi_helpers-y  += qmi_encdec.o
  obj-$(CONFIG_QCOM_RMTFS_MEM)  += rmtfs_mem.o
  obj-$(CONFIG_QCOM_SMD_RPM)+= smd-rpm.o
  obj-$(CONFIG_QCOM_SMEM) +=smem.o
diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
new file mode 100644
index ..a197fc0114c3
--- /dev/null
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -0,0 +1,826 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 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 and
+ * only 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define QMI_ENCDEC_ENCODE_TLV(type, length, p_dst) do { \
+   *p_dst++ = type; \
+   *p_dst++ = ((u8)((length) & 0xFF)); \
+   *p_dst++ = ((u8)(((length) >> 8) & 0xFF)); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_TLV(p_type, p_length, p_src) do { \
+   *p_type = (u8)*p_src++; \
+   *p_length = (u8)*p_src++; \
+   *p_length |= ((u8)*p_src) << 8; \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_N_BYTES(p_dst, p_src, size) \
+do { \
+   memcpy(p_dst, p_src, size); \
+   p_dst = (u8 *)p_dst + size; \
+   p_src = (u8 *)p_src + size; \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
+do { \
+   memcpy(p_dst, p_src, size); \
+   p_dst = (u8 *)p_dst + size; \
+   p_src = (u8 *)p_src + size; \
+} while (0)
+
+#define UPDATE_ENCODE_VARIABLES(temp_si, buf_dst, \
+   encoded_bytes, tlv_len, encode_tlv, rc) \
+do { \
+   buf_dst = (u8 *)buf_dst + rc; \
+   encoded_bytes += rc; \
+   tlv_len += rc; \
+   temp_si = temp_si + 1; \
+   encode_tlv = 1; \
+} while (0)
+
+#define UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc) \
+do { \
+   buf_src = (u8 *)buf_src + rc; \
+   decoded_bytes += rc; \
+} while (0)
+
+#define TLV_LEN_SIZE sizeof(u16)
+#define TLV_TYPE_SIZE sizeof(u8)
+#define OPTIONAL_TLV_TYPE_START 0x10
+
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+ const void *in_c_struct, u32 out_buf_len,
+ int enc_level);
+
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+ const void *in_buf, u32 in_buf_len, int dec_level);
+
+/**
+ * skip_to_next_elem() - Skip to next element in the