[PATCH 0/3] ufs: qcom: Mark "core_reset" as required property

[PATCH 0/3] ufs: qcom: Mark "core_reset" as required property

[Manivannan Sadhasivam]

As concluded in [1], mark the "core_reset" (UFS host controller reset from GCC)
as required in the binding and also add the missing property in MSM8996 DT.

However, to keep backwards compatibility with old DT, the driver cannot make it
required. So it is still an optional property for the driver, but a comment has
been added to clarify this.

- Mani

[1] https://lore.kernel.org/linux-scsi/190651ad-6aeb-69eb-89c5-ed18221b5a7a@quicinc.com/

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Manivannan Sadhasivam (3):
      dt-bindings: ufs: qcom: Make reset properties as required
      scsi: ufs: qcom: Clarify the comment of core_reset property
      arm64: dts: qcom: msm8996: Add missing UFS host controller reset

 Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 2 ++
 arch/arm64/boot/dts/qcom/msm8996.dtsi               | 2 ++
 drivers/ufs/host/ufs-qcom.c                         | 6 +++++-
 3 files changed, 9 insertions(+), 1 deletion(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240129-ufs-core-reset-fix-1bd4ab266f64

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

[PATCH 1/3] dt-bindings: ufs: qcom: Make reset properties as required

[Manivannan Sadhasivam]

Apart from the legacy UFS controllers that were not supported in upstream,
rest of the controllers do require reset property to reset the UFS host
controller. So mark them as required.

Even though this is an ABI break, the bindings should reflect the
capabilities of the hardware.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 10c146424baa..03dce5e402d1 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -108,6 +108,8 @@ properties:
 required:
   - compatible
   - reg
+  - resets
+  - reset-names
 
 allOf:
   - $ref: ufs-common.yaml

-- 
2.25.1

Re: [PATCH 1/3] dt-bindings: ufs: qcom: Make reset properties as required

[Rob Herring]

On Mon, 29 Jan 2024 13:22:04 +0530, Manivannan Sadhasivam wrote:
> Apart from the legacy UFS controllers that were not supported in upstream,
> rest of the controllers do require reset property to reset the UFS host
> controller. So mark them as required.
> 
> Even though this is an ABI break, the bindings should reflect the
> capabilities of the hardware.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/3] scsi: ufs: qcom: Clarify the comment of core_reset property

[Manivannan Sadhasivam]

core_reset is not an optional property for the platforms supported in
upstream. Only for the non-upstreamed legacy platforms it is optional.
But somehow a few of the upstreamed platforms do not pass this property
by mistake.

So clarify the comment to make it clear that even though core_reset is
required, it is kept as optional to support the DTs that do not pass this
property.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/ufs-qcom.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 39eef470f8fa..32760506dfeb 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba)
 	host->hba = hba;
 	ufshcd_set_variant(hba, host);
 
-	/* Setup the optional reset control of HCI */
+	/*
+	 * Even though core_reset is required on all platforms, some DTs never
+	 * passed this property. So we have to keep it optional for supporting
+	 * them.
+	 */
 	host->core_reset = devm_reset_control_get_optional(hba->dev, "rst");
 	if (IS_ERR(host->core_reset)) {
 		err = dev_err_probe(dev, PTR_ERR(host->core_reset),

-- 
2.25.1

Re: [PATCH 2/3] scsi: ufs: qcom: Clarify the comment of core_reset property

[Andrew Halaney]

On Mon, Jan 29, 2024 at 01:22:05PM +0530, Manivannan Sadhasivam wrote:
> core_reset is not an optional property for the platforms supported in
> upstream. Only for the non-upstreamed legacy platforms it is optional.
> But somehow a few of the upstreamed platforms do not pass this property
> by mistake.
> 
> So clarify the comment to make it clear that even though core_reset is
> required, it is kept as optional to support the DTs that do not pass this
> property.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/ufs/host/ufs-qcom.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 39eef470f8fa..32760506dfeb 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>  	host->hba = hba;
>  	ufshcd_set_variant(hba, host);
>  
> -	/* Setup the optional reset control of HCI */
> +	/*
> +	 * Even though core_reset is required on all platforms, some DTs never
> +	 * passed this property. So we have to keep it optional for supporting
> +	 * them.
> +	 */

Any desire to print a warning if !host->core_reset? I'll defer to
Qualcomm to review since they can confirm the accuracy past Can's
comment, but this looks good to me for what its worth.

>  	host->core_reset = devm_reset_control_get_optional(hba->dev, "rst");
>  	if (IS_ERR(host->core_reset)) {
>  		err = dev_err_probe(dev, PTR_ERR(host->core_reset),
> 
> -- 
> 2.25.1
> 
> 

Re: [PATCH 2/3] scsi: ufs: qcom: Clarify the comment of core_reset property

[Manivannan Sadhasivam]

On Mon, Jan 29, 2024 at 02:57:20PM -0600, Andrew Halaney wrote:
> On Mon, Jan 29, 2024 at 01:22:05PM +0530, Manivannan Sadhasivam wrote:
> > core_reset is not an optional property for the platforms supported in
> > upstream. Only for the non-upstreamed legacy platforms it is optional.
> > But somehow a few of the upstreamed platforms do not pass this property
> > by mistake.
> > 
> > So clarify the comment to make it clear that even though core_reset is
> > required, it is kept as optional to support the DTs that do not pass this
> > property.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/ufs/host/ufs-qcom.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 39eef470f8fa..32760506dfeb 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> >  	host->hba = hba;
> >  	ufshcd_set_variant(hba, host);
> >  
> > -	/* Setup the optional reset control of HCI */
> > +	/*
> > +	 * Even though core_reset is required on all platforms, some DTs never
> > +	 * passed this property. So we have to keep it optional for supporting
> > +	 * them.
> > +	 */
> 
> Any desire to print a warning if !host->core_reset? I'll defer to
> Qualcomm to review since they can confirm the accuracy past Can's
> comment, but this looks good to me for what its worth.
> 

My only worry is that the existing users of the legacy DTs will get annoyed by
the warning. And I'm not sure if we can do that.

- Mani

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

[PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Manivannan Sadhasivam]

UFS host controller reset is required for the drivers to properly reset the
controller. Hence, add it.

Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 8d41ed261adf..a6592b3e2db7 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -2114,6 +2114,8 @@ ufshc: ufshc@624000 {
 					<&bimc MASTER_AMPSS_M0 &cnoc SLAVE_UFS_CFG>;
 			interconnect-names = "ufs-ddr", "cpu-ufs";
 
+			resets = <&gcc GCC_UFS_BCR>;
+			reset-names = "rst";
 			lanes-per-direction = <1>;
 			#reset-cells = <1>;
 			status = "disabled";

-- 
2.25.1

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Dmitry Baryshkov]

On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> UFS host controller reset is required for the drivers to properly reset the
> controller. Hence, add it.
>
> Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

I think I had issues previously when I attempted to reset the
controller, but it might be because of the incomplete clocks
programming. Let met check whether it works first.

> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 8d41ed261adf..a6592b3e2db7 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -2114,6 +2114,8 @@ ufshc: ufshc@624000 {
>                                         <&bimc MASTER_AMPSS_M0 &cnoc SLAVE_UFS_CFG>;
>                         interconnect-names = "ufs-ddr", "cpu-ufs";
>
> +                       resets = <&gcc GCC_UFS_BCR>;
> +                       reset-names = "rst";
>                         lanes-per-direction = <1>;
>                         #reset-cells = <1>;
>                         status = "disabled";
>
> --
> 2.25.1
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Manivannan Sadhasivam]

On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > UFS host controller reset is required for the drivers to properly reset the
> > controller. Hence, add it.
> >
> > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> I think I had issues previously when I attempted to reset the
> controller, but it might be because of the incomplete clocks
> programming. Let met check whether it works first.
> 

Sure. Please let me know.

- Mani

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

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Dmitry Baryshkov]

On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > UFS host controller reset is required for the drivers to properly reset the
> > > controller. Hence, add it.
> > >
> > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > I think I had issues previously when I attempted to reset the
> > controller, but it might be because of the incomplete clocks
> > programming. Let met check whether it works first.
> >
>
> Sure. Please let me know.

With the clocking fixes in place (I'll send them in a few minutes) and
with this patch the UFS breaks in the following way:


[    4.397919] scsi host0: ufshcd
[    4.413964] qcom-qmp-ufs-phy 627000.phy: phy initialization timed-out
[    4.415776] phy phy-627000.phy.3: phy poweron failed --> -110
[    4.419511] ufshcd-qcom 624000.ufshc: ufs_qcom_power_up_sequence:
phy power on failed, ret = -110
[    4.431297] mmc0: SDHCI controller on 74a4900.mmc [74a4900.mmc]
using ADMA 64-bit
[    4.484677] ufshcd-qcom 624000.ufshc: Controller enable failed
[    4.499710] qcom-qmp-ufs-phy 627000.phy: phy initialization timed-out
[    4.501507] phy phy-627000.phy.3: phy poweron failed --> -110
[    4.505219] ufshcd-qcom 624000.ufshc: ufs_qcom_power_up_sequence:
phy power on failed, ret = -110
[    4.570505] ufshcd-qcom 624000.ufshc: Controller enable failed
[    4.585232] qcom-qmp-ufs-phy 627000.phy: phy initialization timed-out
[    4.587296] phy phy-627000.phy.3: phy poweron failed --> -110
[    4.590733] ufshcd-qcom 624000.ufshc: ufs_qcom_power_up_sequence:
phy power on failed, ret = -110
[    4.598942] mmc0: new ultra high speed SDR104 SDHC card at address 5048
[    4.606103] mmcblk0: mmc0:5048 SD32G 28.8 GiB
[    4.615871]  mmcblk0: p1
[    4.656006] ufshcd-qcom 624000.ufshc: Controller enable failed
[    4.670985] qcom-qmp-ufs-phy 627000.phy: phy initialization timed-out
[    4.672795] phy phy-627000.phy.3: phy poweron failed --> -110
[    4.676493] ufshcd-qcom 624000.ufshc: ufs_qcom_power_up_sequence:
phy power on failed, ret = -110
[    4.741859] ufshcd-qcom 624000.ufshc: Controller enable failed
[    4.741942] ufshcd-qcom 624000.ufshc: Host controller enable failed
[    4.746717] host_regs: 00000000: 0107001f 00000000 00010100 00000000
[    4.752822] host_regs: 00000010: 01000000 00010217 00000000 00000000
[    4.759424] host_regs: 00000020: 00000000 00000000 00000000 00000000
[    4.765751] host_regs: 00000030: 00000000 00000000 00000000 00000000
[    4.772090] host_regs: 00000040: 00000000 00000000 00000000 00000000
[    4.778438] host_regs: 00000050: 00000000 00000000 00000000 00000000
[    4.784758] host_regs: 00000060: 00000000 00000000 00000000 00000000
[    4.791096] host_regs: 00000070: 00000000 00000000 00000000 00000000
[    4.797434] host_regs: 00000080: 00000000 00000000 00000000 00000000
[    4.803789] host_regs: 00000090: 00000000 00000001 00000000 00000000
[    4.810109] ufshcd-qcom 624000.ufshc: No record of pa_err
[    4.816439] ufshcd-qcom 624000.ufshc: No record of dl_err
[    4.821732] ufshcd-qcom 624000.ufshc: No record of nl_err
[    4.827112] ufshcd-qcom 624000.ufshc: No record of tl_err
[    4.832497] ufshcd-qcom 624000.ufshc: No record of dme_err
[    4.837879] ufshcd-qcom 624000.ufshc: No record of auto_hibern8_err
[    4.843263] ufshcd-qcom 624000.ufshc: No record of fatal_err
[    4.849424] ufshcd-qcom 624000.ufshc: No record of link_startup_fail
[    4.855332] ufshcd-qcom 624000.ufshc: No record of resume_fail
[    4.861670] ufshcd-qcom 624000.ufshc: No record of suspend_fail
[    4.867351] ufshcd-qcom 624000.ufshc: No record of wlun resume_fail
[    4.873132] ufshcd-qcom 624000.ufshc: No record of wlun suspend_fail
[    4.879375] ufshcd-qcom 624000.ufshc: No record of dev_reset
[    4.885971] ufshcd-qcom 624000.ufshc: No record of host_reset
[    4.891617] ufshcd-qcom 624000.ufshc: No record of task_abort
[    4.897273] HCI Vendor Specific Registers 00000000: 000000c8
00000000 00000000 00000000
[    4.903001] HCI Vendor Specific Registers 00000010: 00000000
00000000 00000000 1c00052e
[    4.910870] HCI Vendor Specific Registers 00000020: 3f011300
20020000 00000000 00000000
[    4.918823] HCI Vendor Specific Registers 00000030: 00000000
00000000 00000000 00000000
[    4.926831] UFS_UFS_DBG_RD_REG_OCSC 00000000: 00000000 00000000
00000000 00000000
[    4.934769] UFS_UFS_DBG_RD_REG_OCSC 00000010: 00000000 00000000
00000000 00000000
[    4.942430] UFS_UFS_DBG_RD_REG_OCSC 00000020: 00000000 00000000
00000000 00000000
[    4.949876] UFS_UFS_DBG_RD_REG_OCSC 00000030: 00000000 00000000
00000000 00000000
[    4.957338] UFS_UFS_DBG_RD_REG_OCSC 00000040: 00000000 00000000
00000000 00000000
[    4.964800] UFS_UFS_DBG_RD_REG_OCSC 00000050: 00000000 00000000
00000000 00000000
[    4.972270] UFS_UFS_DBG_RD_REG_OCSC 00000060: 00000000 00000000
00000000 00000000
[    4.979733] UFS_UFS_DBG_RD_REG_OCSC 00000070: 00000000 00000000
00000000 00000000
[    4.987215] UFS_UFS_DBG_RD_REG_OCSC 00000080: 00000000 00000000
00000000 00000000
[    4.994675] UFS_UFS_DBG_RD_REG_OCSC 00000090: 00000000 00000000
00000000 00000000
[    5.002128] UFS_UFS_DBG_RD_REG_OCSC 000000a0: 00000000 00000000
00000000 00000000
[    5.009615] UFS_UFS_DBG_RD_EDTL_RAM 00000000: 00000000 a4491248
fcf4caf8 44ff663f
[    5.017059] UFS_UFS_DBG_RD_EDTL_RAM 00000010: 3495a3e2 7be93e99
2334e629 a9f5c77a
[    5.024530] UFS_UFS_DBG_RD_EDTL_RAM 00000020: e0edb246 e451c5b7
d020df23 c84d85e6
[    5.032019] UFS_UFS_DBG_RD_EDTL_RAM 00000030: 59e307b2 3685dda2
3d4484ee 33b4d9d9
[    5.039460] UFS_UFS_DBG_RD_EDTL_RAM 00000040: 4de322b3 5ba15f50
18e13d42 ca9f93e5
[    5.046936] UFS_UFS_DBG_RD_EDTL_RAM 00000050: 4ce04e39 954c996e
0645064b e235d257
[    5.054385] UFS_UFS_DBG_RD_EDTL_RAM 00000060: b92c1acb 281dc88f
56ff1877 3307091a
[    5.061851] UFS_UFS_DBG_RD_EDTL_RAM 00000070: fc19350a 222e4061
d2cc6217 1fa596f9
[    5.069381] UFS_UFS_DBG_RD_DESC_RAM 00000000: 00000fff 000245b9
40000fff 000245d7
[    5.076783] UFS_UFS_DBG_RD_DESC_RAM 00000010: ed10dc68 0036fe67
353c512f 0038a521
[    5.084250] UFS_UFS_DBG_RD_DESC_RAM 00000020: 8c4f244a 003677cc
c67731d1 00145113
[    5.091712] UFS_UFS_DBG_RD_DESC_RAM 00000030: fc33c5b6 00357243
fce1d206 002dc715
[    5.099178] UFS_UFS_DBG_RD_DESC_RAM 00000040: c54267f1 0039fc1e
e7e564f7 001d3070
[    5.106659] UFS_UFS_DBG_RD_DESC_RAM 00000050: a0c0bff6 002a0367
4a31b6ca 0021a267
[    5.114108] UFS_UFS_DBG_RD_DESC_RAM 00000060: 085b1f23 002c64ef
73830a12 0010ff39
[    5.121586] UFS_UFS_DBG_RD_DESC_RAM 00000070: f902904f 001b0514
714edb47 002ef768
[    5.129041] UFS_UFS_DBG_RD_DESC_RAM 00000080: 369035fc 0010e7fe
17d94504 003895c4
[    5.136508] UFS_UFS_DBG_RD_DESC_RAM 00000090: 26dd0675 001c14dc
7503fb3d 00040c95
[    5.143996] UFS_UFS_DBG_RD_DESC_RAM 000000a0: 945456e2 003932f0
bf4a51ed 002c0765
[    5.151444] UFS_UFS_DBG_RD_DESC_RAM 000000b0: c15810a5 001a4fc3
f9137305 0020c227
[    5.158902] UFS_UFS_DBG_RD_DESC_RAM 000000c0: 65b22d16 003b9a18
63d89770 000ab592
[    5.166365] UFS_UFS_DBG_RD_DESC_RAM 000000d0: d6257375 0016de4d
1e047776 000ec615
[    5.173849] UFS_UFS_DBG_RD_DESC_RAM 000000e0: 23184caa 001c5498
14cd841d 000a2d46
[    5.181299] UFS_UFS_DBG_RD_DESC_RAM 000000f0: 9a229d09 00041b11
d2249690 000e4e1b
[    5.188761] UFS_UFS_DBG_RD_DESC_RAM 00000100: 1a4f6c46 000202d1
07159e46 0011276c
[    5.196227] UFS_UFS_DBG_RD_DESC_RAM 00000110: be679f01 00042f38
1da6fd6c 000c3556
[    5.203694] UFS_UFS_DBG_RD_DESC_RAM 00000120: 1641cfe1 000996fd
7c902c15 0005bd54
[    5.211158] UFS_UFS_DBG_RD_DESC_RAM 00000130: 1ff37280 002c4c5d
3bb1d9f0 00286f65
[    5.218624] UFS_UFS_DBG_RD_DESC_RAM 00000140: 9bbc09e5 0028c6e7
5f5d9dc6 0035d8dd
[    5.226089] UFS_UFS_DBG_RD_DESC_RAM 00000150: 4a1d5167 00036152
da64f552 001ac503
[    5.233572] UFS_UFS_DBG_RD_DESC_RAM 00000160: 8d429104 003cdc65
cb6eb64b 0031356b
[    5.241021] UFS_UFS_DBG_RD_DESC_RAM 00000170: 4438c55a 003dc021
7859ed93 003748ff
[    5.248497] UFS_UFS_DBG_RD_DESC_RAM 00000180: 581e7129 00249e0f
1119282b 002eea5e
[    5.255973] UFS_UFS_DBG_RD_DESC_RAM 00000190: 36c4e1ff 003696c5
3d26c8d6 00341834
[    5.263423] UFS_UFS_DBG_RD_DESC_RAM 000001a0: 9569e755 0038c181
44a04716 00113eee
[    5.270880] UFS_UFS_DBG_RD_DESC_RAM 000001b0: 2b8126c4 00070e4e
3c00b95f 003efa6c
[    5.278349] UFS_UFS_DBG_RD_DESC_RAM 000001c0: fdac7bc9 002bbd97
0d1b10d7 00279d0e
[    5.285814] UFS_UFS_DBG_RD_DESC_RAM 000001d0: fd26970f 003ea7dd
7cfa0dd9 001abe65
[    5.293300] UFS_UFS_DBG_RD_DESC_RAM 000001e0: 462260e8 00088e88
6664d825 0025aedb
[    5.300740] UFS_UFS_DBG_RD_DESC_RAM 000001f0: e6e33f2d 0010a59d
646f03d7 00042b16
[    5.308241] UFS_UFS_DBG_RD_PRDT_RAM 00000000: b6500000 0000916c
c5801d67 000a3550
[    5.315674] UFS_UFS_DBG_RD_PRDT_RAM 00000010: 7437814c 0005a357
e44607eb 00093259
[    5.323140] UFS_UFS_DBG_RD_PRDT_RAM 00000020: 08ff015c 00082c54
09070e95 000c2704
[    5.330602] UFS_UFS_DBG_RD_PRDT_RAM 00000030: 3663d4c0 000c5d89
fb34130e 0009e7ee
[    5.338069] UFS_UFS_DBG_RD_PRDT_RAM 00000040: c6c207f0 0002a5d5
50e088ab 0004be17
[    5.345534] UFS_UFS_DBG_RD_PRDT_RAM 00000050: e0754a0b 000c5ceb
c99e1281 000549b2
[    5.353015] UFS_UFS_DBG_RD_PRDT_RAM 00000060: 3bcb7410 0004cd30
71149cd2 000fc80b
[    5.360465] UFS_UFS_DBG_RD_PRDT_RAM 00000070: 4012a0d8 000e04a1
6dc3ae4d 00080c54
[    5.367951] UFS_UFS_DBG_RD_PRDT_RAM 00000080: 00000040 00000004
e910c009 0001a567
[    5.375408] UFS_UFS_DBG_RD_PRDT_RAM 00000090: 0050cd49 0004eaa0
813bf322 000dd259
[    5.382856] UFS_UFS_DBG_RD_PRDT_RAM 000000a0: c920cae2 000a55a1
41816101 000f8065
[    5.390323] UFS_UFS_DBG_RD_PRDT_RAM 000000b0: e819a0b7 00050746
f1310132 000ada10
[    5.397787] UFS_UFS_DBG_RD_PRDT_RAM 000000c0: 2e4801ec 000a3517
f5544b9f 00008308
[    5.405253] UFS_UFS_DBG_RD_PRDT_RAM 000000d0: f345c26d 00041935
b001d7cf 0004a2ff
[    5.412737] UFS_UFS_DBG_RD_PRDT_RAM 000000e0: 48140248 0005010e
6039f044 0008131e
[    5.420185] UFS_UFS_DBG_RD_PRDT_RAM 000000f0: 0c418401 00000000
70518e69 00053a4c
[    5.427657] UFS_DBG_RD_REG_UAWM 00000000: 00000000 00000000 00000000 0001fec0
[    5.435117] UFS_DBG_RD_REG_UARM 00000000: 00000000 00000000 00000001 00000001
[    5.442255] UFS_DBG_RD_REG_TXUC 00000000: 00000000 00000000 00000000 00000000
[    5.449349] UFS_DBG_RD_REG_TXUC 00000010: 00000000 00000000 00000000 00000000
[    5.456467] UFS_DBG_RD_REG_TXUC 00000020: 00000000 00000000 00000000 00000000
[    5.463585] UFS_DBG_RD_REG_TXUC 00000030: 00000000 00000000 00000000 00000000
[    5.470719] UFS_DBG_RD_REG_TXUC 00000040: 00000000 00000000 00000000 00000000
[    5.477825] UFS_DBG_RD_REG_TXUC 00000050: 00000000 00000000 00000000 00000000
[    5.484963] UFS_DBG_RD_REG_TXUC 00000060: 00000000 00000000 00000000 00000000
[    5.492076] UFS_DBG_RD_REG_TXUC 00000070: 00000000 00000000 00000000 00000000
[    5.499189] UFS_DBG_RD_REG_TXUC 00000080: 00000000 00000000 00000000 00000000
[    5.506294] UFS_DBG_RD_REG_TXUC 00000090: 00000000 00000000 00000000 00000000
[    5.513413] UFS_DBG_RD_REG_TXUC 000000a0: 00000000 00000000 00000000 00000000
[    5.520532] UFS_DBG_RD_REG_TXUC 000000b0: 00000001 00000040 00000000 00000004
[    5.527665] UFS_DBG_RD_REG_RXUC 00000000: 00000000 00000000 00000000 00000004
[    5.534781] UFS_DBG_RD_REG_RXUC 00000010: 00000000 00000000 00000000 00000000
[    5.541883] UFS_DBG_RD_REG_RXUC 00000020: 00000000 00000000 00000000 00000000
[    5.549005] UFS_DBG_RD_REG_RXUC 00000030: 00000000 00000000 00000000 00000000
[    5.556121] UFS_DBG_RD_REG_RXUC 00000040: 00000000 00000000 00000000 00000000
[    5.563239] UFS_DBG_RD_REG_RXUC 00000050: 00000000 00000000 00000000 00000001
[    5.570357] UFS_DBG_RD_REG_RXUC 00000060: 00000040 00000000 00000004
[    5.577481] UFS_DBG_RD_REG_DFC 00000000: 00000000 00000000 00000000 00000000
[    5.583897] UFS_DBG_RD_REG_DFC 00000010: 00000000 00000000 00000000 00000000
[    5.590931] UFS_DBG_RD_REG_DFC 00000020: 00000000 00000000 00000000 00000000
[    5.597999] UFS_DBG_RD_REG_DFC 00000030: 00000000 00000000 00000000 00000000
[    5.604998] UFS_DBG_RD_REG_DFC 00000040: ffffffff 00000000 00000000
[    5.612039] UFS_DBG_RD_REG_TRLUT 00000000: 00000000 00000000
00000000 00000000
[    5.618013] UFS_DBG_RD_REG_TRLUT 00000010: 00000000 00000000
00000000 00000000
[    5.625318] UFS_DBG_RD_REG_TRLUT 00000020: 00000000 00000000
00000000 00000000
[    5.632510] UFS_DBG_RD_REG_TRLUT 00000030: 00000000 00000000
00000000 00000000
[    5.639715] UFS_DBG_RD_REG_TRLUT 00000040: 00000000 00000000
00000000 00000000
[    5.646918] UFS_DBG_RD_REG_TRLUT 00000050: 00000000 00000000
00000000 00000000
[    5.654126] UFS_DBG_RD_REG_TRLUT 00000060: 00000000 00000000
00000000 00000000
[    5.661349] UFS_DBG_RD_REG_TRLUT 00000070: 00000000 00000000
00000000 00000000
[    5.668536] UFS_DBG_RD_REG_TRLUT 00000080: 00000000 00000000
[    5.675741] UFS_DBG_RD_REG_TMRLUT 00000000: 00000000 00000000
00000000 00000000
[    5.681552] UFS_DBG_RD_REG_TMRLUT 00000010: 00000000 00000000
00000000 00000000
[    5.688585] UFS_DBG_RD_REG_TMRLUT 00000020: 00000000
[    5.695866] ufshcd-qcom 624000.ufshc: UFS Host state=0
[    5.701079] ufshcd-qcom 624000.ufshc: outstanding reqs=0x0 tasks=0x0
[    5.706047] ufshcd-qcom 624000.ufshc: saved_err=0x0, saved_uic_err=0x0
[    5.712550] ufshcd-qcom 624000.ufshc: Device power mode=0, UIC link state=0
[    5.718895] ufshcd-qcom 624000.ufshc: PM in progress=0, sys. suspended=0
[    5.725734] ufshcd-qcom 624000.ufshc: Auto BKOPS=0, Host self-block=0
[    5.732677] ufshcd-qcom 624000.ufshc: Clk gate=1
[    5.739003] ufshcd-qcom 624000.ufshc: last_hibern8_exit_tstamp at 0
us, hibern8_exit_cnt=0
[    5.743712] ufshcd-qcom 624000.ufshc: last intr at 0 us, last intr status=0x0
[    5.751790] ufshcd-qcom 624000.ufshc: error handling flags=0x0,
req. abort count=0
[    5.758985] ufshcd-qcom 624000.ufshc: hba->ufs_version=0x200, Host
capabilities=0x107001f, caps=0x12cf
[    5.766477] ufshcd-qcom 624000.ufshc: quirks=0x20, dev. quirks=0x0
[    5.775729] ufshcd-qcom 624000.ufshc: clk: core_clk, rate: 200000000
[    5.781911] ufshcd-qcom 624000.ufshc: clk: core_clk_unipro, rate: 150000000
[    5.788408] ufshcd-qcom 624000.ufshc: clk: core_clk_ice, rate: 300000000
[    5.799734] ufshcd-qcom 624000.ufshc: error -EIO: Initialization
failed with error -5
[    5.823482] ufshcd-qcom 624000.ufshc: error -EIO: ufshcd_pltfrm_init() failed
[    5.823655] ufshcd-qcom: probe of 624000.ufshc failed with error -5

-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Bjorn Andersson]

On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > UFS host controller reset is required for the drivers to properly reset the
> > > > controller. Hence, add it.
> > > >
> > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > >
> > > I think I had issues previously when I attempted to reset the
> > > controller, but it might be because of the incomplete clocks
> > > programming. Let met check whether it works first.
> > >
> >
> > Sure. Please let me know.
> 
> With the clocking fixes in place (I'll send them in a few minutes) and
> with this patch the UFS breaks in the following way:
> 

Was this further reviewed/investigated? What would you like me to do
with this patch?

Regards,
Bjorn

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Dmitry Baryshkov]

On Tue, 2 Apr 2024 at 20:35, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> > On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > >
> > > > > UFS host controller reset is required for the drivers to properly reset the
> > > > > controller. Hence, add it.
> > > > >
> > > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > >
> > > > I think I had issues previously when I attempted to reset the
> > > > controller, but it might be because of the incomplete clocks
> > > > programming. Let met check whether it works first.
> > > >
> > >
> > > Sure. Please let me know.
> >
> > With the clocking fixes in place (I'll send them in a few minutes) and
> > with this patch the UFS breaks in the following way:
> >
>
> Was this further reviewed/investigated? What would you like me to do
> with this patch?

I'd say, hold until we can understand what is going on.

Mani, If you have any ideas what can be causing the issue, I'm open to
testing any ideas or any patches from your side.
Judging that the UFS breaks after toggling the reset, we might be
missing some setup steps.



-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset

[Manivannan Sadhasivam]

On Tue, Apr 02, 2024 at 10:25:32PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 20:35, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > > >
> > > > > > UFS host controller reset is required for the drivers to properly reset the
> > > > > > controller. Hence, add it.
> > > > > >
> > > > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > >
> > > > > I think I had issues previously when I attempted to reset the
> > > > > controller, but it might be because of the incomplete clocks
> > > > > programming. Let met check whether it works first.
> > > > >
> > > >
> > > > Sure. Please let me know.
> > >
> > > With the clocking fixes in place (I'll send them in a few minutes) and
> > > with this patch the UFS breaks in the following way:
> > >
> >
> > Was this further reviewed/investigated? What would you like me to do
> > with this patch?
> 
> I'd say, hold until we can understand what is going on.
> 
> Mani, If you have any ideas what can be causing the issue, I'm open to
> testing any ideas or any patches from your side.
> Judging that the UFS breaks after toggling the reset, we might be
> missing some setup steps.
> 

I've bombarded the Qcom UFS team with too many questions, but didn't get answer
for all of them. And this is one of those questions.

Let me try to dig further and come back.

- Mani

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