[PATCH -next 1/3] remoteproc: qcom: wcss: check return value after calling platform_get_resource_byname()

[PATCH -next 1/3] remoteproc: qcom: wcss: check return value after calling platform_get_resource_byname()

[Yang Yingliang]

If platform_get_resource_byname() fails, 'res' will be set to null pointer,
it will cause null-ptr-deref when it used in devm_ioremap(), so we need check
the return value.

Fixes: 0af65b9b915e ("remoteproc: qcom: wcss: Add non pas wcss Q6 support for QCS404")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index bb0947f7770e..017ee225dc32 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -827,6 +827,8 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
 	int ret;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qdsp6");
+	if (!res)
+		return -EINVAL;
 	wcss->reg_base = devm_ioremap(&pdev->dev, res->start,
 				      resource_size(res));
 	if (!wcss->reg_base)
-- 
2.25.1

[PATCH -next 2/3] remoteproc: qcom: wcss: Use devm_platform_ioremap_resource_byname()

[Yang Yingliang]

Use the devm_platform_ioremap_resource_byname() helper instead of
calling platform_get_resource_byname() and devm_ioremap_resource()
separately.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 017ee225dc32..209fc1bd1424 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -835,8 +835,7 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
 		return -ENOMEM;
 
 	if (wcss->version == WCSS_IPQ8074) {
-		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rmb");
-		wcss->rmb_base = devm_ioremap_resource(&pdev->dev, res);
+		wcss->rmb_base = devm_platform_ioremap_resource_byname(pdev, "rmb");
 		if (IS_ERR(wcss->rmb_base))
 			return PTR_ERR(wcss->rmb_base);
 	}
-- 
2.25.1

Re: [PATCH -next 2/3] remoteproc: qcom: wcss: Use devm_platform_ioremap_resource_byname()

[Mukesh Ojha]

On 8/26/2022 7:15 AM, Yang Yingliang wrote:
> Use the devm_platform_ioremap_resource_byname() helper instead of
> calling platform_get_resource_byname() and devm_ioremap_resource()
> separately.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>   drivers/remoteproc/qcom_q6v5_wcss.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
> index 017ee225dc32..209fc1bd1424 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -835,8 +835,7 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
>   		return -ENOMEM;
>   
>   	if (wcss->version == WCSS_IPQ8074) {
> -		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rmb");
> -		wcss->rmb_base = devm_ioremap_resource(&pdev->dev, res);
> +		wcss->rmb_base = devm_platform_ioremap_resource_byname(pdev, "rmb");

LGTM.

Reviewed-by:Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh


>   		if (IS_ERR(wcss->rmb_base))
>   			return PTR_ERR(wcss->rmb_base);
>   	}

[PATCH -next 3/3] remoteproc: qcom: wcss: Use dev_err_probe() helper in q6v5_wcss_init_clock()

[Yang Yingliang]

dev_err() can be replace with dev_err_probe() which will check if error
code is -EPROBE_DEFER.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 90 ++++++++++-------------------
 1 file changed, 30 insertions(+), 60 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 209fc1bd1424..ec9ad2c019aa 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -902,87 +902,57 @@ static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
 	int ret;
 
 	wcss->xo = devm_clk_get(wcss->dev, "xo");
-	if (IS_ERR(wcss->xo)) {
-		ret = PTR_ERR(wcss->xo);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get xo clock");
-		return ret;
-	}
+	if (IS_ERR(wcss->xo))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->xo),
+				     "failed to get xo clock");
 
 	wcss->gcc_abhs_cbcr = devm_clk_get(wcss->dev, "gcc_abhs_cbcr");
-	if (IS_ERR(wcss->gcc_abhs_cbcr)) {
-		ret = PTR_ERR(wcss->gcc_abhs_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get gcc abhs clock");
-		return ret;
-	}
+	if (IS_ERR(wcss->gcc_abhs_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->gcc_abhs_cbcr),
+				     "failed to get gcc abhs clock");
 
 	wcss->gcc_axim_cbcr = devm_clk_get(wcss->dev, "gcc_axim_cbcr");
-	if (IS_ERR(wcss->gcc_axim_cbcr)) {
-		ret = PTR_ERR(wcss->gcc_axim_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get gcc axim clock\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->gcc_axim_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->gcc_axim_cbcr),
+				     "failed to get gcc axim clock\n");
 
 	wcss->ahbfabric_cbcr_clk = devm_clk_get(wcss->dev,
 						"lcc_ahbfabric_cbc");
-	if (IS_ERR(wcss->ahbfabric_cbcr_clk)) {
-		ret = PTR_ERR(wcss->ahbfabric_cbcr_clk);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get ahbfabric clock\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->ahbfabric_cbcr_clk))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->ahbfabric_cbcr_clk),
+				     "failed to get ahbfabric clock\n");
 
 	wcss->lcc_csr_cbcr = devm_clk_get(wcss->dev, "tcsr_lcc_cbc");
-	if (IS_ERR(wcss->lcc_csr_cbcr)) {
-		ret = PTR_ERR(wcss->lcc_csr_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get csr cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->lcc_csr_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->lcc_csr_cbcr),
+				     "failed to get csr cbcr clk\n");
 
 	wcss->ahbs_cbcr = devm_clk_get(wcss->dev,
 				       "lcc_abhs_cbc");
-	if (IS_ERR(wcss->ahbs_cbcr)) {
-		ret = PTR_ERR(wcss->ahbs_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get ahbs_cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->ahbs_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->ahbs_cbcr),
+				     "failed to get ahbs_cbcr clk\n");
 
 	wcss->tcm_slave_cbcr = devm_clk_get(wcss->dev,
 					    "lcc_tcm_slave_cbc");
-	if (IS_ERR(wcss->tcm_slave_cbcr)) {
-		ret = PTR_ERR(wcss->tcm_slave_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get tcm cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->tcm_slave_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->tcm_slave_cbcr),
+				     "failed to get tcm cbcr clk\n");
 
 	wcss->qdsp6ss_abhm_cbcr = devm_clk_get(wcss->dev, "lcc_abhm_cbc");
-	if (IS_ERR(wcss->qdsp6ss_abhm_cbcr)) {
-		ret = PTR_ERR(wcss->qdsp6ss_abhm_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get abhm cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->qdsp6ss_abhm_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->qdsp6ss_abhm_cbcr),
+				     "failed to get abhm cbcr clk\n");
 
 	wcss->qdsp6ss_axim_cbcr = devm_clk_get(wcss->dev, "lcc_axim_cbc");
-	if (IS_ERR(wcss->qdsp6ss_axim_cbcr)) {
-		ret = PTR_ERR(wcss->qdsp6ss_axim_cbcr);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get axim cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->qdsp6ss_axim_cbcr))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->qdsp6ss_axim_cbcr),
+				     "failed to get axim cbcr clk\n");
 
 	wcss->lcc_bcr_sleep = devm_clk_get(wcss->dev, "lcc_bcr_sleep");
-	if (IS_ERR(wcss->lcc_bcr_sleep)) {
-		ret = PTR_ERR(wcss->lcc_bcr_sleep);
-		if (ret != -EPROBE_DEFER)
-			dev_err(wcss->dev, "failed to get bcr cbcr clk\n");
-		return ret;
-	}
+	if (IS_ERR(wcss->lcc_bcr_sleep))
+		return dev_err_probe(wcss->dev, PTR_ERR(wcss->lcc_bcr_sleep),
+				     "failed to get bcr cbcr clk\n");
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH -next 3/3] remoteproc: qcom: wcss: Use dev_err_probe() helper in q6v5_wcss_init_clock()

[Mukesh Ojha]

On 8/26/2022 7:15 AM, Yang Yingliang wrote:
> dev_err() can be replace with dev_err_probe() which will check if error
> code is -EPROBE_DEFER.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>   drivers/remoteproc/qcom_q6v5_wcss.c | 90 ++++++++++-------------------
>   1 file changed, 30 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
> index 209fc1bd1424..ec9ad2c019aa 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -902,87 +902,57 @@ static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
>   	int ret;
>   
>   	wcss->xo = devm_clk_get(wcss->dev, "xo");
> -	if (IS_ERR(wcss->xo)) {
> -		ret = PTR_ERR(wcss->xo);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get xo clock");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->xo))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->xo),
> +				     "failed to get xo clock");
>   
>   	wcss->gcc_abhs_cbcr = devm_clk_get(wcss->dev, "gcc_abhs_cbcr");
> -	if (IS_ERR(wcss->gcc_abhs_cbcr)) {
> -		ret = PTR_ERR(wcss->gcc_abhs_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get gcc abhs clock");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->gcc_abhs_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->gcc_abhs_cbcr),
> +				     "failed to get gcc abhs clock");
>   
>   	wcss->gcc_axim_cbcr = devm_clk_get(wcss->dev, "gcc_axim_cbcr");
> -	if (IS_ERR(wcss->gcc_axim_cbcr)) {
> -		ret = PTR_ERR(wcss->gcc_axim_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get gcc axim clock\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->gcc_axim_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->gcc_axim_cbcr),
> +				     "failed to get gcc axim clock\n");
>   
>   	wcss->ahbfabric_cbcr_clk = devm_clk_get(wcss->dev,
>   						"lcc_ahbfabric_cbc");
> -	if (IS_ERR(wcss->ahbfabric_cbcr_clk)) {
> -		ret = PTR_ERR(wcss->ahbfabric_cbcr_clk);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get ahbfabric clock\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->ahbfabric_cbcr_clk))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->ahbfabric_cbcr_clk),
> +				     "failed to get ahbfabric clock\n");
>   
>   	wcss->lcc_csr_cbcr = devm_clk_get(wcss->dev, "tcsr_lcc_cbc");
> -	if (IS_ERR(wcss->lcc_csr_cbcr)) {
> -		ret = PTR_ERR(wcss->lcc_csr_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get csr cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->lcc_csr_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->lcc_csr_cbcr),
> +				     "failed to get csr cbcr clk\n");
>   
>   	wcss->ahbs_cbcr = devm_clk_get(wcss->dev,
>   				       "lcc_abhs_cbc");
> -	if (IS_ERR(wcss->ahbs_cbcr)) {
> -		ret = PTR_ERR(wcss->ahbs_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get ahbs_cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->ahbs_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->ahbs_cbcr),
> +				     "failed to get ahbs_cbcr clk\n");
>   
>   	wcss->tcm_slave_cbcr = devm_clk_get(wcss->dev,
>   					    "lcc_tcm_slave_cbc");
> -	if (IS_ERR(wcss->tcm_slave_cbcr)) {
> -		ret = PTR_ERR(wcss->tcm_slave_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get tcm cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->tcm_slave_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->tcm_slave_cbcr),
> +				     "failed to get tcm cbcr clk\n");
>   
>   	wcss->qdsp6ss_abhm_cbcr = devm_clk_get(wcss->dev, "lcc_abhm_cbc");
> -	if (IS_ERR(wcss->qdsp6ss_abhm_cbcr)) {
> -		ret = PTR_ERR(wcss->qdsp6ss_abhm_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get abhm cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->qdsp6ss_abhm_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->qdsp6ss_abhm_cbcr),
> +				     "failed to get abhm cbcr clk\n");
>   
>   	wcss->qdsp6ss_axim_cbcr = devm_clk_get(wcss->dev, "lcc_axim_cbc");
> -	if (IS_ERR(wcss->qdsp6ss_axim_cbcr)) {
> -		ret = PTR_ERR(wcss->qdsp6ss_axim_cbcr);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get axim cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->qdsp6ss_axim_cbcr))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->qdsp6ss_axim_cbcr),
> +				     "failed to get axim cbcr clk\n");
>   
>   	wcss->lcc_bcr_sleep = devm_clk_get(wcss->dev, "lcc_bcr_sleep");
> -	if (IS_ERR(wcss->lcc_bcr_sleep)) {
> -		ret = PTR_ERR(wcss->lcc_bcr_sleep);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(wcss->dev, "failed to get bcr cbcr clk\n");
> -		return ret;
> -	}
> +	if (IS_ERR(wcss->lcc_bcr_sleep))
> +		return dev_err_probe(wcss->dev, PTR_ERR(wcss->lcc_bcr_sleep),
> +				     "failed to get bcr cbcr clk\n");
>   
>   	return 0;

LGTM.

Reviewed-by:Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh


>   }

Re: [PATCH -next 1/3] remoteproc: qcom: wcss: check return value after calling platform_get_resource_byname()

[Mukesh Ojha]

On 8/26/2022 7:15 AM, Yang Yingliang wrote:
> If platform_get_resource_byname() fails, 'res' will be set to null pointer,
> it will cause null-ptr-deref when it used in devm_ioremap(), so we need check
> the return value.
> 
> Fixes: 0af65b9b915e ("remoteproc: qcom: wcss: Add non pas wcss Q6 support for QCS404")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>   drivers/remoteproc/qcom_q6v5_wcss.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
> index bb0947f7770e..017ee225dc32 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -827,6 +827,8 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
>   	int ret;
>   
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qdsp6");
> +	if (!res)
> +		return -EINVAL;

LGTM.

Reviewed-by:Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh

>   	wcss->reg_base = devm_ioremap(&pdev->dev, res->start,
>   				      resource_size(res));
>   	if (!wcss->reg_base)

Re: [PATCH -next 1/3] remoteproc: qcom: wcss: check return value after calling platform_get_resource_byname()

[Yang Yingliang]

Hi,
Are these patches good for merge or any more suggestion ?

Thanks,
Yang
On 2022/8/26 13:33, Mukesh Ojha wrote:
>
>
> On 8/26/2022 7:15 AM, Yang Yingliang wrote:
>> If platform_get_resource_byname() fails, 'res' will be set to null 
>> pointer,
>> it will cause null-ptr-deref when it used in devm_ioremap(), so we 
>> need check
>> the return value.
>>
>> Fixes: 0af65b9b915e ("remoteproc: qcom: wcss: Add non pas wcss Q6 
>> support for QCS404")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/remoteproc/qcom_q6v5_wcss.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c 
>> b/drivers/remoteproc/qcom_q6v5_wcss.c
>> index bb0947f7770e..017ee225dc32 100644
>> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
>> @@ -827,6 +827,8 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss 
>> *wcss,
>>       int ret;
>>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
>> "qdsp6");
>> +    if (!res)
>> +        return -EINVAL;
>
> LGTM.
>
> Reviewed-by:Mukesh Ojha <quic_mojha@quicinc.com>
>
> -Mukesh
>
>>       wcss->reg_base = devm_ioremap(&pdev->dev, res->start,
>>                         resource_size(res));
>>       if (!wcss->reg_base)
> .