| Message ID | 1463111221-6963-6-git-send-email-andy.gross@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On Thu 12 May 20:46 PDT 2016, Andy Gross wrote: > This patch converts the Qualcomm SCM driver to use the streaming DMA APIs > for communication buffers. > > Signed-off-by: Andy Gross <andy.gross@linaro.org> > --- > drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------ > drivers/firmware/qcom_scm.c | 6 +- > drivers/firmware/qcom_scm.h | 10 ++- > 3 files changed, 59 insertions(+), 146 deletions(-) > > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c [..] > +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) > { > int ret; > struct qcom_scm_command *cmd; > struct qcom_scm_response *rsp; > - unsigned long start, end; > + size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len; > + dma_addr_t cmd_phys; > > - cmd = alloc_qcom_scm_command(cmd_len, resp_len); > + cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL); > if (!cmd) > return -ENOMEM; > > + 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((svc_id << 10) | cmd_id); > if (cmd_buf) > - memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len); > + memcpy(cmd->buf, cmd_buf, cmd_len); > + > + rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset); I believe resp_hdr_offset counts from the beginning of the buffer and that this therefor is supposed to be: rsp = (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); With that corrected, feel free to add: Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 13, 2016 at 04:48:52PM -0700, Bjorn Andersson wrote: > > + 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((svc_id << 10) | cmd_id); > > if (cmd_buf) > > - memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len); > > + memcpy(cmd->buf, cmd_buf, cmd_len); > > + > > + rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset); > > I believe resp_hdr_offset counts from the beginning of the buffer and > that this therefor is supposed to be: > > rsp = (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); > > With that corrected, feel free to add: > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> I'll fix that up. Thanks for the review. Andy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andy, On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote: > This patch converts the Qualcomm SCM driver to use the streaming DMA APIs > for communication buffers. > > Signed-off-by: Andy Gross <andy.gross@linaro.org> This patch has landed in linux-next in the form of commit a551c3dbd689 (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org found some boot breakage in next-20160523 on apq8064[1] which was bisected down to this commit. I reverted this commit on top of next-20160523 and it no longer builds, so I didn't validate if things boot again with this patch reverted. Kevin [1] https://kernelci.org/boot/all/job/next/kernel/next-20160523/ -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote: > Hi Andy, > > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote: >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs >> for communication buffers. >> >> Signed-off-by: Andy Gross <andy.gross@linaro.org> > > This patch has landed in linux-next in the form of commit a551c3dbd689 > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org > found some boot breakage in next-20160523 on apq8064[1] which was > bisected down to this commit. > > I reverted this commit on top of next-20160523 and it no longer > builds, so I didn't validate if things boot again with this patch > reverted. Ouch I missed this failure. I'll investigate and get it fixed. Thanks! Andy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote: > On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote: > > Hi Andy, > > > > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote: > >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs > >> for communication buffers. > >> > >> Signed-off-by: Andy Gross <andy.gross@linaro.org> > > > > This patch has landed in linux-next in the form of commit a551c3dbd689 > > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org > > found some boot breakage in next-20160523 on apq8064[1] which was > > bisected down to this commit. > > > > I reverted this commit on top of next-20160523 and it no longer > > builds, so I didn't validate if things boot again with this patch > > reverted. > > Ouch I missed this failure. I'll investigate and get it fixed. So the root cause was the fact that the DFAB clock required by the SCM is an RPM clock. That support isn't present yet in the kernel, so SCM probe fails. The core clock is really only accessed so that we can bump the clock on it up to the max for performance. As such, I'll make it optional in the platform code. This does bring up the issue of probe defer causing issues with the spm driver, as it calls set_warm_boot_addr. That may have to be addressed, but is probably a problem best fixed in the spm. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Andy Gross <andy.gross@linaro.org> writes: > On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote: >> On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote: >> > Hi Andy, >> > >> > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote: >> >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs >> >> for communication buffers. >> >> >> >> Signed-off-by: Andy Gross <andy.gross@linaro.org> >> > >> > This patch has landed in linux-next in the form of commit a551c3dbd689 >> > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org >> > found some boot breakage in next-20160523 on apq8064[1] which was >> > bisected down to this commit. >> > >> > I reverted this commit on top of next-20160523 and it no longer >> > builds, so I didn't validate if things boot again with this patch >> > reverted. >> >> Ouch I missed this failure. I'll investigate and get it fixed. > > So the root cause was the fact that the DFAB clock required by the SCM is an RPM > clock. That support isn't present yet in the kernel, so SCM probe fails. > > The core clock is really only accessed so that we can bump the clock on it up to > the max for performance. As such, I'll make it optional in the platform code. > > This does bring up the issue of probe defer causing issues with the spm driver, > as it calls set_warm_boot_addr. That may have to be addressed, but is probably > a problem best fixed in the spm. Nice, thanks for the explanation. Is there a patch somewhere you'd like me to test? or were you able to dust off an 8064 platform for testing? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25 May 2016 at 15:50, Kevin Hilman <khilman@baylibre.com> wrote: > Andy Gross <andy.gross@linaro.org> writes: > >> On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote: >>> On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote: >>> > Hi Andy, >>> > >>> > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote: >>> >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs >>> >> for communication buffers. >>> >> >>> >> Signed-off-by: Andy Gross <andy.gross@linaro.org> >>> > >>> > This patch has landed in linux-next in the form of commit a551c3dbd689 >>> > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org >>> > found some boot breakage in next-20160523 on apq8064[1] which was >>> > bisected down to this commit. >>> > >>> > I reverted this commit on top of next-20160523 and it no longer >>> > builds, so I didn't validate if things boot again with this patch >>> > reverted. >>> >>> Ouch I missed this failure. I'll investigate and get it fixed. >> >> So the root cause was the fact that the DFAB clock required by the SCM is an RPM >> clock. That support isn't present yet in the kernel, so SCM probe fails. >> >> The core clock is really only accessed so that we can bump the clock on it up to >> the max for performance. As such, I'll make it optional in the platform code. >> >> This does bring up the issue of probe defer causing issues with the spm driver, >> as it calls set_warm_boot_addr. That may have to be addressed, but is probably >> a problem best fixed in the spm. > > Nice, thanks for the explanation. > > Is there a patch somewhere you'd like me to test? or were you able to > dust off an 8064 platform for testing? I had the ifc6410, but it took me a while to find the darn 5V supply. The docs lie when they say 5V-12V. In any case, I threw a patch on top of next to fix this. I need to amend my set of patches to include these changes and add the DT information for the 8064. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/12, Andy Gross wrote: > This patch converts the Qualcomm SCM driver to use the streaming DMA APIs > for communication buffers. Yes, but why? > > Signed-off-by: Andy Gross <andy.gross@linaro.org> > --- > drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------ > drivers/firmware/qcom_scm.c | 6 +- > drivers/firmware/qcom_scm.h | 10 ++- > 3 files changed, 59 insertions(+), 146 deletions(-) > > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c > index 4388d13..e92bf7a 100644 > --- a/drivers/firmware/qcom_scm-32.c > +++ b/drivers/firmware/qcom_scm-32.c > @@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock); > * > * ------------------- <--- struct qcom_scm_command > * | command header | > - * ------------------- <--- qcom_scm_get_command_buffer() > + * ------------------- <--- buf[0] > * | command buffer | > - * ------------------- <--- struct qcom_scm_response and > - * | response header | qcom_scm_command_to_response() > - * ------------------- <--- qcom_scm_get_response_buffer() > + * ------------------- <--- struct qcom_scm_response > + * | response header | > + * ------------------- You don't like my convenience functions? :) I always thought qcom_scm_get_response_buffer() read better than (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset); > * | response buffer | > * ------------------- > * > @@ -96,79 +95,7 @@ struct qcom_scm_response { > __le32 is_complete; > }; > > - > -/** > - * free_qcom_scm_command() - Free an SCM command > - * @cmd: command to free > - * > - * Free an SCM command. > - */ > -static inline void free_qcom_scm_command(struct qcom_scm_command *cmd) > -{ > - kfree(cmd); > -} > - > -/** > - * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_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) > -{ > - return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); > -} > - > -/** > - * qcom_scm_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) > -{ > - return (void *)cmd->buf; > -} > - > -/** > - * qcom_scm_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) > -{ > - return (void *)rsp + le32_to_cpu(rsp->buf_offset); > -} At the least the diff would be more concentrated on what really is happening in this patch if we left these functions as is. > - > -static u32 smc(u32 cmd_addr) > +static u32 smc(dma_addr_t cmd_addr) Please leave this as u32, the interface doesn't support anything wider here so we shouldn't let this change on LPAE. > { > int context_id; > register u32 r0 asm("r0") = 1; > @@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr) > return r0; > } > > -static int __qcom_scm_call(const struct qcom_scm_command *cmd) > -{ > - int ret; > - u32 cmd_addr = virt_to_phys(cmd); > - > - /* > - * Flush the command buffer so that the secure world sees > - * the correct data. > - */ > - secure_flush_area(cmd, cmd->len); Yay we should delete secure_flush_area() too. > - > - ret = smc(cmd_addr); > - if (ret < 0) > - ret = qcom_scm_remap_error(ret); > - > - return ret; > -} > - > - > @@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp) > if (ret) > return ret; > > - ret = __qcom_scm_hdcp_req(req, req_cnt, resp); > + ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp); Hmm maybe we should pass __scm instead of dev? Is there any advantage to that? > qcom_scm_clk_disable(); > return ret; > }
On Thu, Jun 02, 2016 at 04:26:02PM -0700, Stephen Boyd wrote: > On 05/12, Andy Gross wrote: > > This patch converts the Qualcomm SCM driver to use the streaming DMA APIs > > for communication buffers. > > Yes, but why? This was done so that we can remove the secure_flush mechanism, as we are the only consumer (that I could find). Using the DMA APIs would match the scm-64 so it would be symmetric. I'll add this to the commit message to give more reason why the changes are being made. > > > > Signed-off-by: Andy Gross <andy.gross@linaro.org> > > --- > > drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------ > > drivers/firmware/qcom_scm.c | 6 +- > > drivers/firmware/qcom_scm.h | 10 ++- > > 3 files changed, 59 insertions(+), 146 deletions(-) > > > > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c > > index 4388d13..e92bf7a 100644 > > --- a/drivers/firmware/qcom_scm-32.c > > +++ b/drivers/firmware/qcom_scm-32.c > > @@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock); > > * > > * ------------------- <--- struct qcom_scm_command > > * | command header | > > - * ------------------- <--- qcom_scm_get_command_buffer() > > + * ------------------- <--- buf[0] > > * | command buffer | > > - * ------------------- <--- struct qcom_scm_response and > > - * | response header | qcom_scm_command_to_response() > > - * ------------------- <--- qcom_scm_get_response_buffer() > > + * ------------------- <--- struct qcom_scm_response > > + * | response header | > > + * ------------------- > > You don't like my convenience functions? :) I always thought > qcom_scm_get_response_buffer() read better than > > (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset); If it was used in more than one function, yes. But I felt that single use functions were kind of a waste. But this is in the eye of the beholder. > > > * | response buffer | > > * ------------------- > > * > > @@ -96,79 +95,7 @@ struct qcom_scm_response { > > __le32 is_complete; > > }; > > > > - > > -/** > > - * free_qcom_scm_command() - Free an SCM command > > - * @cmd: command to free > > - * > > - * Free an SCM command. > > - */ > > -static inline void free_qcom_scm_command(struct qcom_scm_command *cmd) > > -{ > > - kfree(cmd); > > -} > > - > > -/** > > - * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_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) > > -{ > > - return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); > > -} > > - > > -/** > > - * qcom_scm_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) > > -{ > > - return (void *)cmd->buf; > > -} > > - > > -/** > > - * qcom_scm_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) > > -{ > > - return (void *)rsp + le32_to_cpu(rsp->buf_offset); > > -} > > At the least the diff would be more concentrated on what really > is happening in this patch if we left these functions as is. Ok. I'll run that and see how it turns out. > > - > > -static u32 smc(u32 cmd_addr) > > +static u32 smc(dma_addr_t cmd_addr) > > Please leave this as u32, the interface doesn't support anything > wider here so we shouldn't let this change on LPAE. Fair point. I hadn't considered that. Thanks for catching that. > > { > > int context_id; > > register u32 r0 asm("r0") = 1; > > @@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr) > > return r0; > > } > > > > -static int __qcom_scm_call(const struct qcom_scm_command *cmd) > > -{ > > - int ret; > > - u32 cmd_addr = virt_to_phys(cmd); > > - > > - /* > > - * Flush the command buffer so that the secure world sees > > - * the correct data. > > - */ > > - secure_flush_area(cmd, cmd->len); > > Yay we should delete secure_flush_area() too. Yes I was going to do a follow on to remove that. I opted to not add this to the set of patches. > > - > > - ret = smc(cmd_addr); > > - if (ret < 0) > > - ret = qcom_scm_remap_error(ret); > > - > > - return ret; > > -} > > - > > - > > @@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp) > > if (ret) > > return ret; > > > > - ret = __qcom_scm_hdcp_req(req, req_cnt, resp); > > + ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp); > > Hmm maybe we should pass __scm instead of dev? Is there any > advantage to that? If we needed access to something in the scm other than the device, yes. If we passed scm instead, we might be able to remove the singleton usage. I'll look and see if it makes sense. Otherwise i'd leave the passing of the device. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c index 4388d13..e92bf7a 100644 --- a/drivers/firmware/qcom_scm-32.c +++ b/drivers/firmware/qcom_scm-32.c @@ -23,8 +23,7 @@ #include <linux/errno.h> #include <linux/err.h> #include <linux/qcom_scm.h> - -#include <asm/cacheflush.h> +#include <linux/dma-mapping.h> #include "qcom_scm.h" @@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock); * * ------------------- <--- struct qcom_scm_command * | command header | - * ------------------- <--- qcom_scm_get_command_buffer() + * ------------------- <--- buf[0] * | command buffer | - * ------------------- <--- struct qcom_scm_response and - * | response header | qcom_scm_command_to_response() - * ------------------- <--- qcom_scm_get_response_buffer() + * ------------------- <--- struct qcom_scm_response + * | response header | + * ------------------- * | response buffer | * ------------------- * @@ -96,79 +95,7 @@ struct qcom_scm_response { __le32 is_complete; }; -/** - * alloc_qcom_scm_command() - Allocate an SCM command - * @cmd_size: size of the command buffer - * @resp_size: size of the response buffer - * - * Allocate an SCM command, including enough room for the command - * and response headers as well as the command and response buffers. - * - * Returns a valid &qcom_scm_command on success or %NULL if the allocation fails. - */ -static struct qcom_scm_command *alloc_qcom_scm_command(size_t cmd_size, size_t resp_size) -{ - struct qcom_scm_command *cmd; - size_t len = sizeof(*cmd) + sizeof(struct qcom_scm_response) + cmd_size + - resp_size; - u32 offset; - - cmd = kzalloc(PAGE_ALIGN(len), GFP_KERNEL); - if (cmd) { - cmd->len = cpu_to_le32(len); - offset = offsetof(struct qcom_scm_command, buf); - cmd->buf_offset = cpu_to_le32(offset); - cmd->resp_hdr_offset = cpu_to_le32(offset + cmd_size); - } - return cmd; -} - -/** - * free_qcom_scm_command() - Free an SCM command - * @cmd: command to free - * - * Free an SCM command. - */ -static inline void free_qcom_scm_command(struct qcom_scm_command *cmd) -{ - kfree(cmd); -} - -/** - * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_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) -{ - return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset); -} - -/** - * qcom_scm_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) -{ - return (void *)cmd->buf; -} - -/** - * qcom_scm_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) -{ - return (void *)rsp + le32_to_cpu(rsp->buf_offset); -} - -static u32 smc(u32 cmd_addr) +static u32 smc(dma_addr_t cmd_addr) { int context_id; register u32 r0 asm("r0") = 1; @@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr) return r0; } -static int __qcom_scm_call(const struct qcom_scm_command *cmd) -{ - int ret; - u32 cmd_addr = virt_to_phys(cmd); - - /* - * Flush the command buffer so that the secure world sees - * the correct data. - */ - secure_flush_area(cmd, cmd->len); - - ret = smc(cmd_addr); - if (ret < 0) - ret = qcom_scm_remap_error(ret); - - return ret; -} - -static void qcom_scm_inv_range(unsigned long start, unsigned long end) -{ - u32 cacheline_size, ctr; - - asm volatile("mrc p15, 0, %0, c0, c0, 1" : "=r" (ctr)); - cacheline_size = 4 << ((ctr >> 16) & 0xf); - - start = round_down(start, cacheline_size); - end = round_up(end, cacheline_size); - outer_inv_range(start, end); - while (start < end) { - asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start) - : "memory"); - start += cacheline_size; - } - dsb(); - isb(); -} - /** * qcom_scm_call() - Send an SCM command + * @dev: struct device * @svc_id: service identifier * @cmd_id: command identifier * @cmd_buf: command buffer @@ -247,42 +138,59 @@ static void qcom_scm_inv_range(unsigned long start, unsigned long end) * 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(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, u32 svc_id, u32 cmd_id, + const void *cmd_buf, size_t cmd_len, void *resp_buf, + size_t resp_len) { int ret; struct qcom_scm_command *cmd; struct qcom_scm_response *rsp; - unsigned long start, end; + size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len; + dma_addr_t cmd_phys; - cmd = alloc_qcom_scm_command(cmd_len, resp_len); + cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL); if (!cmd) return -ENOMEM; + 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((svc_id << 10) | cmd_id); if (cmd_buf) - memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len); + memcpy(cmd->buf, cmd_buf, cmd_len); + + rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset); + + cmd_phys = dma_map_single(dev, cmd, alloc_len, DMA_TO_DEVICE); + if (dma_mapping_error(dev, cmd_phys)) { + kfree(cmd); + return -ENOMEM; + } mutex_lock(&qcom_scm_lock); - ret = __qcom_scm_call(cmd); + ret = smc(cmd_phys); + if (ret < 0) + ret = qcom_scm_remap_error(ret); mutex_unlock(&qcom_scm_lock); if (ret) goto out; - rsp = qcom_scm_command_to_response(cmd); - start = (unsigned long)rsp; - do { - qcom_scm_inv_range(start, start + sizeof(*rsp)); + dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len, + sizeof(*rsp), DMA_FROM_DEVICE); } while (!rsp->is_complete); - end = (unsigned long)qcom_scm_get_response_buffer(rsp) + resp_len; - qcom_scm_inv_range(start, end); - - if (resp_buf) - memcpy(resp_buf, qcom_scm_get_response_buffer(rsp), resp_len); + 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, (void *)rsp + le32_to_cpu(rsp->buf_offset), + resp_len); + } out: - free_qcom_scm_command(cmd); + dma_unmap_single(dev, cmd_phys, alloc_len, DMA_TO_DEVICE); + kfree(cmd); return ret; } @@ -437,7 +345,8 @@ int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus) * Set the Linux entry point for the SCM to transfer control to when coming * out of a power down. CPU power down may be executed on cpuidle or hotplug. */ -int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus) +int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, + const cpumask_t *cpus) { int ret; int flags = 0; @@ -463,7 +372,7 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus) cmd.addr = cpu_to_le32(virt_to_phys(entry)); cmd.flags = cpu_to_le32(flags); - ret = qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR, + ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR, &cmd, sizeof(cmd), NULL, 0); if (!ret) { for_each_cpu(cpu, cpus) @@ -487,25 +396,27 @@ void __qcom_scm_cpu_power_down(u32 flags) flags & QCOM_SCM_FLUSH_FLAG_MASK); } -int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id) +int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id) { int ret; __le32 svc_cmd = cpu_to_le32((svc_id << 10) | cmd_id); __le32 ret_val = 0; - ret = qcom_scm_call(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD, &svc_cmd, - sizeof(svc_cmd), &ret_val, sizeof(ret_val)); + ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD, + &svc_cmd, sizeof(svc_cmd), &ret_val, + sizeof(ret_val)); if (ret) return ret; return le32_to_cpu(ret_val); } -int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp) +int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req, + u32 req_cnt, u32 *resp) { if (req_cnt > QCOM_SCM_HDCP_MAX_REQ_CNT) return -ERANGE; - return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP, + return qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP, req, req_cnt * sizeof(*req), resp, sizeof(*resp)); } diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 4d6ae32..23ccec6 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -89,7 +89,7 @@ EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr); */ int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus) { - return __qcom_scm_set_warm_boot_addr(entry, cpus); + return __qcom_scm_set_warm_boot_addr(__scm->dev, entry, cpus); } EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr); @@ -119,7 +119,7 @@ bool qcom_scm_hdcp_available(void) if (ret) return ret; - ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP, + ret = __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP); qcom_scm_clk_disable(); @@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp) if (ret) return ret; - ret = __qcom_scm_hdcp_req(req, req_cnt, resp); + ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp); qcom_scm_clk_disable(); return ret; } diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h index 7dcc733..afe6676 100644 --- a/drivers/firmware/qcom_scm.h +++ b/drivers/firmware/qcom_scm.h @@ -19,7 +19,8 @@ #define QCOM_SCM_FLAG_HLOS 0x01 #define QCOM_SCM_FLAG_COLDBOOT_MC 0x02 #define QCOM_SCM_FLAG_WARMBOOT_MC 0x04 -extern int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus); +extern int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, + const cpumask_t *cpus); extern int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus); #define QCOM_SCM_CMD_TERMINATE_PC 0x2 @@ -29,12 +30,13 @@ extern void __qcom_scm_cpu_power_down(u32 flags); #define QCOM_SCM_SVC_INFO 0x6 #define QCOM_IS_CALL_AVAIL_CMD 0x1 -extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id); +extern int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, + u32 cmd_id); #define QCOM_SCM_SVC_HDCP 0x11 #define QCOM_SCM_CMD_HDCP 0x01 -extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, - u32 *resp); +extern int __qcom_scm_hdcp_req(struct device *dev, + struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp); /* common error codes */ #define QCOM_SCM_ENOMEM -5
This patch converts the Qualcomm SCM driver to use the streaming DMA APIs for communication buffers. Signed-off-by: Andy Gross <andy.gross@linaro.org> --- drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------ drivers/firmware/qcom_scm.c | 6 +- drivers/firmware/qcom_scm.h | 10 ++- 3 files changed, 59 insertions(+), 146 deletions(-)