[PATCH 0/2] remoteproc: qcom: Fix handling of WCNSS IRQs

[PATCH 0/2] remoteproc: qcom: Fix handling of WCNSS IRQs

[Sireesh Kodali]

Hi,

Due to a bug in the code that fetches the IRQs from the device tree, the
optional "stop-ack" IRQ was never used, even if it was defined in the
device tree. This also allowed a typo in the MSM8916 SoC device tree,
where the "stop-ack" irq was instead labeled "stop".

The attached patches fix the handling of IRQs in the code, and fix the
typo in the MSM8916 device tree.

Sireesh Kodali (2):
  remoteproc: qcom: wcnss: Fix handling of IRQs
  arm64: dts: qcom: msm8916: Fix typo in pronto remoteproc node

 arch/arm64/boot/dts/qcom/msm8916.dtsi |  4 ++--
 drivers/remoteproc/qcom_wcnss.c       | 10 ++++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.36.0

[PATCH 1/2] remoteproc: qcom: wcnss: Fix handling of IRQs

[Sireesh Kodali]

The wcnss_get_irq function is expected to return a value > 0 in the
event that an IRQ is succssfully obtained, but it instead returns 0.
This causes the stop and ready IRQs to never actually be used despite
being defined in the device-tree. This patch fixes that.

Fixes: aed361adca9f ("remoteproc: qcom: Introduce WCNSS peripheral image loader")

Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>
---
 drivers/remoteproc/qcom_wcnss.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index 9a223d394087..68f37296b151 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -467,6 +467,7 @@ static int wcnss_request_irq(struct qcom_wcnss *wcnss,
 			     irq_handler_t thread_fn)
 {
 	int ret;
+	int irq_number;
 
 	ret = platform_get_irq_byname(pdev, name);
 	if (ret < 0 && optional) {
@@ -477,14 +478,19 @@ static int wcnss_request_irq(struct qcom_wcnss *wcnss,
 		return ret;
 	}
 
+	irq_number = ret;
+
 	ret = devm_request_threaded_irq(&pdev->dev, ret,
 					NULL, thread_fn,
 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 					"wcnss", wcnss);
-	if (ret)
+	if (ret) {
 		dev_err(&pdev->dev, "request %s IRQ failed\n", name);
+		return ret;
+	}
 
-	return ret;
+	/* Return the IRQ number if the IRQ was successfully acquired */
+	return irq_number;
 }
 
 static int wcnss_alloc_memory_region(struct qcom_wcnss *wcnss)
-- 
2.36.0

[PATCH 2/2] arm64: dts: qcom: msm8916: Fix typo in pronto remoteproc node

[Sireesh Kodali]

The smem-state properties for the pronto node were incorrectly labelled,
reading `qcom,state*` rather than `qcom,smem-state*`. Fix that, allowing
the stop state to be used.

Fixes: 88106096cbf8 ("ARM: dts: msm8916: Add and enable wcnss node")

Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index e34963505e07..7ecd747dc624 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1758,8 +1758,8 @@ pronto: remoteproc@a21b000 {
 					<&rpmpd MSM8916_VDDMX>;
 			power-domain-names = "cx", "mx";
 
-			qcom,state = <&wcnss_smp2p_out 0>;
-			qcom,state-names = "stop";
+			qcom,smem-states = <&wcnss_smp2p_out 0>;
+			qcom,smem-state-names = "stop";
 
 			pinctrl-names = "default";
 			pinctrl-0 = <&wcnss_pin_a>;
-- 
2.36.0

Re: [PATCH 2/2] arm64: dts: qcom: msm8916: Fix typo in pronto remoteproc node

[Stephan Gerhold]

On Tue, May 10, 2022 at 09:56:54AM +0530, Sireesh Kodali wrote:
> The smem-state properties for the pronto node were incorrectly labelled,
> reading `qcom,state*` rather than `qcom,smem-state*`. Fix that, allowing
> the stop state to be used.
> 
> Fixes: 88106096cbf8 ("ARM: dts: msm8916: Add and enable wcnss node")
> 
> Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>

Thanks for finding this!

Reviewed-by: Stephan Gerhold <stephan@gerhold.net>

> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index e34963505e07..7ecd747dc624 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1758,8 +1758,8 @@ pronto: remoteproc@a21b000 {
>  					<&rpmpd MSM8916_VDDMX>;
>  			power-domain-names = "cx", "mx";
>  
> -			qcom,state = <&wcnss_smp2p_out 0>;
> -			qcom,state-names = "stop";
> +			qcom,smem-states = <&wcnss_smp2p_out 0>;
> +			qcom,smem-state-names = "stop";
>  
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&wcnss_pin_a>;
> -- 
> 2.36.0
> 

Re: [PATCH 2/2] arm64: dts: qcom: msm8916: Fix typo in pronto remoteproc node

[Krzysztof Kozlowski]

On 10/05/2022 06:26, Sireesh Kodali wrote:
> The smem-state properties for the pronto node were incorrectly labelled,
> reading `qcom,state*` rather than `qcom,smem-state*`. Fix that, allowing
> the stop state to be used.
> 
> Fixes: 88106096cbf8 ("ARM: dts: msm8916: Add and enable wcnss node")
> 
> Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>

No blank lines between tags, please.

Best regards,
Krzysztof

Re: [PATCH 2/2] arm64: dts: qcom: msm8916: Fix typo in pronto remoteproc node

[Sireesh Kodali]

On Wed May 11, 2022 at 11:41 PM IST, Krzysztof Kozlowski wrote:
> On 10/05/2022 06:26, Sireesh Kodali wrote:
> > The smem-state properties for the pronto node were incorrectly labelled,
> > reading `qcom,state*` rather than `qcom,smem-state*`. Fix that, allowing
> > the stop state to be used.
> > 
> > Fixes: 88106096cbf8 ("ARM: dts: msm8916: Add and enable wcnss node")
> > 
> > Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>
>
> No blank lines between tags, please.
>
> Best regards,
> Krzysztof

Will fix in v2

Thanks,
Sireesh