[PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Krzysztof Kozlowski]

SM8350 HDK and MTP boards were silently dying and rebooting during BAM
DMA probe:

  [    1.574304] vreg_bob: Setting 3008000-3960000uV
  [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
  Optional Info
  Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
  S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
  S - IMAGE_VARIANT_STRING=SocLahainaLAA
  S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
  S - Boot Interface: UFS

It seems that BAM DMA is locally controller (not by firmware) and
requires proper initialization by the driver prior to use, at least on
HDK8350 and MTP8350, but probably on all boards.

Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 88ef478cb5cc..b382ce66387e 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1741,7 +1741,6 @@ cryptobam: dma-controller@1dc4000 {
 			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
 			#dma-cells = <1>;
 			qcom,ee = <0>;
-			qcom,controlled-remotely;
 			iommus = <&apps_smmu 0x594 0x0011>,
 				 <&apps_smmu 0x596 0x0011>;
 		};
-- 
2.34.1

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Krzysztof Kozlowski]

On 21/06/2023 16:36, Krzysztof Kozlowski wrote:
> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
> DMA probe:
> 
>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>   Optional Info
>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>   S - Boot Interface: UFS
> 
> It seems that BAM DMA is locally controller (not by firmware) and
> requires proper initialization by the driver prior to use, at least on
> HDK8350 and MTP8350, but probably on all boards.
> 
> Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

#regzbot fix: f1040a7fe8f069d2259ab3dab9190210005ceb33

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Konrad Dybcio]

On 21.06.2023 16:36, Krzysztof Kozlowski wrote:
> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
> DMA probe:
> 
>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>   Optional Info
>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>   S - Boot Interface: UFS
> 
> It seems that BAM DMA is locally controller (not by firmware) and
> requires proper initialization by the driver prior to use, at least on
> HDK8350 and MTP8350, but probably on all boards.
> 
> Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Tested-by: Konrad Dybcio <konrad.dybcio@linaro.org> # SM8350 PDX215

Konrad
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 88ef478cb5cc..b382ce66387e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1741,7 +1741,6 @@ cryptobam: dma-controller@1dc4000 {
>  			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
>  			#dma-cells = <1>;
>  			qcom,ee = <0>;
> -			qcom,controlled-remotely;
>  			iommus = <&apps_smmu 0x594 0x0011>,
>  				 <&apps_smmu 0x596 0x0011>;
>  		};

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Manivannan Sadhasivam]

On Wed, Jun 21, 2023 at 04:36:27PM +0200, Krzysztof Kozlowski wrote:
> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
> DMA probe:
> 
>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>   Optional Info
>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>   S - Boot Interface: UFS
> 
> It seems that BAM DMA is locally controller (not by firmware) and
> requires proper initialization by the driver prior to use, at least on
> HDK8350 and MTP8350, but probably on all boards.
> 
> Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> 
> ---
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 88ef478cb5cc..b382ce66387e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1741,7 +1741,6 @@ cryptobam: dma-controller@1dc4000 {
>  			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
>  			#dma-cells = <1>;
>  			qcom,ee = <0>;
> -			qcom,controlled-remotely;
>  			iommus = <&apps_smmu 0x594 0x0011>,
>  				 <&apps_smmu 0x596 0x0011>;
>  		};
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Thorsten Leemhuis]

On 21.06.23 16:37, Krzysztof Kozlowski wrote:
> On 21/06/2023 16:36, Krzysztof Kozlowski wrote:
>> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
>> DMA probe:
>>
>>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>>   Optional Info
>>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>>   S - Boot Interface: UFS
>>
>> It seems that BAM DMA is locally controller (not by firmware) and
>> requires proper initialization by the driver prior to use, at least on
>> HDK8350 and MTP8350, but probably on all boards.
>>
>> Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")

BTW, did this make any progress? It's not yet in -next afaics; might be
nice to have this merged so it goes to Linus together with f1040a7fe8f0.

>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Many thx for fixing and involving regzbot. FWIW, this...

> #regzbot fix: f1040a7fe8f069d2259ab3dab9190210005ceb33

...didn't work, as you got things backwards: The fix command is used in
reply to a report to specify which commit fixes it. I'll take care of
fixing that (and also mediate about the docs, maybe this
misunderstanding can be avoided by improving things).

Side note: using a

 Link:
https://lore.kernel.org/lkml/d239ad07-fbdd-16fa-3555-5bcf33c67059@linaro.org/

(or Closes:) in the patch description would have been the ideal way for
regzbot here. Those Links are also what Linus wants to see so everyone
can when needed understand the backstory (now or in the future), which
is why regzbot relies on them.

Whatever, not all that important. Thx again for giving regzbot a try.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Luca Weiss]

Hi Krzysztof,

On Wed Jun 21, 2023 at 4:36 PM CEST, Krzysztof Kozlowski wrote:
> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
> DMA probe:
>
>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>   Optional Info
>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>   S - Boot Interface: UFS
>
> It seems that BAM DMA is locally controller (not by firmware) and
> requires proper initialization by the driver prior to use, at least on
> HDK8350 and MTP8350, but probably on all boards.

Are you sure that the bam (and subsequent the qce) actually probes with
this change? From reading the code I don't see how the bam should probe
without either qcom,controlled-remotely or qcom,powered-remotely but no
clocks supplied. I think the probe just fails with this change, right?

Regards
Luca

>
> Fixes: f1040a7fe8f0 ("arm64: dts: qcom: sm8350: Add Crypto Engine support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 88ef478cb5cc..b382ce66387e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1741,7 +1741,6 @@ cryptobam: dma-controller@1dc4000 {
>  			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
>  			#dma-cells = <1>;
>  			qcom,ee = <0>;
> -			qcom,controlled-remotely;
>  			iommus = <&apps_smmu 0x594 0x0011>,
>  				 <&apps_smmu 0x596 0x0011>;
>  		};

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Krzysztof Kozlowski]

On 26/06/2023 13:32, Luca Weiss wrote:
> Hi Krzysztof,
> 
> On Wed Jun 21, 2023 at 4:36 PM CEST, Krzysztof Kozlowski wrote:
>> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
>> DMA probe:
>>
>>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>>   Optional Info
>>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>>   S - Boot Interface: UFS
>>
>> It seems that BAM DMA is locally controller (not by firmware) and
>> requires proper initialization by the driver prior to use, at least on
>> HDK8350 and MTP8350, but probably on all boards.
> 
> Are you sure that the bam (and subsequent the qce) actually probes with
> this change? From reading the code I don't see how the bam should probe
> without either qcom,controlled-remotely or qcom,powered-remotely but no

Why the binding does not require either this or that? Eh, buggy stuff...


> clocks supplied. I think the probe just fails with this change, right?

I will need to double check. I was happy enough to be able to boot my
device instead of having crashes, but indeed it would be nice to fix it
fully.


Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sm8350: fix BAM DMA crash and reboot

[Krzysztof Kozlowski]

On 26/06/2023 14:46, Krzysztof Kozlowski wrote:
> On 26/06/2023 13:32, Luca Weiss wrote:
>> Hi Krzysztof,
>>
>> On Wed Jun 21, 2023 at 4:36 PM CEST, Krzysztof Kozlowski wrote:
>>> SM8350 HDK and MTP boards were silently dying and rebooting during BAM
>>> DMA probe:
>>>
>>>   [    1.574304] vreg_bob: Setting 3008000-3960000uV
>>>   [    1.576918] bam-dFormat: Log Type - Time(microsec) - Message -
>>>   Optional Info
>>>   Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>>   S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.0-00637.1-LAHAINA-1
>>>   S - IMAGE_VARIANT_STRING=SocLahainaLAA
>>>   S - OEM_IMAGE_VERSION_STRING=crm-ubuntu77
>>>   S - Boot Interface: UFS
>>>
>>> It seems that BAM DMA is locally controller (not by firmware) and
>>> requires proper initialization by the driver prior to use, at least on
>>> HDK8350 and MTP8350, but probably on all boards.
>>
>> Are you sure that the bam (and subsequent the qce) actually probes with
>> this change? From reading the code I don't see how the bam should probe
>> without either qcom,controlled-remotely or qcom,powered-remotely but no
> 
> Why the binding does not require either this or that? Eh, buggy stuff...
> 
> 
>> clocks supplied. I think the probe just fails with this change, right?
> 
> I will need to double check. I was happy enough to be able to boot my
> device instead of having crashes, but indeed it would be nice to fix it
> fully.

You were right and my patch is not correct. I checked downstream and
there are no clocks there. Therefore I assume something else is wrong here.

I will send v2 disabling this.

Best regards,
Krzysztof