2024-05-16 23:00:28

by Chris Lew

[permalink] [raw]
Subject: [PATCH 7/7] arm64: dts: qcom: sm8650: Add hwlock to remoteproc

Add the hwlock property to remoteproc. This enables the remoteproc to
try and bust the smem hwspinlock if the remoteproc has crashed while
holding the hwspinlock.

Signed-off-by: Chris Lew <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 62a6e77730bc..a65a1679f003 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2878,6 +2878,7 @@ remoteproc_mpss: remoteproc@4080000 {

qcom,smem-states = <&smp2p_modem_out 0>;
qcom,smem-state-names = "stop";
+ hwlocks = <&tcsr_mutex 3>;

status = "disabled";

@@ -5024,6 +5025,7 @@ remoteproc_adsp: remoteproc@30000000 {

qcom,smem-states = <&smp2p_adsp_out 0>;
qcom,smem-state-names = "stop";
+ hwlocks = <&tcsr_mutex 3>;

status = "disabled";

@@ -5183,6 +5185,7 @@ remoteproc_cdsp: remoteproc@32300000 {

qcom,smem-states = <&smp2p_cdsp_out 0>;
qcom,smem-state-names = "stop";
+ hwlocks = <&tcsr_mutex 3>;

status = "disabled";


--
2.25.1



2024-05-22 07:27:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 7/7] arm64: dts: qcom: sm8650: Add hwlock to remoteproc

On 17/05/2024 00:58, Chris Lew wrote:
> Add the hwlock property to remoteproc. This enables the remoteproc to
> try and bust the smem hwspinlock if the remoteproc has crashed while
> holding the hwspinlock.
>
> Signed-off-by: Chris Lew <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 62a6e77730bc..a65a1679f003 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -2878,6 +2878,7 @@ remoteproc_mpss: remoteproc@4080000 {
>
> qcom,smem-states = <&smp2p_modem_out 0>;
> qcom,smem-state-names = "stop";
> + hwlocks = <&tcsr_mutex 3>;

lock #3 is used by smem, so this proves you are taking someone else's
lock. I commented on this in the binding, but let's be specific:

NAK, please carry:

Nacked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2024-05-22 17:52:45

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 7/7] arm64: dts: qcom: sm8650: Add hwlock to remoteproc

On Wed, May 22, 2024 at 09:27:29AM +0200, Krzysztof Kozlowski wrote:
> On 17/05/2024 00:58, Chris Lew wrote:
> > Add the hwlock property to remoteproc. This enables the remoteproc to
> > try and bust the smem hwspinlock if the remoteproc has crashed while
> > holding the hwspinlock.
> >
> > Signed-off-by: Chris Lew <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> > index 62a6e77730bc..a65a1679f003 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> > @@ -2878,6 +2878,7 @@ remoteproc_mpss: remoteproc@4080000 {
> >
> > qcom,smem-states = <&smp2p_modem_out 0>;
> > qcom,smem-state-names = "stop";
> > + hwlocks = <&tcsr_mutex 3>;
>
> lock #3 is used by smem, so this proves you are taking someone else's
> lock.

The lock is a shared resource, it's not "some else's lock".

Regards,
Bjorn

> I commented on this in the binding, but let's be specific:
>
> NAK, please carry:
>
> Nacked-by: Krzysztof Kozlowski <[email protected]>
>
> Best regards,
> Krzysztof
>