diff mbox series

[v2,12/18] firmware: qcom_scm-32: Use qcom_scm_desc in non-atomic calls

Message ID 1573593774-12539-13-git-send-email-eberman@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Restructure, improve target support for qcom_scm driver | expand

Commit Message

Elliot Berman Nov. 12, 2019, 9:22 p.m. UTC
Use qcom_scm_desc in non-atomic calls to remove legacy convention
details from every SCM wrapper function.

Signed-off-by: Elliot Berman <eberman@codeaurora.org>
---
 drivers/firmware/qcom_scm-32.c | 283 +++++++++++++++++++++++++----------------
 1 file changed, 172 insertions(+), 111 deletions(-)

Comments

Stephen Boyd Nov. 19, 2019, 10:03 p.m. UTC | #1
Quoting Elliot Berman (2019-11-12 13:22:48)
> Use qcom_scm_desc in non-atomic calls to remove legacy convention
> details from every SCM wrapper function.
> 
> Signed-off-by: Elliot Berman <eberman@codeaurora.org>
> ---
>  drivers/firmware/qcom_scm-32.c | 283 +++++++++++++++++++++++++----------------
>  1 file changed, 172 insertions(+), 111 deletions(-)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index c3aeccf..4c287f6 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -39,6 +39,46 @@ static struct qcom_scm_entry qcom_scm_wb[] = {
>  
>  static DEFINE_MUTEX(qcom_scm_lock);
>  
> +#define MAX_QCOM_SCM_ARGS 10
> +#define MAX_QCOM_SCM_RETS 3
> +
> +enum qcom_scm_arg_types {
> +       QCOM_SCM_VAL,
> +       QCOM_SCM_RO,
> +       QCOM_SCM_RW,
> +       QCOM_SCM_BUFVAL,
> +};
> +
> +#define QCOM_SCM_ARGS_IMPL(num, a, b, c, d, e, f, g, h, i, j, ...) (\
> +                          (((a) & 0x3) << 4) | \
> +                          (((b) & 0x3) << 6) | \
> +                          (((c) & 0x3) << 8) | \
> +                          (((d) & 0x3) << 10) | \
> +                          (((e) & 0x3) << 12) | \
> +                          (((f) & 0x3) << 14) | \
> +                          (((g) & 0x3) << 16) | \
> +                          (((h) & 0x3) << 18) | \
> +                          (((i) & 0x3) << 20) | \
> +                          (((j) & 0x3) << 22) | \
> +                          ((num) & 0xf))
> +
> +#define QCOM_SCM_ARGS(...) QCOM_SCM_ARGS_IMPL(__VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
> +
> +/**
> + * struct qcom_scm_desc
> + * @arginfo:   Metadata describing the arguments in args[]
> + * @args:      The array of arguments for the secure syscall
> + * @res:       The values returned by the secure syscall

Please document all fields. 'res' looks like 'result' actually.

> + */
> +struct qcom_scm_desc {
> +       u32 svc;
> +       u32 cmd;
> +       u32 arginfo;
> +       u64 args[MAX_QCOM_SCM_ARGS];
> +       u64 result[MAX_QCOM_SCM_RETS];

I would split this out to be a pointer argument to the function so that
some callers can pass NULL and we can not waste time converting junk. It
would be good to also indicate how many result values we're expecting
back so that we don't try to access junk that hasn't changed.

> +       u32 owner;
> +};
> +
>  #define LEGACY_FUNCNUM(s, c)   (((s) << 10) | ((c) & 0x3ff))
>  
>  /**
> @@ -135,16 +175,8 @@ static u32 __qcom_scm_call_do(u32 cmd_addr)
>  }
>  
>  /**
> - * qcom_scm_call() - Send an SCM command
> - * @dev: struct device
> - * @svc_id: service identifier
> - * @cmd_id: command identifier
> - * @cmd_buf: command buffer
> - * @cmd_len: length of the command buffer
> - * @resp_buf: response buffer
> - * @resp_len: length of the response buffer
> - *
> - * Sends a command to the SCM and waits for the command to finish processing.
> + * qcom_scm_call() - Sends a command to the SCM and waits for the command to
> + * finish processing.

Please document arguments to this function.

>   *
>   * A note on cache maintenance:
>   * Note that any buffers that are expected to be accessed by the secure world
> @@ -153,15 +185,19 @@ static u32 __qcom_scm_call_do(u32 cmd_addr)
>   * and response buffers is taken care of by qcom_scm_call; however, callers are
>   * responsible for any other cached buffers passed over to the secure world.
>   */
> -static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
> -                        const void *cmd_buf, size_t cmd_len, void *resp_buf,
> -                        size_t resp_len)
> +static int qcom_scm_call(struct device *dev, struct qcom_scm_desc *desc)
>  {
> +       int arglen = desc->arginfo & 0xf;

unsigned int? Or even u8?

>         int ret;
> +       size_t i;

Should be unsigned int and not size_t. size_t is for sizes.

>         struct qcom_scm_legacy_command *cmd;
>         struct qcom_scm_legacy_response *rsp;
> +       const size_t cmd_len = arglen * sizeof(__le32);
> +       const size_t resp_len = MAX_QCOM_SCM_RETS * sizeof(__le32);
>         size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len;
>         dma_addr_t cmd_phys;
> +       __le32 *arg_buf;
> +       __le32 *res_buf;

const res_buf?

>  
>         cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
>         if (!cmd)
> @@ -170,10 +206,11 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
>         cmd->len = cpu_to_le32(alloc_len);
>         cmd->buf_offset = cpu_to_le32(sizeof(*cmd));
>         cmd->resp_hdr_offset = cpu_to_le32(sizeof(*cmd) + cmd_len);
> +       cmd->id = cpu_to_le32(LEGACY_FUNCNUM(desc->svc, desc->cmd));
>  
> -       cmd->id = cpu_to_le32(LEGACY_FUNCNUM(svc_id, cmd_id));
> -       if (cmd_buf)
> -               memcpy(legacy_get_command_buffer(cmd), cmd_buf, cmd_len);
> +       arg_buf = legacy_get_command_buffer(cmd);
> +       for (i = 0; i < arglen; i++)
> +               arg_buf[i] = cpu_to_le32(desc->args[i]);
>  
>         rsp = legacy_command_to_response(cmd);
>  
> @@ -196,13 +233,13 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
>                                         sizeof(*rsp), DMA_FROM_DEVICE);
>         } while (!rsp->is_complete);
>  
> -       if (resp_buf) {
> -               dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len +
> -                                       le32_to_cpu(rsp->buf_offset),
> -                                       resp_len, DMA_FROM_DEVICE);
> -               memcpy(resp_buf, legacy_get_response_buffer(rsp),
> -                      resp_len);
> -       }
> +       dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len +
> +                               le32_to_cpu(rsp->buf_offset),
> +                               resp_len, DMA_FROM_DEVICE);
> +
> +       res_buf = legacy_get_response_buffer(rsp);
> +       for (i = 0; i < MAX_QCOM_SCM_RETS; i++)
> +               desc->result[i] = le32_to_cpu(res_buf[i]);
>  out:
>         dma_unmap_single(dev, cmd_phys, alloc_len, DMA_TO_DEVICE);
>         kfree(cmd);
> @@ -305,10 +342,10 @@ int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry,
>         int ret;
>         int flags = 0;
>         int cpu;
> -       struct {
> -               __le32 flags;
> -               __le32 addr;
> -       } cmd;
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_BOOT,
> +               .cmd = QCOM_SCM_BOOT_SET_ADDR,
> +       };
>  
>         /*
>          * Reassign only if we are switching from hotplug entry point
> @@ -324,10 +361,11 @@ int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry,
>         if (!flags)
>                 return 0;
>  
> -       cmd.addr = cpu_to_le32(virt_to_phys(entry));
> -       cmd.flags = cpu_to_le32(flags);
> -       ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_ADDR,
> -                           &cmd, sizeof(cmd), NULL, 0);
> +       desc.args[0] = flags;
> +       desc.args[1] = virt_to_phys(entry);
> +       desc.arginfo = QCOM_SCM_ARGS(2);

Would be good to only have on way of filling in the descriptor.

> +
> +       ret = qcom_scm_call(dev, &desc);
>         if (!ret) {
>                 for_each_cpu(cpu, cpus)
>                         qcom_scm_wb[cpu].entry = entry;
> @@ -353,26 +391,47 @@ void __qcom_scm_cpu_power_down(u32 flags)
>  int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id)
>  {
>         int ret;
> -       __le32 svc_cmd = cpu_to_le32(LEGACY_FUNCNUM(svc_id, cmd_id));
> -       __le32 ret_val = 0;
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_INFO,
> +               .cmd = QCOM_SCM_INFO_IS_CALL_AVAIL,
> +               .args[0] = LEGACY_FUNCNUM(svc_id, cmd_id),
> +               .arginfo = QCOM_SCM_ARGS(1),
> +       };
>  
> -       ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_SCM_INFO_IS_CALL_AVAIL,
> -                           &svc_cmd, sizeof(svc_cmd), &ret_val,
> -                           sizeof(ret_val));
> -       if (ret)
> -               return ret;
> +       ret = qcom_scm_call(dev, &desc);
>  
> -       return le32_to_cpu(ret_val);
> +       return ret ? : desc.result[0];
>  }
>  
>  int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
>                         u32 req_cnt, u32 *resp)
>  {
> +       int ret;
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_HDCP,
> +               .cmd = QCOM_SCM_HDCP_INVOKE,
> +               .owner = ARM_SMCCC_OWNER_SIP,
> +       };
> +
>         if (req_cnt > QCOM_SCM_HDCP_MAX_REQ_CNT)
>                 return -ERANGE;
>  
> -       return qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_HDCP_INVOKE,
> -               req, req_cnt * sizeof(*req), resp, sizeof(*resp));
> +       desc.args[0] = req[0].addr;
> +       desc.args[1] = req[0].val;
> +       desc.args[2] = req[1].addr;
> +       desc.args[3] = req[1].val;
> +       desc.args[4] = req[2].addr;
> +       desc.args[5] = req[2].val;
> +       desc.args[6] = req[3].addr;
> +       desc.args[7] = req[3].val;
> +       desc.args[8] = req[4].addr;
> +       desc.args[9] = req[4].val;

Is req_cnt always 5? Shouldn't this be some sort of while loop over
req_cnt to pack two values into desc.args[] from req?

> +       desc.arginfo = QCOM_SCM_ARGS(10);
> +
> +       ret = qcom_scm_call(dev, &desc);
> +       *resp = desc.result[0];
> +
> +       return ret;
>  }
>  
>  void __qcom_scm_init(void)
> @@ -381,104 +440,107 @@ void __qcom_scm_init(void)
>  
>  bool __qcom_scm_pas_supported(struct device *dev, u32 peripheral)
>  {
> -       __le32 out;
> -       __le32 in;
>         int ret;
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_PIL,
> +               .cmd = QCOM_SCM_PIL_PAS_IS_SUPPORTED,
> +               .owner = ARM_SMCCC_OWNER_SIP,

owner was passed before? Where is this coming from?

> +       };
>  
> -       in = cpu_to_le32(peripheral);
> -       ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
> -                           QCOM_SCM_PIL_PAS_IS_SUPPORTED,
> -                           &in, sizeof(in),
> -                           &out, sizeof(out));
> +       desc.args[0] = peripheral;
> +       desc.arginfo = QCOM_SCM_ARGS(1);
>  
> -       return ret ? false : !!out;
> +       ret = qcom_scm_call(dev, &desc);
> +
> +       return ret ? false : !!desc.result[0];
>  }
>
diff mbox series

Patch

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index c3aeccf..4c287f6 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -39,6 +39,46 @@  static struct qcom_scm_entry qcom_scm_wb[] = {
 
 static DEFINE_MUTEX(qcom_scm_lock);
 
+#define MAX_QCOM_SCM_ARGS 10
+#define MAX_QCOM_SCM_RETS 3
+
+enum qcom_scm_arg_types {
+	QCOM_SCM_VAL,
+	QCOM_SCM_RO,
+	QCOM_SCM_RW,
+	QCOM_SCM_BUFVAL,
+};
+
+#define QCOM_SCM_ARGS_IMPL(num, a, b, c, d, e, f, g, h, i, j, ...) (\
+			   (((a) & 0x3) << 4) | \
+			   (((b) & 0x3) << 6) | \
+			   (((c) & 0x3) << 8) | \
+			   (((d) & 0x3) << 10) | \
+			   (((e) & 0x3) << 12) | \
+			   (((f) & 0x3) << 14) | \
+			   (((g) & 0x3) << 16) | \
+			   (((h) & 0x3) << 18) | \
+			   (((i) & 0x3) << 20) | \
+			   (((j) & 0x3) << 22) | \
+			   ((num) & 0xf))
+
+#define QCOM_SCM_ARGS(...) QCOM_SCM_ARGS_IMPL(__VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
+
+/**
+ * struct qcom_scm_desc
+ * @arginfo:	Metadata describing the arguments in args[]
+ * @args:	The array of arguments for the secure syscall
+ * @res:	The values returned by the secure syscall
+ */
+struct qcom_scm_desc {
+	u32 svc;
+	u32 cmd;
+	u32 arginfo;
+	u64 args[MAX_QCOM_SCM_ARGS];
+	u64 result[MAX_QCOM_SCM_RETS];
+	u32 owner;
+};
+
 #define LEGACY_FUNCNUM(s, c)	(((s) << 10) | ((c) & 0x3ff))
 
 /**
@@ -135,16 +175,8 @@  static u32 __qcom_scm_call_do(u32 cmd_addr)
 }
 
 /**
- * qcom_scm_call() - Send an SCM command
- * @dev: struct device
- * @svc_id: service identifier
- * @cmd_id: command identifier
- * @cmd_buf: command buffer
- * @cmd_len: length of the command buffer
- * @resp_buf: response buffer
- * @resp_len: length of the response buffer
- *
- * Sends a command to the SCM and waits for the command to finish processing.
+ * qcom_scm_call() - Sends a command to the SCM and waits for the command to
+ * finish processing.
  *
  * A note on cache maintenance:
  * Note that any buffers that are expected to be accessed by the secure world
@@ -153,15 +185,19 @@  static u32 __qcom_scm_call_do(u32 cmd_addr)
  * and response buffers is taken care of by qcom_scm_call; however, callers are
  * responsible for any other cached buffers passed over to the secure world.
  */
-static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
-			 const void *cmd_buf, size_t cmd_len, void *resp_buf,
-			 size_t resp_len)
+static int qcom_scm_call(struct device *dev, struct qcom_scm_desc *desc)
 {
+	int arglen = desc->arginfo & 0xf;
 	int ret;
+	size_t i;
 	struct qcom_scm_legacy_command *cmd;
 	struct qcom_scm_legacy_response *rsp;
+	const size_t cmd_len = arglen * sizeof(__le32);
+	const size_t resp_len = MAX_QCOM_SCM_RETS * sizeof(__le32);
 	size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len;
 	dma_addr_t cmd_phys;
+	__le32 *arg_buf;
+	__le32 *res_buf;
 
 	cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
 	if (!cmd)
@@ -170,10 +206,11 @@  static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
 	cmd->len = cpu_to_le32(alloc_len);
 	cmd->buf_offset = cpu_to_le32(sizeof(*cmd));
 	cmd->resp_hdr_offset = cpu_to_le32(sizeof(*cmd) + cmd_len);
+	cmd->id = cpu_to_le32(LEGACY_FUNCNUM(desc->svc, desc->cmd));
 
-	cmd->id = cpu_to_le32(LEGACY_FUNCNUM(svc_id, cmd_id));
-	if (cmd_buf)
-		memcpy(legacy_get_command_buffer(cmd), cmd_buf, cmd_len);
+	arg_buf = legacy_get_command_buffer(cmd);
+	for (i = 0; i < arglen; i++)
+		arg_buf[i] = cpu_to_le32(desc->args[i]);
 
 	rsp = legacy_command_to_response(cmd);
 
@@ -196,13 +233,13 @@  static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
 					sizeof(*rsp), DMA_FROM_DEVICE);
 	} while (!rsp->is_complete);
 
-	if (resp_buf) {
-		dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len +
-					le32_to_cpu(rsp->buf_offset),
-					resp_len, DMA_FROM_DEVICE);
-		memcpy(resp_buf, legacy_get_response_buffer(rsp),
-		       resp_len);
-	}
+	dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len +
+				le32_to_cpu(rsp->buf_offset),
+				resp_len, DMA_FROM_DEVICE);
+
+	res_buf = legacy_get_response_buffer(rsp);
+	for (i = 0; i < MAX_QCOM_SCM_RETS; i++)
+		desc->result[i] = le32_to_cpu(res_buf[i]);
 out:
 	dma_unmap_single(dev, cmd_phys, alloc_len, DMA_TO_DEVICE);
 	kfree(cmd);
@@ -305,10 +342,10 @@  int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry,
 	int ret;
 	int flags = 0;
 	int cpu;
-	struct {
-		__le32 flags;
-		__le32 addr;
-	} cmd;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_BOOT,
+		.cmd = QCOM_SCM_BOOT_SET_ADDR,
+	};
 
 	/*
 	 * Reassign only if we are switching from hotplug entry point
@@ -324,10 +361,11 @@  int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry,
 	if (!flags)
 		return 0;
 
-	cmd.addr = cpu_to_le32(virt_to_phys(entry));
-	cmd.flags = cpu_to_le32(flags);
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_ADDR,
-			    &cmd, sizeof(cmd), NULL, 0);
+	desc.args[0] = flags;
+	desc.args[1] = virt_to_phys(entry);
+	desc.arginfo = QCOM_SCM_ARGS(2);
+
+	ret = qcom_scm_call(dev, &desc);
 	if (!ret) {
 		for_each_cpu(cpu, cpus)
 			qcom_scm_wb[cpu].entry = entry;
@@ -353,26 +391,47 @@  void __qcom_scm_cpu_power_down(u32 flags)
 int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id)
 {
 	int ret;
-	__le32 svc_cmd = cpu_to_le32(LEGACY_FUNCNUM(svc_id, cmd_id));
-	__le32 ret_val = 0;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_INFO,
+		.cmd = QCOM_SCM_INFO_IS_CALL_AVAIL,
+		.args[0] = LEGACY_FUNCNUM(svc_id, cmd_id),
+		.arginfo = QCOM_SCM_ARGS(1),
+	};
 
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_SCM_INFO_IS_CALL_AVAIL,
-			    &svc_cmd, sizeof(svc_cmd), &ret_val,
-			    sizeof(ret_val));
-	if (ret)
-		return ret;
+	ret = qcom_scm_call(dev, &desc);
 
-	return le32_to_cpu(ret_val);
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
 			u32 req_cnt, u32 *resp)
 {
+	int ret;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_HDCP,
+		.cmd = QCOM_SCM_HDCP_INVOKE,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+
 	if (req_cnt > QCOM_SCM_HDCP_MAX_REQ_CNT)
 		return -ERANGE;
 
-	return qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_HDCP_INVOKE,
-		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
+	desc.args[0] = req[0].addr;
+	desc.args[1] = req[0].val;
+	desc.args[2] = req[1].addr;
+	desc.args[3] = req[1].val;
+	desc.args[4] = req[2].addr;
+	desc.args[5] = req[2].val;
+	desc.args[6] = req[3].addr;
+	desc.args[7] = req[3].val;
+	desc.args[8] = req[4].addr;
+	desc.args[9] = req[4].val;
+	desc.arginfo = QCOM_SCM_ARGS(10);
+
+	ret = qcom_scm_call(dev, &desc);
+	*resp = desc.result[0];
+
+	return ret;
 }
 
 void __qcom_scm_init(void)
@@ -381,104 +440,107 @@  void __qcom_scm_init(void)
 
 bool __qcom_scm_pas_supported(struct device *dev, u32 peripheral)
 {
-	__le32 out;
-	__le32 in;
 	int ret;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_IS_SUPPORTED,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 
-	in = cpu_to_le32(peripheral);
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
-			    QCOM_SCM_PIL_PAS_IS_SUPPORTED,
-			    &in, sizeof(in),
-			    &out, sizeof(out));
+	desc.args[0] = peripheral;
+	desc.arginfo = QCOM_SCM_ARGS(1);
 
-	return ret ? false : !!out;
+	ret = qcom_scm_call(dev, &desc);
+
+	return ret ? false : !!desc.result[0];
 }
 
 int __qcom_scm_pas_init_image(struct device *dev, u32 peripheral,
 			      dma_addr_t metadata_phys)
 {
-	__le32 scm_ret;
 	int ret;
-	struct {
-		__le32 proc;
-		__le32 image_addr;
-	} request;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 
-	request.proc = cpu_to_le32(peripheral);
-	request.image_addr = cpu_to_le32(metadata_phys);
+	desc.args[0] = peripheral;
+	desc.args[1] = metadata_phys;
+	desc.arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW);
 
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
-			    QCOM_SCM_PIL_PAS_INIT_IMAGE,
-			    &request, sizeof(request),
-			    &scm_ret, sizeof(scm_ret));
+	ret = qcom_scm_call(dev, &desc);
 
-	return ret ? : le32_to_cpu(scm_ret);
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_pas_mem_setup(struct device *dev, u32 peripheral,
-			     phys_addr_t addr, phys_addr_t size)
+			      phys_addr_t addr, phys_addr_t size)
 {
-	__le32 scm_ret;
 	int ret;
-	struct {
-		__le32 proc;
-		__le32 addr;
-		__le32 len;
-	} request;
-
-	request.proc = cpu_to_le32(peripheral);
-	request.addr = cpu_to_le32(addr);
-	request.len = cpu_to_le32(size);
-
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
-			    QCOM_SCM_PIL_PAS_MEM_SETUP,
-			    &request, sizeof(request),
-			    &scm_ret, sizeof(scm_ret));
-
-	return ret ? : le32_to_cpu(scm_ret);
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_MEM_SETUP,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+
+	desc.args[0] = peripheral;
+	desc.args[1] = addr;
+	desc.args[2] = size;
+	desc.arginfo = QCOM_SCM_ARGS(3);
+
+	ret = qcom_scm_call(dev, &desc);
+
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_pas_auth_and_reset(struct device *dev, u32 peripheral)
 {
-	__le32 out;
-	__le32 in;
 	int ret;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_AUTH_AND_RESET,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 
-	in = cpu_to_le32(peripheral);
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
-			    QCOM_SCM_PIL_PAS_AUTH_AND_RESET,
-			    &in, sizeof(in),
-			    &out, sizeof(out));
+	ret = qcom_scm_call(dev, &desc);
 
-	return ret ? : le32_to_cpu(out);
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_pas_shutdown(struct device *dev, u32 peripheral)
 {
-	__le32 out;
-	__le32 in;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_SHUTDOWN,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 	int ret;
 
-	in = cpu_to_le32(peripheral);
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL,
-			    QCOM_SCM_PIL_PAS_SHUTDOWN,
-			    &in, sizeof(in),
-			    &out, sizeof(out));
+	desc.args[0] = peripheral;
+	desc.arginfo = QCOM_SCM_ARGS(1);
 
-	return ret ? : le32_to_cpu(out);
+	ret = qcom_scm_call(dev, &desc);
+
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)
 {
-	__le32 out;
-	__le32 in = cpu_to_le32(reset);
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_MSS_RESET,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 	int ret;
 
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_PIL, QCOM_SCM_PIL_PAS_MSS_RESET,
-			&in, sizeof(in),
-			&out, sizeof(out));
+	desc.args[0] = reset;
+	desc.args[1] = 0;
+	desc.arginfo = QCOM_SCM_ARGS(2);
 
-	return ret ? : le32_to_cpu(out);
+	ret = qcom_scm_call(dev, &desc);
+
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
@@ -489,20 +551,19 @@  int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id)
 {
-	struct {
-		__le32 state;
-		__le32 id;
-	} req;
-	__le32 scm_ret = 0;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_BOOT,
+		.cmd = QCOM_SCM_BOOT_SET_REMOTE_STATE,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
 	int ret;
 
-	req.state = cpu_to_le32(state);
-	req.id = cpu_to_le32(id);
+	desc.args[0] = state;
+	desc.args[1] = id;
 
-	ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_REMOTE_STATE,
-			    &req, sizeof(req), &scm_ret, sizeof(scm_ret));
+	ret = qcom_scm_call(dev, &desc);
 
-	return ret ? : le32_to_cpu(scm_ret);
+	return ret ? : desc.result[0];
 }
 
 int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,