| Message ID | 1573593774-12539-2-git-send-email-eberman@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Restructure, improve target support for qcom_scm driver | expand |
Quoting Elliot Berman (2019-11-12 13:22:37) > Rename legacy-specific structures and macros with legacy prefix; rename > smccc-specific structures and macros with smccc prefix. Yes that's what's happening in the patch, but there isn't the most important part in the commit text here, which is _why_ this change is meaningful. Presumably the reason is to clearly separate the original SCM calling convention from the newer SCM calling conventions. > > Signed-off-by: Elliot Berman <eberman@codeaurora.org> > --- > drivers/firmware/qcom_scm-32.c | 70 ++++++++++++++++++++++-------------------- > drivers/firmware/qcom_scm-64.c | 53 ++++++++++++++++---------------- > 2 files changed, 64 insertions(+), 59 deletions(-) > > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c > index bee8729..5d52641 100644 > --- a/drivers/firmware/qcom_scm-32.c > +++ b/drivers/firmware/qcom_scm-32.c > @@ -39,22 +39,22 @@ static struct qcom_scm_entry qcom_scm_wb[] = { > static DEFINE_MUTEX(qcom_scm_lock); > > /** > - * struct qcom_scm_command - one SCM command buffer > + * struct qcom_scm_legacy_command - one SCM command buffer > * @len: total available memory for command and response > * @buf_offset: start of command buffer > * @resp_hdr_offset: start of response buffer > * @id: command to be executed > - * @buf: buffer returned from qcom_scm_get_command_buffer() > + * @buf: buffer returned from legacy_get_command_buffer() I'd prefer to keep qcom_ or at least scm_ somewhere in the name. Just plain legacy_ is too generic. > * > * An SCM command is laid out in memory as follows: > * > - * ------------------- <--- struct qcom_scm_command > + * ------------------- <--- struct qcom_scm_legacy_command > * | command header | > - * ------------------- <--- qcom_scm_get_command_buffer() > + * ------------------- <--- legacy_get_command_buffer() > * | command buffer | > - * ------------------- <--- struct qcom_scm_response and > - * | response header | qcom_scm_command_to_response() > - * ------------------- <--- qcom_scm_get_response_buffer() > + * ------------------- <--- struct qcom_scm_legacy_response and > + * | response header | legacy_command_to_response() > + * ------------------- <--- legacy_get_response_buffer() > * | response buffer | > * ------------------- > * > @@ -62,7 +62,7 @@ static DEFINE_MUTEX(qcom_scm_lock); > * you should always use the appropriate qcom_scm_get_*_buffer() routines Shouldn't this comment be updated too to say "legacy"? > * to access the buffers in a safe manner. > */ > -struct qcom_scm_command { > +struct qcom_scm_legacy_command { Like here, it's called qcom_scm_legacy_<foo> so maybe that should be how it works. > __le32 len; > __le32 buf_offset; > __le32 resp_hdr_offset; > @@ -71,52 +71,55 @@ struct qcom_scm_command { > }; > > /** > - * struct qcom_scm_response - one SCM response buffer > + * struct qcom_scm_legacy_response - one SCM response buffer > * @len: total available memory for response > - * @buf_offset: start of response data relative to start of qcom_scm_response > + * @buf_offset: start of response data relative to start of > + * qcom_scm_legacy_response > * @is_complete: indicates if the command has finished processing > */ > -struct qcom_scm_response { > +struct qcom_scm_legacy_response { > __le32 len; > __le32 buf_offset; > __le32 is_complete; > }; > > /** > - * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_response > + * legacy_command_to_response() - Get a pointer to a qcom_scm_legacy_response > * @cmd: command > * > * Returns a pointer to a response for a command. > */ > -static inline struct qcom_scm_response *qcom_scm_command_to_response( > - const struct qcom_scm_command *cmd) > +static inline struct qcom_scm_legacy_response *legacy_command_to_response( > + const struct qcom_scm_legacy_command *cmd) > { > return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); > } > > /** > - * qcom_scm_get_command_buffer() - Get a pointer to a command buffer > + * legacy_get_command_buffer() - Get a pointer to a command buffer > * @cmd: command > * > * Returns a pointer to the command buffer of a command. > */ > -static inline void *qcom_scm_get_command_buffer(const struct qcom_scm_command *cmd) > +static inline void *legacy_get_command_buffer( > + const struct qcom_scm_legacy_command *cmd) > { > return (void *)cmd->buf; > } > > /** > - * qcom_scm_get_response_buffer() - Get a pointer to a response buffer > + * legacy_get_response_buffer() - Get a pointer to a response buffer > * @rsp: response > * > * Returns a pointer to a response buffer of a response. > */ > -static inline void *qcom_scm_get_response_buffer(const struct qcom_scm_response *rsp) > +static inline void *legacy_get_response_buffer( > + const struct qcom_scm_legacy_response *rsp) > { > return (void *)rsp + le32_to_cpu(rsp->buf_offset); > } > > -static u32 smc(u32 cmd_addr) > +static u32 __qcom_scm_call_do(u32 cmd_addr) > { > int context_id; > register u32 r0 asm("r0") = 1; > @@ -164,8 +167,8 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > size_t resp_len) > { > int ret; > - struct qcom_scm_command *cmd; > - struct qcom_scm_response *rsp; > + struct qcom_scm_legacy_command *cmd; > + struct qcom_scm_legacy_response *rsp; > size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len; > dma_addr_t cmd_phys; > > @@ -179,9 +182,9 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > > cmd->id = cpu_to_le32((svc_id << 10) | cmd_id); > if (cmd_buf) > - memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len); > + memcpy(legacy_get_command_buffer(cmd), cmd_buf, cmd_len); > > - rsp = qcom_scm_command_to_response(cmd); > + rsp = legacy_command_to_response(cmd); > > cmd_phys = dma_map_single(dev, cmd, alloc_len, DMA_TO_DEVICE); > if (dma_mapping_error(dev, cmd_phys)) { > @@ -190,7 +193,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > } > > mutex_lock(&qcom_scm_lock); > - ret = smc(cmd_phys); > + ret = __qcom_scm_call_do(cmd_phys); What is this change about? Is it confusing to have a function called 'smc'? Please mention why this should change in the commit text. > if (ret < 0) > ret = qcom_scm_remap_error(ret); > mutex_unlock(&qcom_scm_lock); > @@ -206,7 +209,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > 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, qcom_scm_get_response_buffer(rsp), > + memcpy(resp_buf, legacy_get_response_buffer(rsp), > resp_len); > } > out: > @@ -215,11 +218,12 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > return ret; > } > > -#define SCM_CLASS_REGISTER (0x2 << 8) > -#define SCM_MASK_IRQS BIT(5) > -#define SCM_ATOMIC(svc, cmd, n) (((((svc) << 10)|((cmd) & 0x3ff)) << 12) | \ > - SCM_CLASS_REGISTER | \ > - SCM_MASK_IRQS | \ > +#define LEGACY_CLASS_REGISTER (0x2 << 8) > +#define LEGACY_MASK_IRQS BIT(5) > +#define LEGACY_ATOMIC_ID(svc, cmd, n) \ > + (((((svc) << 10)|((cmd) & 0x3ff)) << 12) | \ > + LEGACY_CLASS_REGISTER | \ > + LEGACY_MASK_IRQS | \ > (n & 0xf)) > > /** > @@ -235,7 +239,7 @@ static s32 qcom_scm_call_atomic1(u32 svc, u32 cmd, u32 arg1) > { > int context_id; > > - register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 1); > + register u32 r0 asm("r0") = LEGACY_ATOMIC_ID(svc, cmd, 1); > register u32 r1 asm("r1") = (u32)&context_id; > register u32 r2 asm("r2") = arg1; > > @@ -268,7 +272,7 @@ static s32 qcom_scm_call_atomic2(u32 svc, u32 cmd, u32 arg1, u32 arg2) > { > int context_id; > > - register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 2); > + register u32 r0 asm("r0") = LEGACY_ATOMIC_ID(svc, cmd, 2); > register u32 r1 asm("r1") = (u32)&context_id; > register u32 r2 asm("r2") = arg1; > register u32 r3 asm("r3") = arg2; Is this hunk really necessary? The defines are local to the file. > diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c > index 7686786..8226b94 100644 > --- a/drivers/firmware/qcom_scm-64.c > +++ b/drivers/firmware/qcom_scm-64.c > @@ -14,7 +14,7 @@ > > #include "qcom_scm.h" > > -#define QCOM_SCM_FNID(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) > +#define SMCCC_FUNCNUM(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) Is this generic? Maybe it should go into the SMCCC file then if it isn't qcom specific? Otherwise, please leave QCOM in the name. > > #define MAX_QCOM_SCM_ARGS 10 > #define MAX_QCOM_SCM_RETS 3 > @@ -58,11 +58,11 @@ static DEFINE_MUTEX(qcom_scm_lock); > #define QCOM_SCM_EBUSY_WAIT_MS 30 > #define QCOM_SCM_EBUSY_MAX_RETRY 20 > > -#define N_EXT_QCOM_SCM_ARGS 7 > -#define FIRST_EXT_ARG_IDX 3 > -#define N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1) > +#define SMCCC_N_EXT_ARGS 7 > +#define SMCCC_FIRST_EXT_IDX 3 > +#define SMCCC_N_REG_ARGS (MAX_QCOM_SCM_ARGS - SMCCC_N_EXT_ARGS + 1) > > -static void __qcom_scm_call_do(const struct qcom_scm_desc *desc, > +static void __qcom_scm_call_do_quirk(const struct qcom_scm_desc *desc, > struct arm_smccc_res *res, u32 fn_id, > u64 x5, u32 type) > { From here.... > @@ -85,22 +85,23 @@ static void __qcom_scm_call_do(const struct qcom_scm_desc *desc, > } while (res->a0 == QCOM_SCM_INTERRUPTED); > } > > -static void qcom_scm_call_do(const struct qcom_scm_desc *desc, > +static void qcom_scm_call_do_smccc(const struct qcom_scm_desc *desc, > struct arm_smccc_res *res, u32 fn_id, > u64 x5, bool atomic) > { > int retry_count = 0; > > if (atomic) { > - __qcom_scm_call_do(desc, res, fn_id, x5, ARM_SMCCC_FAST_CALL); > + __qcom_scm_call_do_quirk(desc, res, fn_id, x5, > + ARM_SMCCC_FAST_CALL); > return; > } > > do { > mutex_lock(&qcom_scm_lock); > > - __qcom_scm_call_do(desc, res, fn_id, x5, > - ARM_SMCCC_STD_CALL); > + __qcom_scm_call_do_quirk(desc, res, fn_id, x5, > + ARM_SMCCC_STD_CALL); > > mutex_unlock(&qcom_scm_lock); > > @@ -112,21 +113,21 @@ static void qcom_scm_call_do(const struct qcom_scm_desc *desc, > } while (res->a0 == QCOM_SCM_V2_EBUSY); > } > > -static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > - const struct qcom_scm_desc *desc, > - struct arm_smccc_res *res, bool atomic) > +static int ___qcom_scm_call_smccc(struct device *dev, u32 svc_id, u32 cmd_id, > + const struct qcom_scm_desc *desc, > + struct arm_smccc_res *res, bool atomic) > { > int arglen = desc->arginfo & 0xf; > int i; > - u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id); > - u64 x5 = desc->args[FIRST_EXT_ARG_IDX]; > + u32 fn_id = SMCCC_FUNCNUM(svc_id, cmd_id); > + u64 x5 = desc->args[SMCCC_FIRST_EXT_IDX]; > dma_addr_t args_phys = 0; > void *args_virt = NULL; > size_t alloc_len; > gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL; > > - if (unlikely(arglen > N_REGISTER_ARGS)) { > - alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64); > + if (unlikely(arglen > SMCCC_N_REG_ARGS)) { > + alloc_len = SMCCC_N_EXT_ARGS * sizeof(u64); > args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag); > > if (!args_virt) > @@ -135,15 +136,15 @@ static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > if (qcom_smccc_convention == ARM_SMCCC_SMC_32) { > __le32 *args = args_virt; > > - for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++) > + for (i = 0; i < SMCCC_N_EXT_ARGS; i++) > args[i] = cpu_to_le32(desc->args[i + > - FIRST_EXT_ARG_IDX]); > + SMCCC_FIRST_EXT_IDX]); > } else { > __le64 *args = args_virt; > > - for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++) > + for (i = 0; i < SMCCC_N_EXT_ARGS; i++) > args[i] = cpu_to_le64(desc->args[i + > - FIRST_EXT_ARG_IDX]); > + SMCCC_FIRST_EXT_IDX]); > } > > args_phys = dma_map_single(dev, args_virt, alloc_len, > @@ -157,7 +158,7 @@ static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > x5 = args_phys; > } > > - qcom_scm_call_do(desc, res, fn_id, x5, atomic); > + qcom_scm_call_do_smccc(desc, res, fn_id, x5, atomic); > > if (args_virt) { > dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE); > @@ -185,7 +186,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, > struct arm_smccc_res *res) > { > might_sleep(); > - return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, false); > + return ___qcom_scm_call_smccc(dev, svc_id, cmd_id, desc, res, false); > } > > /** > @@ -203,7 +204,7 @@ static int qcom_scm_call_atomic(struct device *dev, u32 svc_id, u32 cmd_id, > const struct qcom_scm_desc *desc, > struct arm_smccc_res *res) > { > - return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, true); > + return ___qcom_scm_call_smccc(dev, svc_id, cmd_id, desc, res, true); > } > > /** > @@ -253,7 +254,7 @@ int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id) > struct arm_smccc_res res; > > desc.arginfo = QCOM_SCM_ARGS(1); > - desc.args[0] = QCOM_SCM_FNID(svc_id, cmd_id) | > + desc.args[0] = SMCCC_FUNCNUM(svc_id, cmd_id) | > (ARM_SMCCC_OWNER_SIP << ARM_SMCCC_OWNER_SHIFT); > > ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD, > @@ -295,7 +296,7 @@ void __qcom_scm_init(void) > { > u64 cmd; > struct arm_smccc_res res; > - u32 function = QCOM_SCM_FNID(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD); > + u32 function = SMCCC_FUNCNUM(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD); > > /* First try a SMC64 call */ > cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, ... to here I don't understand why any of it needs to change. It looks like a bunch of churn and it conflates qcom SCM calls with SMCCC which is not desirable. Those two concepts are different.
On 2019-11-15 15:27, Stephen Boyd wrote: > ... to here I don't understand why any of it needs to change. It looks > like a bunch of churn and it conflates qcom SCM calls with SMCCC which > is not desirable. Those two concepts are different. I can see the confusion. The goal with this patch is to make it more clear which macros and structures are for SCM interface from those which deal with the implementation of how an SCM call is implemented with the smc instruction. It's not presently clear that struct qcom_scm_response (for instance) is only relevant in the context of legacy convention. I choose the name "legacy" since only older firmwares use it and having "scm_buffer_get_command_buffer" seems even more confusing to me! "SMCCC" was chosen for lack of a better name. Additionally, the concern with having qcom_scm_ prefix on these functions (especially legacy_get_*_buffer()) is you get long function names which didn't seem desirable. If the long names are preferable, I can update series with the longer form of the names.
Quoting eberman@codeaurora.org (2019-11-15 17:19:13) > On 2019-11-15 15:27, Stephen Boyd wrote: > > ... to here I don't understand why any of it needs to change. It looks > > like a bunch of churn and it conflates qcom SCM calls with SMCCC which > > is not desirable. Those two concepts are different. > > I can see the confusion. The goal with this patch is to make it more > clear which > macros and structures are for SCM interface from those which deal with > the > implementation of how an SCM call is implemented with the smc > instruction. It's > not presently clear that struct qcom_scm_response (for instance) is only > relevant in the context of legacy convention. > > I choose the name "legacy" since only older firmwares use it and having > "scm_buffer_get_command_buffer" seems even more confusing to me! "SMCCC" > was > chosen for lack of a better name. > > Additionally, the concern with having qcom_scm_ prefix on these > functions > (especially legacy_get_*_buffer()) is you get long function names which > didn't > seem desirable. If the long names are preferable, I can update series > with the > longer form of the names. > This is the hardest problem in computer science. Figuring out a name. ;-) Maybe call it scm_buffer_*? Because it _is_ scm communication with shared buffers? The newer calling convention passes arguments in registers, but the original calling convention passed a buffer around from non-secure to secure world and then back again and had a embryonic register based calling convention. That buffer passing still sort of happens with the new style but it isn't done unless the register count is larger than 5 or so and the return buffer isn't unbounded in size like it was done. It also sort of follows the ARM SMC Calling Convection spec now.
diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c index bee8729..5d52641 100644 --- a/drivers/firmware/qcom_scm-32.c +++ b/drivers/firmware/qcom_scm-32.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2010,2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2010,2015,2019 The Linux Foundation. All rights reserved. * Copyright (C) 2015 Linaro Ltd. */ @@ -39,22 +39,22 @@ static struct qcom_scm_entry qcom_scm_wb[] = { static DEFINE_MUTEX(qcom_scm_lock); /** - * struct qcom_scm_command - one SCM command buffer + * struct qcom_scm_legacy_command - one SCM command buffer * @len: total available memory for command and response * @buf_offset: start of command buffer * @resp_hdr_offset: start of response buffer * @id: command to be executed - * @buf: buffer returned from qcom_scm_get_command_buffer() + * @buf: buffer returned from legacy_get_command_buffer() * * An SCM command is laid out in memory as follows: * - * ------------------- <--- struct qcom_scm_command + * ------------------- <--- struct qcom_scm_legacy_command * | command header | - * ------------------- <--- qcom_scm_get_command_buffer() + * ------------------- <--- legacy_get_command_buffer() * | command buffer | - * ------------------- <--- struct qcom_scm_response and - * | response header | qcom_scm_command_to_response() - * ------------------- <--- qcom_scm_get_response_buffer() + * ------------------- <--- struct qcom_scm_legacy_response and + * | response header | legacy_command_to_response() + * ------------------- <--- legacy_get_response_buffer() * | response buffer | * ------------------- * @@ -62,7 +62,7 @@ static DEFINE_MUTEX(qcom_scm_lock); * you should always use the appropriate qcom_scm_get_*_buffer() routines * to access the buffers in a safe manner. */ -struct qcom_scm_command { +struct qcom_scm_legacy_command { __le32 len; __le32 buf_offset; __le32 resp_hdr_offset; @@ -71,52 +71,55 @@ struct qcom_scm_command { }; /** - * struct qcom_scm_response - one SCM response buffer + * struct qcom_scm_legacy_response - one SCM response buffer * @len: total available memory for response - * @buf_offset: start of response data relative to start of qcom_scm_response + * @buf_offset: start of response data relative to start of + * qcom_scm_legacy_response * @is_complete: indicates if the command has finished processing */ -struct qcom_scm_response { +struct qcom_scm_legacy_response { __le32 len; __le32 buf_offset; __le32 is_complete; }; /** - * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_response + * legacy_command_to_response() - Get a pointer to a qcom_scm_legacy_response * @cmd: command * * Returns a pointer to a response for a command. */ -static inline struct qcom_scm_response *qcom_scm_command_to_response( - const struct qcom_scm_command *cmd) +static inline struct qcom_scm_legacy_response *legacy_command_to_response( + const struct qcom_scm_legacy_command *cmd) { return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); } /** - * qcom_scm_get_command_buffer() - Get a pointer to a command buffer + * legacy_get_command_buffer() - Get a pointer to a command buffer * @cmd: command * * Returns a pointer to the command buffer of a command. */ -static inline void *qcom_scm_get_command_buffer(const struct qcom_scm_command *cmd) +static inline void *legacy_get_command_buffer( + const struct qcom_scm_legacy_command *cmd) { return (void *)cmd->buf; } /** - * qcom_scm_get_response_buffer() - Get a pointer to a response buffer + * legacy_get_response_buffer() - Get a pointer to a response buffer * @rsp: response * * Returns a pointer to a response buffer of a response. */ -static inline void *qcom_scm_get_response_buffer(const struct qcom_scm_response *rsp) +static inline void *legacy_get_response_buffer( + const struct qcom_scm_legacy_response *rsp) { return (void *)rsp + le32_to_cpu(rsp->buf_offset); } -static u32 smc(u32 cmd_addr) +static u32 __qcom_scm_call_do(u32 cmd_addr) { int context_id; register u32 r0 asm("r0") = 1; @@ -164,8 +167,8 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, size_t resp_len) { int ret; - struct qcom_scm_command *cmd; - struct qcom_scm_response *rsp; + struct qcom_scm_legacy_command *cmd; + struct qcom_scm_legacy_response *rsp; size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len; dma_addr_t cmd_phys; @@ -179,9 +182,9 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, cmd->id = cpu_to_le32((svc_id << 10) | cmd_id); if (cmd_buf) - memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len); + memcpy(legacy_get_command_buffer(cmd), cmd_buf, cmd_len); - rsp = qcom_scm_command_to_response(cmd); + rsp = legacy_command_to_response(cmd); cmd_phys = dma_map_single(dev, cmd, alloc_len, DMA_TO_DEVICE); if (dma_mapping_error(dev, cmd_phys)) { @@ -190,7 +193,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, } mutex_lock(&qcom_scm_lock); - ret = smc(cmd_phys); + ret = __qcom_scm_call_do(cmd_phys); if (ret < 0) ret = qcom_scm_remap_error(ret); mutex_unlock(&qcom_scm_lock); @@ -206,7 +209,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, 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, qcom_scm_get_response_buffer(rsp), + memcpy(resp_buf, legacy_get_response_buffer(rsp), resp_len); } out: @@ -215,11 +218,12 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, return ret; } -#define SCM_CLASS_REGISTER (0x2 << 8) -#define SCM_MASK_IRQS BIT(5) -#define SCM_ATOMIC(svc, cmd, n) (((((svc) << 10)|((cmd) & 0x3ff)) << 12) | \ - SCM_CLASS_REGISTER | \ - SCM_MASK_IRQS | \ +#define LEGACY_CLASS_REGISTER (0x2 << 8) +#define LEGACY_MASK_IRQS BIT(5) +#define LEGACY_ATOMIC_ID(svc, cmd, n) \ + (((((svc) << 10)|((cmd) & 0x3ff)) << 12) | \ + LEGACY_CLASS_REGISTER | \ + LEGACY_MASK_IRQS | \ (n & 0xf)) /** @@ -235,7 +239,7 @@ static s32 qcom_scm_call_atomic1(u32 svc, u32 cmd, u32 arg1) { int context_id; - register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 1); + register u32 r0 asm("r0") = LEGACY_ATOMIC_ID(svc, cmd, 1); register u32 r1 asm("r1") = (u32)&context_id; register u32 r2 asm("r2") = arg1; @@ -268,7 +272,7 @@ static s32 qcom_scm_call_atomic2(u32 svc, u32 cmd, u32 arg1, u32 arg2) { int context_id; - register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 2); + register u32 r0 asm("r0") = LEGACY_ATOMIC_ID(svc, cmd, 2); register u32 r1 asm("r1") = (u32)&context_id; register u32 r2 asm("r2") = arg1; register u32 r3 asm("r3") = arg2; diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c index 7686786..8226b94 100644 --- a/drivers/firmware/qcom_scm-64.c +++ b/drivers/firmware/qcom_scm-64.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015,2019 The Linux Foundation. All rights reserved. */ #include <linux/io.h> @@ -14,7 +14,7 @@ #include "qcom_scm.h" -#define QCOM_SCM_FNID(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) +#define SMCCC_FUNCNUM(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) #define MAX_QCOM_SCM_ARGS 10 #define MAX_QCOM_SCM_RETS 3 @@ -58,11 +58,11 @@ static DEFINE_MUTEX(qcom_scm_lock); #define QCOM_SCM_EBUSY_WAIT_MS 30 #define QCOM_SCM_EBUSY_MAX_RETRY 20 -#define N_EXT_QCOM_SCM_ARGS 7 -#define FIRST_EXT_ARG_IDX 3 -#define N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1) +#define SMCCC_N_EXT_ARGS 7 +#define SMCCC_FIRST_EXT_IDX 3 +#define SMCCC_N_REG_ARGS (MAX_QCOM_SCM_ARGS - SMCCC_N_EXT_ARGS + 1) -static void __qcom_scm_call_do(const struct qcom_scm_desc *desc, +static void __qcom_scm_call_do_quirk(const struct qcom_scm_desc *desc, struct arm_smccc_res *res, u32 fn_id, u64 x5, u32 type) { @@ -85,22 +85,23 @@ static void __qcom_scm_call_do(const struct qcom_scm_desc *desc, } while (res->a0 == QCOM_SCM_INTERRUPTED); } -static void qcom_scm_call_do(const struct qcom_scm_desc *desc, +static void qcom_scm_call_do_smccc(const struct qcom_scm_desc *desc, struct arm_smccc_res *res, u32 fn_id, u64 x5, bool atomic) { int retry_count = 0; if (atomic) { - __qcom_scm_call_do(desc, res, fn_id, x5, ARM_SMCCC_FAST_CALL); + __qcom_scm_call_do_quirk(desc, res, fn_id, x5, + ARM_SMCCC_FAST_CALL); return; } do { mutex_lock(&qcom_scm_lock); - __qcom_scm_call_do(desc, res, fn_id, x5, - ARM_SMCCC_STD_CALL); + __qcom_scm_call_do_quirk(desc, res, fn_id, x5, + ARM_SMCCC_STD_CALL); mutex_unlock(&qcom_scm_lock); @@ -112,21 +113,21 @@ static void qcom_scm_call_do(const struct qcom_scm_desc *desc, } while (res->a0 == QCOM_SCM_V2_EBUSY); } -static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, - const struct qcom_scm_desc *desc, - struct arm_smccc_res *res, bool atomic) +static int ___qcom_scm_call_smccc(struct device *dev, u32 svc_id, u32 cmd_id, + const struct qcom_scm_desc *desc, + struct arm_smccc_res *res, bool atomic) { int arglen = desc->arginfo & 0xf; int i; - u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id); - u64 x5 = desc->args[FIRST_EXT_ARG_IDX]; + u32 fn_id = SMCCC_FUNCNUM(svc_id, cmd_id); + u64 x5 = desc->args[SMCCC_FIRST_EXT_IDX]; dma_addr_t args_phys = 0; void *args_virt = NULL; size_t alloc_len; gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL; - if (unlikely(arglen > N_REGISTER_ARGS)) { - alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64); + if (unlikely(arglen > SMCCC_N_REG_ARGS)) { + alloc_len = SMCCC_N_EXT_ARGS * sizeof(u64); args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag); if (!args_virt) @@ -135,15 +136,15 @@ static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, if (qcom_smccc_convention == ARM_SMCCC_SMC_32) { __le32 *args = args_virt; - for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++) + for (i = 0; i < SMCCC_N_EXT_ARGS; i++) args[i] = cpu_to_le32(desc->args[i + - FIRST_EXT_ARG_IDX]); + SMCCC_FIRST_EXT_IDX]); } else { __le64 *args = args_virt; - for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++) + for (i = 0; i < SMCCC_N_EXT_ARGS; i++) args[i] = cpu_to_le64(desc->args[i + - FIRST_EXT_ARG_IDX]); + SMCCC_FIRST_EXT_IDX]); } args_phys = dma_map_single(dev, args_virt, alloc_len, @@ -157,7 +158,7 @@ static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, x5 = args_phys; } - qcom_scm_call_do(desc, res, fn_id, x5, atomic); + qcom_scm_call_do_smccc(desc, res, fn_id, x5, atomic); if (args_virt) { dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE); @@ -185,7 +186,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id, struct arm_smccc_res *res) { might_sleep(); - return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, false); + return ___qcom_scm_call_smccc(dev, svc_id, cmd_id, desc, res, false); } /** @@ -203,7 +204,7 @@ static int qcom_scm_call_atomic(struct device *dev, u32 svc_id, u32 cmd_id, const struct qcom_scm_desc *desc, struct arm_smccc_res *res) { - return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, true); + return ___qcom_scm_call_smccc(dev, svc_id, cmd_id, desc, res, true); } /** @@ -253,7 +254,7 @@ int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id) struct arm_smccc_res res; desc.arginfo = QCOM_SCM_ARGS(1); - desc.args[0] = QCOM_SCM_FNID(svc_id, cmd_id) | + desc.args[0] = SMCCC_FUNCNUM(svc_id, cmd_id) | (ARM_SMCCC_OWNER_SIP << ARM_SMCCC_OWNER_SHIFT); ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD, @@ -295,7 +296,7 @@ void __qcom_scm_init(void) { u64 cmd; struct arm_smccc_res res; - u32 function = QCOM_SCM_FNID(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD); + u32 function = SMCCC_FUNCNUM(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD); /* First try a SMC64 call */ cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64,
Rename legacy-specific structures and macros with legacy prefix; rename smccc-specific structures and macros with smccc prefix. Signed-off-by: Elliot Berman <eberman@codeaurora.org> --- drivers/firmware/qcom_scm-32.c | 70 ++++++++++++++++++++++-------------------- drivers/firmware/qcom_scm-64.c | 53 ++++++++++++++++---------------- 2 files changed, 64 insertions(+), 59 deletions(-)