Remove redundant memory allocation failure.
WARNING: Possible unnecessary 'out of memory' message
+ if (!mdata_buf) {
+ dev_err(__scm->dev, "Allocation of metadata buffer failed.\n");
Signed-off-by: Mukesh Ojha <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
---
Changes in v2: https://lore.kernel.org/lkml/[email protected]/
- Added R-by tag
drivers/firmware/qcom/qcom_scm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 49ddbcab0680..a11fb063cc67 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -554,10 +554,9 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
*/
mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
GFP_KERNEL);
- if (!mdata_buf) {
- dev_err(__scm->dev, "Allocation of metadata buffer failed.\n");
+ if (!mdata_buf)
return -ENOMEM;
- }
+
memcpy(mdata_buf, metadata, size);
ret = qcom_scm_clk_enable();
--
2.7.4
Remove redundant scm argument from qcom_scm_waitq_wakeup().
Signed-off-by: Mukesh Ojha <[email protected]>
---
Changes in v2: https://lore.kernel.org/lkml/[email protected]/
- Fixed incorrect word in the commit text.
drivers/firmware/qcom/qcom_scm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index a11fb063cc67..d9153204cba3 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1771,7 +1771,7 @@ int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
return 0;
}
-static int qcom_scm_waitq_wakeup(struct qcom_scm *scm, unsigned int wq_ctx)
+static int qcom_scm_waitq_wakeup(unsigned int wq_ctx)
{
int ret;
@@ -1803,7 +1803,7 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
goto out;
}
- ret = qcom_scm_waitq_wakeup(scm, wq_ctx);
+ ret = qcom_scm_waitq_wakeup(wq_ctx);
if (ret)
goto out;
} while (more_pending);
--
2.7.4
QCOM_SCM_BOOT_SET_DLOAD_MODE scm command is applicable for very
older SoCs where this command is supported from firmware and
for newer SoCs, dload mode tcsr registers is used for setting
the download mode.
Currently, qcom_scm_set_download_mode() checks for availability
of QCOM_SCM_BOOT_SET_DLOAD_MODE command even for SoCs where this
is not used. Fix this by switching the condition to keep the
command availability check only if dload mode registers are not
available.
Signed-off-by: Mukesh Ojha <[email protected]>
Reviewed-by: Elliot Berman <[email protected]>
---
Changes in v2: https://lore.kernel.org/lkml/[email protected]/
- Reworded the commit text.
drivers/firmware/qcom/qcom_scm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index d9153204cba3..e238ebe21099 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -495,17 +495,14 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
static void qcom_scm_set_download_mode(bool enable)
{
- bool avail;
int ret = 0;
- avail = __qcom_scm_is_call_available(__scm->dev,
- QCOM_SCM_SVC_BOOT,
- QCOM_SCM_BOOT_SET_DLOAD_MODE);
- if (avail) {
- ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
- } else if (__scm->dload_mode_addr) {
+ if (__scm->dload_mode_addr) {
ret = qcom_scm_io_writel(__scm->dload_mode_addr,
- enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+ enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+ } else if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_BOOT,
+ QCOM_SCM_BOOT_SET_DLOAD_MODE)) {
+ ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
} else {
dev_err(__scm->dev,
"No available mechanism for setting download mode\n");
--
2.7.4
It is possible qcom_scm_is_available() gives wrong indication that
if __scm is initialized while __scm->dev is not and similar issue
is also possible with __scm->waitq_comp.
Fix this appropriately by the use of release barrier and read barrier
that will make sure if __scm is initialized so, is all of its field
variable.
Fixes: d0f6fa7ba2d6 ("firmware: qcom: scm: Convert SCM to platform driver")
Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
Signed-off-by: Mukesh Ojha <[email protected]>
---
Changes in v2:
- Added barrier instruction to make the stores available only
after __scm initialization.
- Moved __scm->waitq_comp initialized slight up in program order.
- Added Fixes tag.
drivers/firmware/qcom/qcom_scm.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index e238ebe21099..f014a934a603 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1737,7 +1737,7 @@ static int qcom_scm_qseecom_init(struct qcom_scm *scm)
*/
bool qcom_scm_is_available(void)
{
- return !!__scm;
+ return !!READ_ONCE(__scm);
}
EXPORT_SYMBOL_GPL(qcom_scm_is_available);
@@ -1818,10 +1818,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
if (!scm)
return -ENOMEM;
+ scm->dev = &pdev->dev;
ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
if (ret < 0)
return ret;
+ init_completion(&scm->waitq_comp);
mutex_init(&scm->scm_bw_lock);
scm->path = devm_of_icc_get(&pdev->dev, NULL);
@@ -1853,10 +1855,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
if (ret)
return ret;
- __scm = scm;
- __scm->dev = &pdev->dev;
-
- init_completion(&__scm->waitq_comp);
+ /* Let all above stores be available after this */
+ smp_store_release(&__scm, scm);
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0) {
--
2.7.4
On 21.03.2024 16:24, Mukesh Ojha wrote:
> QCOM_SCM_BOOT_SET_DLOAD_MODE scm command is applicable for very
> older SoCs where this command is supported from firmware and
> for newer SoCs, dload mode tcsr registers is used for setting
> the download mode.
This is a bit of a spaghetti sentence, but I get what you're saying.
And the patch looks good!
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 21.03.2024 16:24, Mukesh Ojha wrote:
> Remove redundant scm argument from qcom_scm_waitq_wakeup().
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
Gentle ping..
-Mukesh
On 3/21/2024 8:53 PM, Mukesh Ojha wrote:
> Remove redundant memory allocation failure.
>
> WARNING: Possible unnecessary 'out of memory' message
> + if (!mdata_buf) {
> + dev_err(__scm->dev, "Allocation of metadata buffer failed.\n");
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> ---
> Changes in v2: https://lore.kernel.org/lkml/[email protected]/
> - Added R-by tag
>
> drivers/firmware/qcom/qcom_scm.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 49ddbcab0680..a11fb063cc67 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -554,10 +554,9 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
> */
> mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
> GFP_KERNEL);
> - if (!mdata_buf) {
> - dev_err(__scm->dev, "Allocation of metadata buffer failed.\n");
> + if (!mdata_buf)
> return -ENOMEM;
> - }
> +
> memcpy(mdata_buf, metadata, size);
>
> ret = qcom_scm_clk_enable();
On Thu, 21 Mar 2024 20:53:59 +0530, Mukesh Ojha wrote:
> Remove redundant memory allocation failure.
>
> WARNING: Possible unnecessary 'out of memory' message
> + if (!mdata_buf) {
> + dev_err(__scm->dev, "Allocation of metadata buffer failed.\n");
>
>
> [...]
Applied, thanks!
[1/4] firmware: qcom: scm: Remove log reporting memory allocation failure
commit: 3de990f7895906a7a18d2dff63e3e525acaa4ecc
[2/4] firmware: scm: Remove redundant scm argument from qcom_scm_waitq_wakeup()
commit: 000636d91d605f6209a635a29d0487af5b12b237
[3/4] firmware: qcom: scm: Rework dload mode availability check
commit: 398a4c58f3f29ac3ff4d777dc91fe40a07bbca8c
[4/4] firmware: qcom: scm: Fix __scm and waitq completion variable initialization
commit: 2e4955167ec5c04534cebea9e8273a907e7a75e1
Best regards,
--
Bjorn Andersson <[email protected]>