2019-02-04 17:43:53

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 0/5] UFS on APQ8098/MSM8998

Hello,

This series is now good enough for the limelight, IMO.

Difference between, v2 and v3:
- Add qcom,msm8998-qmp-ufs-phy compat string and match it in the PHY driver
- Drop vdd-hba-fixed-regulator
- Write the reg addresses with full 32-bit width
- Set regulator-allow-set-load only on the 3 rails used by UFS.
- Revert the patch introducing ufshcd_set_vccq_rail_unused

Difference between v1 and v2:
- New patch to add 'regulator-allow-set-load' prop to all vreg nodes
- Rename rpmcc node to 'clock-controller' + Add Review tags
- Drop UFS pinctrl gymnastics (not required, probably left enabled in bootloader)
- Delete GCC_UFS_ICE_CORE_CLK (ICE not used upstream, I think)
- Fix sizes of ufsphy register areas based on Jeffrey's feedback
- Hack ufshcd_set_vccq_rail_unused into a NOP to work around lock up + reboot

Marc Gonzalez (5):
arm64: dts: qcom: msm8998: Add UFS nodes
arm64: dts: qcom: msm8998: Allow drivers to set-load
dt-bindings: phy-qcom-qmp: Add qcom,msm8998-qmp-ufs-phy
phy: qcom-qmp: Add QMP UFS PHY support for msm8998
Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

.../devicetree/bindings/phy/qcom-qmp-phy.txt | 1 +
arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 22 +++++++
arch/arm64/boot/dts/qcom/msm8998.dtsi | 63 +++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-qmp.c | 3 +
drivers/scsi/ufs/ufs.h | 1 -
drivers/scsi/ufs/ufshcd.c | 59 ++---------------
6 files changed, 93 insertions(+), 56 deletions(-)

--
2.17.1


2019-02-04 17:44:37

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 1/5] arm64: dts: qcom: msm8998: Add UFS nodes

Add host controller and PHY DT nodes.

Signed-off-by: Marc Gonzalez <[email protected]>
---
vddp-ref-clk-max-microamp = <100>; sounds tiny and fishy.
Jeffrey, Bjorn, can you check?
The PHY driver doesn't seem to try to set any load...
---
arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 19 +++++++
arch/arm64/boot/dts/qcom/msm8998.dtsi | 63 +++++++++++++++++++++++
2 files changed, 82 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index 50e9033aa7f6..80075283847a 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -257,3 +257,22 @@
pinctrl-0 = <&sdc2_clk_on &sdc2_cmd_on &sdc2_data_on &sdc2_cd_on>;
pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
};
+
+&ufshc {
+ vcc-supply = <&vreg_l20a_2p95>;
+ vccq-supply = <&vreg_l26a_1p2>;
+ vccq2-supply = <&vreg_s4a_1p8>;
+ vcc-max-microamp = <750000>;
+ vccq-max-microamp = <560000>;
+ vccq2-max-microamp = <750000>;
+};
+
+&ufsphy {
+ vdda-phy-supply = <&vreg_l1a_0p875>;
+ vdda-pll-supply = <&vreg_l2a_1p2>;
+ vddp-ref-clk-supply = <&vreg_l26a_1p2>;
+ vdda-phy-max-microamp = <51400>;
+ vdda-pll-max-microamp = <14600>;
+ vddp-ref-clk-max-microamp = <100>;
+ vddp-ref-clk-always-on;
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 6f4f4b79853b..831af20143da 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -711,6 +711,69 @@
redistributor-stride = <0x0 0x20000>;
interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ ufshc: ufshc@1da4000 {
+ compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
+ "jedec,ufs-2.0";
+ reg = <0x01da4000 0x2500>;
+ interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&ufsphy_lanes>;
+ phy-names = "ufsphy";
+ lanes-per-direction = <2>;
+ power-domains = <&gcc UFS_GDSC>;
+
+ clock-names =
+ "core_clk",
+ "bus_aggr_clk",
+ "iface_clk",
+ "core_clk_unipro",
+ "ref_clk",
+ "tx_lane0_sync_clk",
+ "rx_lane0_sync_clk",
+ "rx_lane1_sync_clk";
+ clocks =
+ <&gcc GCC_UFS_AXI_CLK>,
+ <&gcc GCC_AGGRE1_UFS_AXI_CLK>,
+ <&gcc GCC_UFS_AHB_CLK>,
+ <&gcc GCC_UFS_UNIPRO_CORE_CLK>,
+ <&rpmcc RPM_SMD_LN_BB_CLK1>,
+ <&gcc GCC_UFS_TX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_RX_SYMBOL_0_CLK>,
+ <&gcc GCC_UFS_RX_SYMBOL_1_CLK>;
+ freq-table-hz =
+ <50000000 200000000>,
+ <0 0>,
+ <0 0>,
+ <37500000 150000000>,
+ <0 0>,
+ <0 0>,
+ <0 0>,
+ <0 0>;
+
+ resets = <&gcc GCC_UFS_BCR>;
+ reset-names = "rst";
+ };
+
+ ufsphy: phy@1da7000 {
+ compatible = "qcom,msm8998-qmp-ufs-phy";
+ reg = <0x01da7000 0x18c>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ clock-names = "ref", "ref_aux";
+ clocks =
+ <&gcc GCC_UFS_CLKREF_CLK>,
+ <&gcc GCC_UFS_PHY_AUX_CLK>;
+
+ ufsphy_lanes: lanes@1da7400 {
+ reg = <0x01da7400 0x128>,
+ <0x01da7600 0x1fc>,
+ <0x01da7c00 0x1dc>,
+ <0x01da7800 0x128>,
+ <0x01da7a00 0x1fc>;
+ #phy-cells = <0>;
+ };
+ };
};
};

--
2.17.1

2019-02-04 17:44:58

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 2/5] arm64: dts: qcom: msm8998: Allow drivers to set-load

The UFS host controller driver needs to set the load on 3 power rails
(l20, l26, s4) but the operation fails silently unless we specify the
regulator-allow-set-load property in the corresponding DT nodes.

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

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index 80075283847a..198885db775e 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -104,6 +104,7 @@
vreg_s4a_1p8: s4 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-allow-set-load;
};
vreg_s5a_2p04: s5 {
regulator-min-microvolt = <1904000>;
@@ -188,6 +189,7 @@
vreg_l20a_2p95: l20 {
regulator-min-microvolt = <2960000>;
regulator-max-microvolt = <2960000>;
+ regulator-allow-set-load;
};
vreg_l21a_2p95: l21 {
regulator-min-microvolt = <2960000>;
@@ -212,6 +214,7 @@
vreg_l26a_1p2: l26 {
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
+ regulator-allow-set-load;
};
vreg_l28_3p0: l28 {
regulator-min-microvolt = <3008000>;
--
2.17.1

2019-02-04 17:45:26

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 4/5] phy: qcom-qmp: Add QMP UFS PHY support for msm8998

Use same init sequence as sdm845.

Signed-off-by: Marc Gonzalez <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b4006818e1b6..39b9c31b67d0 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -1735,6 +1735,9 @@ static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
}, {
.compatible = "qcom,msm8996-qmp-usb3-phy",
.data = &msm8996_usb3phy_cfg,
+ }, {
+ .compatible = "qcom,msm8998-qmp-ufs-phy",
+ .data = &sdm845_ufsphy_cfg,
}, {
.compatible = "qcom,ipq8074-qmp-pcie-phy",
.data = &ipq8074_pciephy_cfg,
--
2.17.1

2019-02-04 17:45:48

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 3/5] dt-bindings: phy-qcom-qmp: Add qcom,msm8998-qmp-ufs-phy

Add compatible string for QMP UFS phy on msm8998.

Signed-off-by: Marc Gonzalez <[email protected]>
---
Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index 41a1074228ba..6ec278f5af86 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -9,6 +9,7 @@ Required properties:
"qcom,ipq8074-qmp-pcie-phy" for PCIe phy on IPQ8074
"qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
"qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996,
+ "qcom,msm8998-qmp-ufs-phy" for UFS QMP phy on msm8998,
"qcom,sdm845-qmp-usb3-phy" for USB3 QMP V3 phy on sdm845,
"qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845,
"qcom,sdm845-qmp-ufs-phy" for UFS QMP phy on sdm845.
--
2.17.1

2019-02-04 17:47:07

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.

Calling ufshcd_set_vccq_rail_unused hangs my system.
It seems vccq is not *not* needed.

Signed-off-by: Marc Gonzalez <[email protected]>
---
drivers/scsi/ufs/ufs.h | 1 -
drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
2 files changed, 4 insertions(+), 56 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index dd65fea07687..7da7318eb6a6 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -514,7 +514,6 @@ struct ufs_vreg {
struct regulator *reg;
const char *name;
bool enabled;
- bool unused;
int min_uV;
int max_uV;
int min_uA;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9ba7671b84f8..8b9a01073d62 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
bool skip_ref_clk);
static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
-static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
@@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
ufs_fixup_device_setup(hba, &card);
ufshcd_tune_unipro_params(hba);

- ret = ufshcd_set_vccq_rail_unused(hba,
- (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
- if (ret)
- goto out;
-
/* UFS device is also active now */
ufshcd_set_ufs_dev_active(hba);
ufshcd_force_reset_auto_bkops(hba);
@@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
struct ufs_vreg *vreg)
{
- if (!vreg)
- return 0;
- else if (vreg->unused)
- return 0;
- else
- return ufshcd_config_vreg_load(hba->dev, vreg,
- UFS_VREG_LPM_LOAD_UA);
+ return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
}

static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
struct ufs_vreg *vreg)
{
- if (!vreg)
- return 0;
- else if (vreg->unused)
- return 0;
- else
- return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
+ return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
}

static int ufshcd_config_vreg(struct device *dev,
@@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
{
int ret = 0;

- if (!vreg)
- goto out;
- else if (vreg->enabled || vreg->unused)
+ if (!vreg || vreg->enabled)
goto out;

ret = ufshcd_config_vreg(dev, vreg, true);
@@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
{
int ret = 0;

- if (!vreg)
- goto out;
- else if (!vreg->enabled || vreg->unused)
+ if (!vreg || !vreg->enabled)
goto out;

ret = regulator_disable(vreg->reg);
@@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
return 0;
}

-static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
-{
- int ret = 0;
- struct ufs_vreg_info *info = &hba->vreg_info;
-
- if (!info)
- goto out;
- else if (!info->vccq)
- goto out;
-
- if (unused) {
- /* shut off the rail here */
- ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
- /*
- * Mark this rail as no longer used, so it doesn't get enabled
- * later by mistake
- */
- if (!ret)
- info->vccq->unused = true;
- } else {
- /*
- * rail should have been already enabled hence just make sure
- * that unused flag is cleared.
- */
- info->vccq->unused = false;
- }
-out:
- return ret;
-}
-
static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
bool skip_ref_clk)
{
--
2.17.1

2019-02-04 18:07:19

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] arm64: dts: qcom: msm8998: Add UFS nodes

On 2/4/2019 10:36 AM, Marc Gonzalez wrote:
> Add host controller and PHY DT nodes.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
> vddp-ref-clk-max-microamp = <100>; sounds tiny and fishy.
> Jeffrey, Bjorn, can you check?
> The PHY driver doesn't seem to try to set any load...

Yes, 0.1mA matches the hardware documentation.
IMO, this should be the last change in the series, not the first.
Otherwise, seems good.

Reviewed-by: Jeffrey Hugo <[email protected]>

> ---
> arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 19 +++++++
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 63 +++++++++++++++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 50e9033aa7f6..80075283847a 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -257,3 +257,22 @@
> pinctrl-0 = <&sdc2_clk_on &sdc2_cmd_on &sdc2_data_on &sdc2_cd_on>;
> pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
> };
> +
> +&ufshc {
> + vcc-supply = <&vreg_l20a_2p95>;
> + vccq-supply = <&vreg_l26a_1p2>;
> + vccq2-supply = <&vreg_s4a_1p8>;
> + vcc-max-microamp = <750000>;
> + vccq-max-microamp = <560000>;
> + vccq2-max-microamp = <750000>;
> +};
> +
> +&ufsphy {
> + vdda-phy-supply = <&vreg_l1a_0p875>;
> + vdda-pll-supply = <&vreg_l2a_1p2>;
> + vddp-ref-clk-supply = <&vreg_l26a_1p2>;
> + vdda-phy-max-microamp = <51400>;
> + vdda-pll-max-microamp = <14600>;
> + vddp-ref-clk-max-microamp = <100>;
> + vddp-ref-clk-always-on;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 6f4f4b79853b..831af20143da 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -711,6 +711,69 @@
> redistributor-stride = <0x0 0x20000>;
> interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> };
> +
> + ufshc: ufshc@1da4000 {
> + compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
> + "jedec,ufs-2.0";
> + reg = <0x01da4000 0x2500>;
> + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&ufsphy_lanes>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + power-domains = <&gcc UFS_GDSC>;
> +
> + clock-names =
> + "core_clk",
> + "bus_aggr_clk",
> + "iface_clk",
> + "core_clk_unipro",
> + "ref_clk",
> + "tx_lane0_sync_clk",
> + "rx_lane0_sync_clk",
> + "rx_lane1_sync_clk";
> + clocks =
> + <&gcc GCC_UFS_AXI_CLK>,
> + <&gcc GCC_AGGRE1_UFS_AXI_CLK>,
> + <&gcc GCC_UFS_AHB_CLK>,
> + <&gcc GCC_UFS_UNIPRO_CORE_CLK>,
> + <&rpmcc RPM_SMD_LN_BB_CLK1>,
> + <&gcc GCC_UFS_TX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_RX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_RX_SYMBOL_1_CLK>;
> + freq-table-hz =
> + <50000000 200000000>,
> + <0 0>,
> + <0 0>,
> + <37500000 150000000>,
> + <0 0>,
> + <0 0>,
> + <0 0>,
> + <0 0>;
> +
> + resets = <&gcc GCC_UFS_BCR>;
> + reset-names = "rst";
> + };
> +
> + ufsphy: phy@1da7000 {
> + compatible = "qcom,msm8998-qmp-ufs-phy";
> + reg = <0x01da7000 0x18c>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + clock-names = "ref", "ref_aux";
> + clocks =
> + <&gcc GCC_UFS_CLKREF_CLK>,
> + <&gcc GCC_UFS_PHY_AUX_CLK>;
> +
> + ufsphy_lanes: lanes@1da7400 {
> + reg = <0x01da7400 0x128>,
> + <0x01da7600 0x1fc>,
> + <0x01da7c00 0x1dc>,
> + <0x01da7800 0x128>,
> + <0x01da7a00 0x1fc>;
> + #phy-cells = <0>;
> + };
> + };
> };
> };
>
>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 18:08:30

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] arm64: dts: qcom: msm8998: Allow drivers to set-load

On 2/4/2019 10:37 AM, Marc Gonzalez wrote:
> The UFS host controller driver needs to set the load on 3 power rails
> (l20, l26, s4) but the operation fails silently unless we specify the
> regulator-allow-set-load property in the corresponding DT nodes.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---

Reviewed-by: Jeffrey Hugo <[email protected]>

> arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 80075283847a..198885db775e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -104,6 +104,7 @@
> vreg_s4a_1p8: s4 {
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> + regulator-allow-set-load;
> };
> vreg_s5a_2p04: s5 {
> regulator-min-microvolt = <1904000>;
> @@ -188,6 +189,7 @@
> vreg_l20a_2p95: l20 {
> regulator-min-microvolt = <2960000>;
> regulator-max-microvolt = <2960000>;
> + regulator-allow-set-load;
> };
> vreg_l21a_2p95: l21 {
> regulator-min-microvolt = <2960000>;
> @@ -212,6 +214,7 @@
> vreg_l26a_1p2: l26 {
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> + regulator-allow-set-load;
> };
> vreg_l28_3p0: l28 {
> regulator-min-microvolt = <3008000>;
>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 18:09:23

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] dt-bindings: phy-qcom-qmp: Add qcom,msm8998-qmp-ufs-phy

On 2/4/2019 10:38 AM, Marc Gonzalez wrote:
> Add compatible string for QMP UFS phy on msm8998.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> index 41a1074228ba..6ec278f5af86 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -9,6 +9,7 @@ Required properties:
> "qcom,ipq8074-qmp-pcie-phy" for PCIe phy on IPQ8074
> "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
> "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996,
> + "qcom,msm8998-qmp-ufs-phy" for UFS QMP phy on msm8998,
> "qcom,sdm845-qmp-usb3-phy" for USB3 QMP V3 phy on sdm845,
> "qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845,
> "qcom,sdm845-qmp-ufs-phy" for UFS QMP phy on sdm845.
>

I think this needs to be based on phy-next, otherwise I forsee a merge
conflict.

Also, I would expect changes to the rest of the file as well (what
clocks are required, etc).

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 18:11:57

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/4/2019 10:42 AM, Marc Gonzalez wrote:
> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>
> Calling ufshcd_set_vccq_rail_unused hangs my system.
> It seems vccq is not *not* needed.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---

Reviewed-by: Jeffrey Hugo <[email protected]>

> drivers/scsi/ufs/ufs.h | 1 -
> drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
> 2 files changed, 4 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index dd65fea07687..7da7318eb6a6 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -514,7 +514,6 @@ struct ufs_vreg {
> struct regulator *reg;
> const char *name;
> bool enabled;
> - bool unused;
> int min_uV;
> int max_uV;
> int min_uA;
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9ba7671b84f8..8b9a01073d62 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> bool skip_ref_clk);
> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
> static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
> @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> ufs_fixup_device_setup(hba, &card);
> ufshcd_tune_unipro_params(hba);
>
> - ret = ufshcd_set_vccq_rail_unused(hba,
> - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
> - if (ret)
> - goto out;
> -
> /* UFS device is also active now */
> ufshcd_set_ufs_dev_active(hba);
> ufshcd_force_reset_auto_bkops(hba);
> @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
> static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
> struct ufs_vreg *vreg)
> {
> - if (!vreg)
> - return 0;
> - else if (vreg->unused)
> - return 0;
> - else
> - return ufshcd_config_vreg_load(hba->dev, vreg,
> - UFS_VREG_LPM_LOAD_UA);
> + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
> }
>
> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> struct ufs_vreg *vreg)
> {
> - if (!vreg)
> - return 0;
> - else if (vreg->unused)
> - return 0;
> - else
> - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> }
>
> static int ufshcd_config_vreg(struct device *dev,
> @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
> {
> int ret = 0;
>
> - if (!vreg)
> - goto out;
> - else if (vreg->enabled || vreg->unused)
> + if (!vreg || vreg->enabled)
> goto out;
>
> ret = ufshcd_config_vreg(dev, vreg, true);
> @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
> {
> int ret = 0;
>
> - if (!vreg)
> - goto out;
> - else if (!vreg->enabled || vreg->unused)
> + if (!vreg || !vreg->enabled)
> goto out;
>
> ret = regulator_disable(vreg->reg);
> @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
> return 0;
> }
>
> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
> -{
> - int ret = 0;
> - struct ufs_vreg_info *info = &hba->vreg_info;
> -
> - if (!info)
> - goto out;
> - else if (!info->vccq)
> - goto out;
> -
> - if (unused) {
> - /* shut off the rail here */
> - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
> - /*
> - * Mark this rail as no longer used, so it doesn't get enabled
> - * later by mistake
> - */
> - if (!ret)
> - info->vccq->unused = true;
> - } else {
> - /*
> - * rail should have been already enabled hence just make sure
> - * that unused flag is cleared.
> - */
> - info->vccq->unused = false;
> - }
> -out:
> - return ret;
> -}
> -
> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> bool skip_ref_clk)
> {
>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 18:12:23

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] phy: qcom-qmp: Add QMP UFS PHY support for msm8998

On 2/4/2019 10:39 AM, Marc Gonzalez wrote:
> Use same init sequence as sdm845.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---

Reviewed-by: Jeffrey Hugo <[email protected]>

> drivers/phy/qualcomm/phy-qcom-qmp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index b4006818e1b6..39b9c31b67d0 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -1735,6 +1735,9 @@ static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
> }, {
> .compatible = "qcom,msm8996-qmp-usb3-phy",
> .data = &msm8996_usb3phy_cfg,
> + }, {
> + .compatible = "qcom,msm8998-qmp-ufs-phy",
> + .data = &sdm845_ufsphy_cfg,
> }, {
> .compatible = "qcom,ipq8074-qmp-pcie-phy",
> .data = &ipq8074_pciephy_cfg,
>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 21:34:04

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"


> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>
> Calling ufshcd_set_vccq_rail_unused hangs my system.
> It seems vccq is not *not* needed.
This patch essentially implements the UFS_DEVICE_NO_VCCQ quirk,
Which is needed for both Samsung and Hynix devices.
Once acked by those vendors, can be removed from the quirk list as well.

Thanks,
Avri
>
> Signed-off-by: Marc Gonzalez <[email protected]>

2019-02-04 21:47:27

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/4/2019 12:51 PM, Avri Altman wrote:
>
>> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>>
>> Calling ufshcd_set_vccq_rail_unused hangs my system.
>> It seems vccq is not *not* needed.
> This patch essentially implements the UFS_DEVICE_NO_VCCQ quirk,
> Which is needed for both Samsung and Hynix devices.

Needed, or optimal?
I'm rather sure Marc's device has a Samsung UFS chip, as do my devices
and we've seen nothing but benefit from the proposed revert.

> Once acked by those vendors, can be removed from the quirk list as well.
>
> Thanks,
> Avri
>>
>> Signed-off-by: Marc Gonzalez <[email protected]>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-04 21:48:57

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On Mon 04 Feb 11:51 PST 2019, Avri Altman wrote:

>
> > This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
> >
> > Calling ufshcd_set_vccq_rail_unused hangs my system.
> > It seems vccq is not *not* needed.
> This patch essentially implements the UFS_DEVICE_NO_VCCQ quirk,
> Which is needed for both Samsung and Hynix devices.
> Once acked by those vendors, can be removed from the quirk list as well.
>

If a device does not need VCCQ, for some reason, then why is the VCCQ
supply specified for this device?

Disabling unused regulators is already handled by the regulator
framework, so why does the UFSHCD driver take this role as well?

Regards,
Bjorn

2019-02-05 05:19:12

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Hi Marc,

On 04/02/19 11:12 PM, Marc Gonzalez wrote:
> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>
> Calling ufshcd_set_vccq_rail_unused hangs my system.
> It seems vccq is not *not* needed.
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---

AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
either floating or connected to Ground, at least on the devices that I
have worked on).
You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
quirks, in that case VCCQ regulator should having been disabled.
So you mean your system hanged because vccq regulator got disabled?

> drivers/scsi/ufs/ufs.h | 1 -
> drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
> 2 files changed, 4 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index dd65fea07687..7da7318eb6a6 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -514,7 +514,6 @@ struct ufs_vreg {
> struct regulator *reg;
> const char *name;
> bool enabled;
> - bool unused;
> int min_uV;
> int max_uV;
> int min_uA;
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9ba7671b84f8..8b9a01073d62 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> bool skip_ref_clk);
> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
> static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
> @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> ufs_fixup_device_setup(hba, &card);
> ufshcd_tune_unipro_params(hba);
>
> - ret = ufshcd_set_vccq_rail_unused(hba,
> - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
> - if (ret)
> - goto out;
> -
> /* UFS device is also active now */
> ufshcd_set_ufs_dev_active(hba);
> ufshcd_force_reset_auto_bkops(hba);
> @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
> static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
> struct ufs_vreg *vreg)
> {
> - if (!vreg)
> - return 0;
> - else if (vreg->unused)
> - return 0;
> - else
> - return ufshcd_config_vreg_load(hba->dev, vreg,
> - UFS_VREG_LPM_LOAD_UA);
> + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
> }
>
> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> struct ufs_vreg *vreg)
> {
> - if (!vreg)
> - return 0;
> - else if (vreg->unused)
> - return 0;
> - else
> - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> }
>
> static int ufshcd_config_vreg(struct device *dev,
> @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
> {
> int ret = 0;
>
> - if (!vreg)
> - goto out;
> - else if (vreg->enabled || vreg->unused)
> + if (!vreg || vreg->enabled)
> goto out;
>
> ret = ufshcd_config_vreg(dev, vreg, true);
> @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
> {
> int ret = 0;
>
> - if (!vreg)
> - goto out;
> - else if (!vreg->enabled || vreg->unused)
> + if (!vreg || !vreg->enabled)
> goto out;
>
> ret = regulator_disable(vreg->reg);
> @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
> return 0;
> }
>
> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
> -{
> - int ret = 0;
> - struct ufs_vreg_info *info = &hba->vreg_info;
> -
> - if (!info)
> - goto out;
> - else if (!info->vccq)
> - goto out;
> -
> - if (unused) {
> - /* shut off the rail here */
> - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
> - /*
> - * Mark this rail as no longer used, so it doesn't get enabled
> - * later by mistake
> - */
> - if (!ret)
> - info->vccq->unused = true;
> - } else {
> - /*
> - * rail should have been already enabled hence just make sure
> - * that unused flag is cleared.
> - */
> - info->vccq->unused = false;
> - }
> -out:
> - return ret;
> -}
> -
> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> bool skip_ref_clk)
> {
>

2019-02-05 06:34:31

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On Mon 04 Feb 20:58 PST 2019, Alim Akhtar wrote:

> Hi Marc,
>
> On 04/02/19 11:12 PM, Marc Gonzalez wrote:
> > This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
> >
> > Calling ufshcd_set_vccq_rail_unused hangs my system.
> > It seems vccq is not *not* needed.
> >
> > Signed-off-by: Marc Gonzalez <[email protected]>
> > ---
>
> AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
> either floating or connected to Ground, at least on the devices that I
> have worked on).

But why does such system define a vccq-supply? If the system doesn't
have a regulator connected to VCCQ, then the UFS driver shouldn't be
told that there is one. And if VCCQ is optional the UFS driver should
support the fact that this regulator might not be supplied (i.e. call
regulator_get_optional() and handle the error indicating that the supply
isn't specified).

Regards,
Bjorn

> You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
> quirks, in that case VCCQ regulator should having been disabled.
> So you mean your system hanged because vccq regulator got disabled?
>
> > drivers/scsi/ufs/ufs.h | 1 -
> > drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
> > 2 files changed, 4 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index dd65fea07687..7da7318eb6a6 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -514,7 +514,6 @@ struct ufs_vreg {
> > struct regulator *reg;
> > const char *name;
> > bool enabled;
> > - bool unused;
> > int min_uV;
> > int max_uV;
> > int min_uA;
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 9ba7671b84f8..8b9a01073d62 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
> > static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> > bool skip_ref_clk);
> > static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> > -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
> > static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
> > static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
> > static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
> > @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> > ufs_fixup_device_setup(hba, &card);
> > ufshcd_tune_unipro_params(hba);
> >
> > - ret = ufshcd_set_vccq_rail_unused(hba,
> > - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
> > - if (ret)
> > - goto out;
> > -
> > /* UFS device is also active now */
> > ufshcd_set_ufs_dev_active(hba);
> > ufshcd_force_reset_auto_bkops(hba);
> > @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
> > static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
> > struct ufs_vreg *vreg)
> > {
> > - if (!vreg)
> > - return 0;
> > - else if (vreg->unused)
> > - return 0;
> > - else
> > - return ufshcd_config_vreg_load(hba->dev, vreg,
> > - UFS_VREG_LPM_LOAD_UA);
> > + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
> > }
> >
> > static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> > struct ufs_vreg *vreg)
> > {
> > - if (!vreg)
> > - return 0;
> > - else if (vreg->unused)
> > - return 0;
> > - else
> > - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> > + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> > }
> >
> > static int ufshcd_config_vreg(struct device *dev,
> > @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
> > {
> > int ret = 0;
> >
> > - if (!vreg)
> > - goto out;
> > - else if (vreg->enabled || vreg->unused)
> > + if (!vreg || vreg->enabled)
> > goto out;
> >
> > ret = ufshcd_config_vreg(dev, vreg, true);
> > @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
> > {
> > int ret = 0;
> >
> > - if (!vreg)
> > - goto out;
> > - else if (!vreg->enabled || vreg->unused)
> > + if (!vreg || !vreg->enabled)
> > goto out;
> >
> > ret = regulator_disable(vreg->reg);
> > @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
> > return 0;
> > }
> >
> > -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
> > -{
> > - int ret = 0;
> > - struct ufs_vreg_info *info = &hba->vreg_info;
> > -
> > - if (!info)
> > - goto out;
> > - else if (!info->vccq)
> > - goto out;
> > -
> > - if (unused) {
> > - /* shut off the rail here */
> > - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
> > - /*
> > - * Mark this rail as no longer used, so it doesn't get enabled
> > - * later by mistake
> > - */
> > - if (!ret)
> > - info->vccq->unused = true;
> > - } else {
> > - /*
> > - * rail should have been already enabled hence just make sure
> > - * that unused flag is cleared.
> > - */
> > - info->vccq->unused = false;
> > - }
> > -out:
> > - return ret;
> > -}
> > -
> > static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> > bool skip_ref_clk)
> > {
> >

2019-02-05 11:10:47

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Hi Bjorn,

On 05/02/19 11:57 AM, Bjorn Andersson wrote:
> On Mon 04 Feb 20:58 PST 2019, Alim Akhtar wrote:
>
>> Hi Marc,
>>
>> On 04/02/19 11:12 PM, Marc Gonzalez wrote:
>>> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>>>
>>> Calling ufshcd_set_vccq_rail_unused hangs my system.
>>> It seems vccq is not *not* needed.
>>>
>>> Signed-off-by: Marc Gonzalez <[email protected]>
>>> ---
>>
>> AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
>> either floating or connected to Ground, at least on the devices that I
>> have worked on).
>
> But why does such system define a vccq-supply? If the system doesn't
> have a regulator connected to VCCQ, then the UFS driver shouldn't be
> told that there is one. And if VCCQ is optional the UFS driver should
> support the fact that this regulator might not be supplied (i.e. call
> regulator_get_optional() and handle the error indicating that the supply
> isn't specified).
>
As per JESD220C, chapter 6.1, it does says "VCCQ - Supply voltage used
typically for the memory controller and optionally for the PHY
interface, the memory IO, and any other internal very low voltage block"
And we have VCCQ2 - which serve the pretty much same purpose. The
voltage range for VCCQ and VCCQ2 are different, VCCQ has a lower voltage
suitable to some low voltage block inside UFS device. I think this is
design consideration which allow some vendor to use one less physical
pin may be. And also depends on the voltage requirements of some of the
internal circuit.


> Regards,
> Bjorn
>
>> You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
>> quirks, in that case VCCQ regulator should having been disabled.
>> So you mean your system hanged because vccq regulator got disabled?
>>
>>> drivers/scsi/ufs/ufs.h | 1 -
>>> drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
>>> 2 files changed, 4 insertions(+), 56 deletions(-)
>>>
>>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>>> index dd65fea07687..7da7318eb6a6 100644
>>> --- a/drivers/scsi/ufs/ufs.h
>>> +++ b/drivers/scsi/ufs/ufs.h
>>> @@ -514,7 +514,6 @@ struct ufs_vreg {
>>> struct regulator *reg;
>>> const char *name;
>>> bool enabled;
>>> - bool unused;
>>> int min_uV;
>>> int max_uV;
>>> int min_uA;
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 9ba7671b84f8..8b9a01073d62 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
>>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
>>> bool skip_ref_clk);
>>> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
>>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
>>> static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
>>> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
>>> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
>>> @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>>> ufs_fixup_device_setup(hba, &card);
>>> ufshcd_tune_unipro_params(hba);
>>>
>>> - ret = ufshcd_set_vccq_rail_unused(hba,
>>> - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
>>> - if (ret)
>>> - goto out;
>>> -
>>> /* UFS device is also active now */
>>> ufshcd_set_ufs_dev_active(hba);
>>> ufshcd_force_reset_auto_bkops(hba);
>>> @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
>>> static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
>>> struct ufs_vreg *vreg)
>>> {
>>> - if (!vreg)
>>> - return 0;
>>> - else if (vreg->unused)
>>> - return 0;
>>> - else
>>> - return ufshcd_config_vreg_load(hba->dev, vreg,
>>> - UFS_VREG_LPM_LOAD_UA);
>>> + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
>>> }
>>>
>>> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>>> struct ufs_vreg *vreg)
>>> {
>>> - if (!vreg)
>>> - return 0;
>>> - else if (vreg->unused)
>>> - return 0;
>>> - else
>>> - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
>>> + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
>>> }
>>>
>>> static int ufshcd_config_vreg(struct device *dev,
>>> @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>> {
>>> int ret = 0;
>>>
>>> - if (!vreg)
>>> - goto out;
>>> - else if (vreg->enabled || vreg->unused)
>>> + if (!vreg || vreg->enabled)
>>> goto out;
>>>
>>> ret = ufshcd_config_vreg(dev, vreg, true);
>>> @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>> {
>>> int ret = 0;
>>>
>>> - if (!vreg)
>>> - goto out;
>>> - else if (!vreg->enabled || vreg->unused)
>>> + if (!vreg || !vreg->enabled)
>>> goto out;
>>>
>>> ret = regulator_disable(vreg->reg);
>>> @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
>>> return 0;
>>> }
>>>
>>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
>>> -{
>>> - int ret = 0;
>>> - struct ufs_vreg_info *info = &hba->vreg_info;
>>> -
>>> - if (!info)
>>> - goto out;
>>> - else if (!info->vccq)
>>> - goto out;
>>> -
>>> - if (unused) {
>>> - /* shut off the rail here */
>>> - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
>>> - /*
>>> - * Mark this rail as no longer used, so it doesn't get enabled
>>> - * later by mistake
>>> - */
>>> - if (!ret)
>>> - info->vccq->unused = true;
>>> - } else {
>>> - /*
>>> - * rail should have been already enabled hence just make sure
>>> - * that unused flag is cleared.
>>> - */
>>> - info->vccq->unused = false;
>>> - }
>>> -out:
>>> - return ret;
>>> -}
>>> -
>>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
>>> bool skip_ref_clk)
>>> {
>>>
>
>

2019-02-05 17:47:34

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 05/02/2019 05:58, Alim Akhtar wrote:

> On 04/02/19 11:12 PM, Marc Gonzalez wrote:
>
>> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>>
>> Calling ufshcd_set_vccq_rail_unused hangs my system.
>> It seems vccq is not *not* needed.
>
> AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
> either floating or connected to Ground, at least on the devices that I
> have worked on).
> You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
> quirks, in that case VCCQ regulator should having been disabled.
> So you mean your system hanged because vccq regulator got disabled?

If I keep the proposed patch, this is what I get:

ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
l9: supplied by bob
l10: supplied by bob
l13: supplied by bob
l16: supplied by bob
l18: supplied by bob
l19: supplied by bob
l21: supplied by bob
l22: supplied by bob
l23: supplied by bob
l24: supplied by bob
l25: supplied by bob
l28: [garbled]
ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[3, 3], lane[2, 2], pwr[FAST MODE, FAST MODE], rate = 2
scsi 0:0:0:49488: scsi_add_lun: correcting incorrect peripheral device type 0x0 for W-LUN 0x c150hN
scsi 0:0:0:49488: Well-known LUN SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
scsi 0:0:0:49476: scsi_add_lun: correcting incorrect peripheral device type 0x0 for W-LUN 0x c144hN
scsi 0:0:0:49476: Well-known LUN SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
scsi 0:0:0:49456: scsi_add_lun: correcting incorrect peripheral device type 0x0 for W-LUN 0x c130hN
scsi 0:0:0:49456: Well-known LUN SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
scsi 0:0:0:0: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
sd 0:0:0:0: Power-on or device reset occurred
sd 0:0:0:0: [sda] 14145536 4096-byte logical blocks: (57.9 GB/54.0 GiB)
sd 0:0:0:0: [sda] Write Protect is off
scsi 0:0:0:1: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
sd 0:0:0:1: Power-on or device reset occurred
scsi 0:0:0:2: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
sd 0:0:0:1: [sdb] 1024 4096-byte logical blocks: (4.19 MB/4.00 MiB)
sd 0:0:0:1: [sdb] Write Protect is off
sd 0:0:0:2: Power-on or device reset occurred
scsi 0:0:0:3: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
sd 0:0:0:2: [sdc] 1024 4096-byte logical blocks: (4.19 MB/4.00 MiB)
sd 0:0:0:1: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA
sd 0:0:0:3: Power-on or device reset occurred
sda: sda1 sda2 sda3 sda4 sda5 sda6 sda7
sd 0:0:0:3: [sdd] 32768 4096-byte logical blocks: (134 MB/128 MiB)
sd 0:0:0:2: [sdc] Write Protect is off
scsi 0:0:0:4: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
sd 0:0:0:3: [sdd] Write Protect is off
scsi 0:0:0:5: Direct-Access SAMSUNG KLUCG4J1EB-B0B1 0200 PQ: 0 ANSI: 6
random: fast init done
sd 0:0:0:4: Power-on or device reset occurred
sd 0:0:0:5: Power-on or device reset occurred
sd 0:0:0:0: [sda] Attached SCSI disk
sd 0:0:0:4: [sde] 1048576 4096-byte logical blocks: (4.29 GB/4.00 GiB)
sd 0:0:0:5: [sdf] 393216 4096-byte logical blocks: (1.61 GB/1.50 GiB)
sd 0:0:0:3: [sdd] Write cache: enabled, read cache: enabled, supports DPO and FUA
sdb: sdb1
sd 0:0:0:4: [sde] Write Protect is off
sd 0:0:0:4: [sde] Write cache: enabled, read cache: enabled, supports DPO and FUA
sd 0:0:0:5: [sdf] Write Protect is off
sd 0:0:0:2: [sdc] Write cache: enabled, read cache: enabled, supports DPO and FUA
sde: sde1 sde2 sde3 sde4 sde5 sde6 sde7 sde8 sde9 sde10 sde11 sde12 sde13 sde14 sde15 sde16 sde17 sde18 sde19 sde20 sde21 sde22 sde23 sde24 sde25 sde26 sde27 sde28 sde29 sde30 sde31 sde32 sde33 sde34 sde35 sde36 sde37 sde38
sd 0:0:0:5: [sdf] Write cache: enabled, read cache: enabled, supports DPO and FUA
sdd: sdd1 sdd2
sd 0:0:0:1: [sdb] Attached SCSI disk
sdc: sdc1
sd 0:0:0:2: [sdc] Attached SCSI disk
sd 0:0:0:4: [sde] Attached SCSI disk
sd 0:0:0:3: [sdd] Attached SCSI disk
sdf: sdf1 sdf2 sdf3
sd 0:0:0:5: [sdf] Attached SCSI disk



If I drop the proposed patch, this is what I get:

ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
l9: supplied by bob
l10: supplied by bob
l13: supplied by bob
l16: supplied by bob
l18: supplied by bob
l19: supplied by bob
l21: supplied by bob
l22: supplied by bob
l23: supplied by bob
l24: supplied by bob
l25: supplied by bob
l28: [garbled]
regulator_disable: ENTER vdd_l26
regulator_disable: EXIT vdd_l26
ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
ufshcd-qcom 1da4000.ufshc: link startup failed 1
ufshcd-qcom 1da4000.ufshc: UFS Host state=0
ufshcd-qcom 1da4000.ufshc: lrb in use=0x1000000, outstanding reqs=0x1000000 tasks=0x0
ufshcd-qcom 1da4000.ufshc: saved_err=0x4, saved_uic_err=0x5
ufshcd-qcom 1da4000.ufshc: Device power mode=1, UIC link state=0
ufshcd-qcom 1da4000.ufshc: PM in progress=0, sys. suspended=0
ufshcd-qcom 1da4000.ufshc: Auto BKOPS=1, Host self-block=0
ufshcd-qcom 1da4000.ufshc: Clk gate=1
ufshcd-qcom 1da4000.ufshc: error handling flags=0x1, req. abort count=0
ufshcd-qcom 1da4000.ufshc: Host capabilities=0x1587001f, caps=0xf
ufshcd-qcom 1da4000.ufshc: quirks=0x0, dev. quirks=0xe6
ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
host_regs: 00000000: 1587001f 00000000 00000210 00000000
host_regs: 00000010: 01000000 00010217 00000000 00000000
host_regs: 00000020: 00000000 00000470 00000000 00000000
host_regs: 00000030: 00000008 00000001 00000000 00000000
host_regs: 00000040: 00000000 00000000 00000000 00000000
host_regs: 00000050: 00000000 00000000 00000000 00000000
host_regs: 00000060: 00000000 00000000 00000000 00000000
host_regs: 00000070: 00000000 00000000 00000000 00000000
host_regs: 00000080: 00000000 00000000 00000000 00000000
host_regs: 00000090: 00000001 00410000 00000000 00000001
ufshcd-qcom 1da4000.ufshc: hba->ufs_version = 0x210, hba->capabilities = 0x1587001f
ufshcd-qcom 1da4000.ufshc: hba->outstanding_reqs = 0x1000000, hba->outstanding_tasks = 0x0
ufshcd-qcom 1da4000.ufshc: last_hibern8_exit_tstamp at 0 us, hibern8_exit_cnt = 0
ufshcd-qcom 1da4000.ufshc: dl_err[0] = 0x80002000 at 1321487 us
ufshcd-qcom 1da4000.ufshc: dl_err[7] = 0x80000002 at 1187360 us
ufshcd-qcom 1da4000.ufshc: clk: core_clk, rate: 200000000
ufshcd-qcom 1da4000.ufshc: clk: core_clk_unipro, rate: 150000000
HCI Vendor Specific Registers 00000000: 000000c8 00000000 00000000 00000000
HCI Vendor Specific Registers 00000010: 00000000 00000000 00000001 5c5c052c
HCI Vendor Specific Registers 00000020: 3f0113ff 30010000 00000007 00000000
HCI Vendor Specific Registers 00000030: 00000000 00000000 02500000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000000: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000010: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000020: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000030: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000040: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000050: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000060: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000070: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000080: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 00000090: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_REG_OCSC 000000a0: 00000000 00000000 00000000 00000000
UFS_UFS_DBG_RD_EDTL_RAM 00000000: 00000000 46269aca bba683dc ae26420e
UFS_UFS_DBG_RD_EDTL_RAM 00000010: fe46b054 3c97c041 038ac9ef a263479f
UFS_UFS_DBG_RD_EDTL_RAM 00000020: 3fac4ac6 4b8f4fdf 6e561798 cac010d9
UFS_UFS_DBG_RD_EDTL_RAM 00000030: 28dbbdbc 8b912ab6 8e7eebaa defdea74
UFS_UFS_DBG_RD_EDTL_RAM 00000040: caefd827 4f68b8b7 437f8c09 e622feb7
UFS_UFS_DBG_RD_EDTL_RAM 00000050: 26521148 6e7241fd c609e7e6 3d582eaf
UFS_UFS_DBG_RD_EDTL_RAM 00000060: 00000024 aa98783f e71f3ce9 eeb2b7af
UFS_UFS_DBG_RD_EDTL_RAM 00000070: 01bed85b 6bba9c5f 79bb6c47 00000000
UFS_UFS_DBG_RD_DESC_RAM 00000000: 40000fff 000269c0 c0004fff 0002674c
UFS_UFS_DBG_RD_DESC_RAM 00000010: 47ca8e27 002ad393 c14304be 0029798f
UFS_UFS_DBG_RD_DESC_RAM 00000020: 28ffe38d 0023cc67 e5aa2e2c 001aa623
UFS_UFS_DBG_RD_DESC_RAM 00000030: 8680feea 0028a93f e6ef9ebb 001a39be
UFS_UFS_DBG_RD_DESC_RAM 00000040: 28f68ce7 000a3abd 79a0a809 000eaa08
UFS_UFS_DBG_RD_DESC_RAM 00000050: ceb38db8 000aa74a f3e9f7fb 002c2a6c
UFS_UFS_DBG_RD_DESC_RAM 00000060: 687de4d6 001a7a3a e6e89b2b 00063fac
UFS_UFS_DBG_RD_DESC_RAM 00000070: 2fb2c052 003aabea c26ee943 002b8d8d
UFS_UFS_DBG_RD_DESC_RAM 00000080: bbbad83b 002aabe5 6df4bf83 003bee50
UFS_UFS_DBG_RD_DESC_RAM 00000090: f7b08e03 0009cb86 caf5b2d0 000eee24
UFS_UFS_DBG_RD_DESC_RAM 000000a0: ad263982 001a27d4 8fa39e9e 00213a02
UFS_UFS_DBG_RD_DESC_RAM 000000b0: edc2c838 001acc7a a5e1952c 002a25b1
UFS_UFS_DBG_RD_DESC_RAM 000000c0: ee0eb208 0029f609 b18cacbb 000eb1c8
UFS_UFS_DBG_RD_DESC_RAM 000000d0: 113eeebe 001a3c94 fab74732 000abef5
UFS_UFS_DBG_RD_DESC_RAM 000000e0: 21e5e2fe 000b8b63 e4eafcfc 00348e05
UFS_UFS_DBG_RD_DESC_RAM 000000f0: 206a7c3e 002ae252 b6087166 0023e74d
UFS_UFS_DBG_RD_DESC_RAM 00000100: 092820a0 0039b70e 294903ae 0014880f
UFS_UFS_DBG_RD_DESC_RAM 00000110: a726fffa 0029233a b86ef955 0029b166
UFS_UFS_DBG_RD_DESC_RAM 00000120: 320f0ce3 000e81e4 3ad7c22a 0027bec4
UFS_UFS_DBG_RD_DESC_RAM 00000130: bb59d781 002becf1 755abb91 00293cc7
UFS_UFS_DBG_RD_DESC_RAM 00000140: a2722c99 003c9eff 13f31639 002bb4db
UFS_UFS_DBG_RD_DESC_RAM 00000150: 08ab4fda 00006b23 a2e638e6 00218abb
UFS_UFS_DBG_RD_DESC_RAM 00000160: cbaaefd5 000a9af3 f83a3754 002a902c
UFS_UFS_DBG_RD_DESC_RAM 00000170: ebf336e9 000b1ca2 0e7b19ef 001a04d3
UFS_UFS_DBG_RD_DESC_RAM 00000180: 40000023 0005e12a ead2d0f1 00283aee
UFS_UFS_DBG_RD_DESC_RAM 00000190: a8f5ae8f 001c6188 fb09ea2b 003b2f4a
UFS_UFS_DBG_RD_DESC_RAM 000001a0: e5383e2a 0033ffde 8f4f229a 000b636b
UFS_UFS_DBG_RD_DESC_RAM 000001b0: 9fdfbd3c 0013ab96 fe4e16ba 00206c8e
UFS_UFS_DBG_RD_DESC_RAM 000001c0: 4a8ec4b6 000ccfa3 f283af16 0010eb18
UFS_UFS_DBG_RD_DESC_RAM 000001d0: 62126082 00299b58 46ab47cc 002cfef1
UFS_UFS_DBG_RD_DESC_RAM 000001e0: aa527ed6 003fb422 a92b35b2 0018328f
UFS_UFS_DBG_RD_DESC_RAM 000001f0: f864fb9b 0010cdb4 ebb0b0cc 003ea82e
UFS_UFS_DBG_RD_PRDT_RAM 00000000: 01700000 00009b3f c8cee2fd 0009ea97
UFS_UFS_DBG_RD_PRDT_RAM 00000010: ea5b24f6 000b2b69 e40a919e 00036be0
UFS_UFS_DBG_RD_PRDT_RAM 00000020: af556e2f 000f323d aeff7e6a 0008edfa
UFS_UFS_DBG_RD_PRDT_RAM 00000030: 3732fc69 000da4f3 c3eb170f 000a3dd2
UFS_UFS_DBG_RD_PRDT_RAM 00000040: 633dfce6 000a899d e5057d97 000aeade
UFS_UFS_DBG_RD_PRDT_RAM 00000050: 3aa2e736 0003b7a2 3366d6c7 0002b88c
UFS_UFS_DBG_RD_PRDT_RAM 00000060: fd937dbb 00085cbb 81f13deb 0000267d
UFS_UFS_DBG_RD_PRDT_RAM 00000070: 6e2ecd02 0000c225 a30b1e7e 000520e6
UFS_UFS_DBG_RD_PRDT_RAM 00000080: eb449c67 000b2380 2da23d73 000b6b77
UFS_UFS_DBG_RD_PRDT_RAM 00000090: d4ed6ef3 000a75ab 7a6e58f3 0000e3e3
UFS_UFS_DBG_RD_PRDT_RAM 000000a0: eada9737 0004d23e 0069bed2 0007a34e
UFS_UFS_DBG_RD_PRDT_RAM 000000b0: ffad4d8a 000eb58f f72bde37 00067c73
UFS_UFS_DBG_RD_PRDT_RAM 000000c0: 24100000 00017865 d66413bf 000b7be3
UFS_UFS_DBG_RD_PRDT_RAM 000000d0: 5f0e4bd8 000ced6c 994a332e 000f6108
UFS_UFS_DBG_RD_PRDT_RAM 000000e0: c4f6bedc 000f08aa 3c8d7fba 00019a7d
UFS_UFS_DBG_RD_PRDT_RAM 000000f0: 26a77938 000efaee 78000000 00017865
UFS_DBG_RD_REG_UAWM 00000000: 00000000 0000004a 00000000 0001fec0
UFS_DBG_RD_REG_UARM 00000000: 00000000 00000001 00000011 00000001
UFS_DBG_RD_REG_TXUC 00000000: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000010: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000020: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000030: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000040: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000050: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000060: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000070: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000080: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 00000090: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 000000a0: 00000000 00000000 00000000 00000000
UFS_DBG_RD_REG_TXUC 000000b0: 00000001 00000000 00000000 00000004

/*** system hangs here for several seconds, then reboots ***/

Format: 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.XF.1.2.2-00157-M8998LZB-1
S - IMAGE_VARIANT_STRING=Msm8998LA
S - OEM_IMAGE_VERSION_STRING=speedcore
S - Boot Interface: UFS
S - Secure Boot: Off
S - Boot Config @ 0x00786070 = 0x000002c1
S - JTAG ID @ 0x00786130 = 0x200620e1
S - OEM ID @ 0x00786138 = 0x00000000
S - Serial Number @ 0x00784138 = 0x13ab5626
S - OEM Config Row 0 @ 0x00784188 = 0x0000000000000000
S - OEM Config Row 1 @ 0x00784190 = 0x0000000000004000
S - Feature Config Row 0 @ 0x007841a0 = 0x0050300000000000
S - Feature Config Row 1 @ 0x007841a8 = 0x0000ffff00087fff
S - Core 0 Frequency, 1305 MHz
S - PBL Patch Ver: 1
B - 0 - PBL, Start
B - 9445 - bootable_media_detect_entry, Start
B - 55529 - bootable_media_detect_success, Start
B - 55536 - elf_loader_entry, Start
B - 56834 - auth_hash_seg_entry, Start
B - 57150 - auth_hash_seg_exit, Start
B - 107643 - elf_segs_hash_verify_entry, Start
B - 139027 - elf_segs_hash_verify_exit, Start
B - 139044 - auth_xbl_sec_hash_seg_entry, Start
B - 162946 - auth_xbl_sec_hash_seg_exit, Start
B - 162948 - xbl_sec_segs_hash_verify_entry, Start
B - 168357 - xbl_sec_segs_hash_verify_exit, Start
B - 168423 - PBL, End
B - 201818 - SBL1, Start
B - 323757 - boot_flash_init, Start
D - 30 - boot_flash_init, Delta
B - 326289 - sbl1_ddr_set_default_params, Start
D - 0 - sbl1_ddr_set_default_params, Delta
B - 334280 - boot_config_data_table_init, Start
D - 109525 - boot_config_data_table_init, Delta - (60 Bytes)
B - 448380 - CDT Version:3,Platform ID:8,Major ID:1,Minor ID:0,Subtype:1
B - 453077 - Image Load, Start
D - 579 - Auth Metadata
D - 580 - Segments hash check
D - 12322 - PMIC Image Loaded, Delta - (45640 Bytes)
B - 468724 - pm_device_init, Start
B - 476257 - PM: PON REASON: PM0=0x8000024000020001:0x0 PM1=0x8000084000080020:0x0 PM2=0x8000084000080020:0x0
B - 535183 - PM: SET_VAL:Skip
D - 62738 - pm_device_init, Delta
B - 537105 - pm_driver_init, Start
D - 5215 - pm_driver_init, Delta
B - 545614 - pm_sbl_chg_init, Start
B - 547902 - PM: Trigger FG IMA Reset
B - 551470 - PM: Trigger FG IMA Reset.Completed
B - 556960 - PM: BOOTUP, Debug Board being used
D - 11529 - pm_sbl_chg_init, Delta
B - 563914 - vsense_init, Start
D - 0 - vsense_init, Delta
B - 621071 - Pre_DDR_clock_init, Start
D - 213 - Pre_DDR_clock_init, Delta
D - 0 - sbl1_ddr_set_params, Delta
B - 630465 - do_ddr_training, Start
D - 0 - do_ddr_training, Delta
B - 655628 - clock_init, Start
D - 335 - clock_init, Delta
B - 658556 - Image Load, Start
D - 2074 - APDP Image Loaded, Delta - (0 Bytes)
B - 868914 - usb: chgr - SDP_CHARGER
B - 869524 - Image Load, Start
D - 6496 - Auth Metadata
D - 4270 - Segments hash check
D - 15982 - XBLRamDump Image Loaded, Delta - (314564 Bytes)
B - 976213 - MDPDetectPanel: DSC major:0 minor:0 invalid, using default

B - 977372 - DisplayDxe: Resolution 1440x2560 (1 intf)

B - 10088637 - usb: init start
B - 10089034 - usb: ss_lane_2nd
B - 10093426 - usb: VBUS High!
B - 10393912 - usb: HIGH , 0x900e
B - 10521768 - usb: HIGH , 0x900e
B - 10616867 - usb: ENUM success

2019-02-05 18:02:11

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 05/02/2019 18:24, Marc Gonzalez wrote:

> /*** system hangs here for several seconds, then reboots ***/

Silly me. The system crashes in ufshcd_dump_regs() which is a bug
I fixed myself. Once I cherry-pick the appropriate fix, the board
no longer reboots, but UFS init does fail.

Full boot log here:
https://pastebin.ubuntu.com/p/KwpRnWMFw5/

In any case, it's obvious that disabling vccq on this system is
a mistake. How would you solve the problem? (A quirk on top of a
quirk sounds silly.)

Regards.

2019-02-05 18:59:53

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On Tue, Feb 5, 2019 at 9:52 AM Marc Gonzalez <[email protected]> wrote:
>
> On 05/02/2019 18:24, Marc Gonzalez wrote:
>
> > /*** system hangs here for several seconds, then reboots ***/
>
> Silly me. The system crashes in ufshcd_dump_regs() which is a bug
> I fixed myself. Once I cherry-pick the appropriate fix, the board
> no longer reboots, but UFS init does fail.
>
> Full boot log here:
> https://pastebin.ubuntu.com/p/KwpRnWMFw5/
>
> In any case, it's obvious that disabling vccq on this system is
> a mistake. How would you solve the problem? (A quirk on top of a
> quirk sounds silly.)
>

I think Bjorn is right that this whole quirk seems to be compensating
for an incorrectly specified device tree (one that specifies
vccq-supply but that doesn't go anywhere).... though maybe this was
made to support boards with footprint-compatible UFS parts from
different vendors? Like the same DT is used for two different SKUs,
one which stamps down a UFS part that uses VCCQ, and another that
doesn't use it, though the wire is there.

But the revert itself shouldn't really fix anything for you Marc,
should it? It looks like this quirk turns on for Samsung and SKHynix
parts, which presumably just don't use VCCQ. So maybe your device tree
doesn't match your schematics, where the DT's vccq supply is actually
the vccq2 supply going into the UFS chip?
-Evan

2019-02-05 20:10:23

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/5/2019 11:19 AM, Evan Green wrote:
> On Tue, Feb 5, 2019 at 9:52 AM Marc Gonzalez <[email protected]> wrote:
>>
>> On 05/02/2019 18:24, Marc Gonzalez wrote:
>>
>>> /*** system hangs here for several seconds, then reboots ***/
>>
>> Silly me. The system crashes in ufshcd_dump_regs() which is a bug
>> I fixed myself. Once I cherry-pick the appropriate fix, the board
>> no longer reboots, but UFS init does fail.
>>
>> Full boot log here:
>> https://pastebin.ubuntu.com/p/KwpRnWMFw5/
>>
>> In any case, it's obvious that disabling vccq on this system is
>> a mistake. How would you solve the problem? (A quirk on top of a
>> quirk sounds silly.)
>>
>
> I think Bjorn is right that this whole quirk seems to be compensating
> for an incorrectly specified device tree (one that specifies
> vccq-supply but that doesn't go anywhere).... though maybe this was
> made to support boards with footprint-compatible UFS parts from
> different vendors? Like the same DT is used for two different SKUs,
> one which stamps down a UFS part that uses VCCQ, and another that
> doesn't use it, though the wire is there.

This doesn't make sense from a DT perspective, as I understand it. If
some board has different configurations (say different display panels),
FW is supposed to somehow detect that, and edit the DT before passing
the DT off to Linux.

In your example, if SKU1 has a UFS part that needs VCCQ, and SKU2 does
not, then FW should somehow query the HW, determine which SKU the device
is, and patch vccq in or out of the DT as necessary.

>
> But the revert itself shouldn't really fix anything for you Marc,
> should it? It looks like this quirk turns on for Samsung and SKHynix
> parts, which presumably just don't use VCCQ. So maybe your device tree
> doesn't match your schematics, where the DT's vccq supply is actually
> the vccq2 supply going into the UFS chip?

I see the same issue on the reference device, the MSM8998 MTP. The
schematics for that indicate there is both vccq and vccq2, at different
voltages. vccq goes to both the UFS block, and the phy.

Digging into things, the problem doesn't seem to be that vccq is off -
the phy will keep it on. However, the load from the phy is minimal.
What really loads the regulator is the UFS block. Without the load from
the UFS block, the regulator is likely in a low power mode.

I cannot tell if vccq is routed to the actual storage chip based on the
documentation I see, however I can tell that it powers blocks within the
host controller block which assist the controller in communicating with
the storage chip. These blocks consume quite a bit of power, so likely,
if the regulator load is not properly accounted for, these blocks
brownout, and communication with the storage chip is broken.

The schematics do refer to this input to the UFS block as "vccq".

So, at a minimum, the host controller itself consumes vccq, and
therefore it is unfair to disable vccq solely based on the UFS storage
chip that happens to be connected to the controller.

So, we've got two boards that are actually harmed by this quirk. I've
not seen anything indicating what harm happens without this quirk. Can
someone indicate what that is?

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-05 20:16:04

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On Tue 05 Feb 02:52 PST 2019, Alim Akhtar wrote:

> Hi Bjorn,
>
> On 05/02/19 11:57 AM, Bjorn Andersson wrote:
> > On Mon 04 Feb 20:58 PST 2019, Alim Akhtar wrote:
> >
> >> Hi Marc,
> >>
> >> On 04/02/19 11:12 PM, Marc Gonzalez wrote:
> >>> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
> >>>
> >>> Calling ufshcd_set_vccq_rail_unused hangs my system.
> >>> It seems vccq is not *not* needed.
> >>>
> >>> Signed-off-by: Marc Gonzalez <[email protected]>
> >>> ---
> >>
> >> AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
> >> either floating or connected to Ground, at least on the devices that I
> >> have worked on).
> >
> > But why does such system define a vccq-supply? If the system doesn't
> > have a regulator connected to VCCQ, then the UFS driver shouldn't be
> > told that there is one. And if VCCQ is optional the UFS driver should
> > support the fact that this regulator might not be supplied (i.e. call
> > regulator_get_optional() and handle the error indicating that the supply
> > isn't specified).
> >
> As per JESD220C, chapter 6.1, it does says "VCCQ - Supply voltage used
> typically for the memory controller and optionally for the PHY
> interface, the memory IO, and any other internal very low voltage block"
> And we have VCCQ2 - which serve the pretty much same purpose. The
> voltage range for VCCQ and VCCQ2 are different, VCCQ has a lower voltage
> suitable to some low voltage block inside UFS device. I think this is
> design consideration which allow some vendor to use one less physical
> pin may be. And also depends on the voltage requirements of some of the
> internal circuit.
>

This looks to me that you are required to have a VCCQ. But you said that
you do not have a regulator supplying VCCQ on your board, and if that
really is the case then you should not specify vccq-supply.

The patch Marc is reverting states that for devices that does not have
VCCQ connected, some unrelated regulator should be assigned to the UFS
driver so that it can turn it off.


If you have a regulator connected to VCCQ then it should go in
vccq-supply, if you have a regulator connected to VCCQ2 the is should go
in vccq2-supply. If you don't have these pins connected then there
shouldn't be any regulators specified here!

Regards,
Bjorn

>
> > Regards,
> > Bjorn
> >
> >> You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
> >> quirks, in that case VCCQ regulator should having been disabled.
> >> So you mean your system hanged because vccq regulator got disabled?
> >>
> >>> drivers/scsi/ufs/ufs.h | 1 -
> >>> drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
> >>> 2 files changed, 4 insertions(+), 56 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> >>> index dd65fea07687..7da7318eb6a6 100644
> >>> --- a/drivers/scsi/ufs/ufs.h
> >>> +++ b/drivers/scsi/ufs/ufs.h
> >>> @@ -514,7 +514,6 @@ struct ufs_vreg {
> >>> struct regulator *reg;
> >>> const char *name;
> >>> bool enabled;
> >>> - bool unused;
> >>> int min_uV;
> >>> int max_uV;
> >>> int min_uA;
> >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >>> index 9ba7671b84f8..8b9a01073d62 100644
> >>> --- a/drivers/scsi/ufs/ufshcd.c
> >>> +++ b/drivers/scsi/ufs/ufshcd.c
> >>> @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
> >>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> >>> bool skip_ref_clk);
> >>> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> >>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
> >>> static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
> >>> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
> >>> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
> >>> @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> >>> ufs_fixup_device_setup(hba, &card);
> >>> ufshcd_tune_unipro_params(hba);
> >>>
> >>> - ret = ufshcd_set_vccq_rail_unused(hba,
> >>> - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
> >>> - if (ret)
> >>> - goto out;
> >>> -
> >>> /* UFS device is also active now */
> >>> ufshcd_set_ufs_dev_active(hba);
> >>> ufshcd_force_reset_auto_bkops(hba);
> >>> @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
> >>> static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
> >>> struct ufs_vreg *vreg)
> >>> {
> >>> - if (!vreg)
> >>> - return 0;
> >>> - else if (vreg->unused)
> >>> - return 0;
> >>> - else
> >>> - return ufshcd_config_vreg_load(hba->dev, vreg,
> >>> - UFS_VREG_LPM_LOAD_UA);
> >>> + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
> >>> }
> >>>
> >>> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> >>> struct ufs_vreg *vreg)
> >>> {
> >>> - if (!vreg)
> >>> - return 0;
> >>> - else if (vreg->unused)
> >>> - return 0;
> >>> - else
> >>> - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> >>> + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
> >>> }
> >>>
> >>> static int ufshcd_config_vreg(struct device *dev,
> >>> @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
> >>> {
> >>> int ret = 0;
> >>>
> >>> - if (!vreg)
> >>> - goto out;
> >>> - else if (vreg->enabled || vreg->unused)
> >>> + if (!vreg || vreg->enabled)
> >>> goto out;
> >>>
> >>> ret = ufshcd_config_vreg(dev, vreg, true);
> >>> @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
> >>> {
> >>> int ret = 0;
> >>>
> >>> - if (!vreg)
> >>> - goto out;
> >>> - else if (!vreg->enabled || vreg->unused)
> >>> + if (!vreg || !vreg->enabled)
> >>> goto out;
> >>>
> >>> ret = regulator_disable(vreg->reg);
> >>> @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
> >>> return 0;
> >>> }
> >>>
> >>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
> >>> -{
> >>> - int ret = 0;
> >>> - struct ufs_vreg_info *info = &hba->vreg_info;
> >>> -
> >>> - if (!info)
> >>> - goto out;
> >>> - else if (!info->vccq)
> >>> - goto out;
> >>> -
> >>> - if (unused) {
> >>> - /* shut off the rail here */
> >>> - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
> >>> - /*
> >>> - * Mark this rail as no longer used, so it doesn't get enabled
> >>> - * later by mistake
> >>> - */
> >>> - if (!ret)
> >>> - info->vccq->unused = true;
> >>> - } else {
> >>> - /*
> >>> - * rail should have been already enabled hence just make sure
> >>> - * that unused flag is cleared.
> >>> - */
> >>> - info->vccq->unused = false;
> >>> - }
> >>> -out:
> >>> - return ret;
> >>> -}
> >>> -
> >>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> >>> bool skip_ref_clk)
> >>> {
> >>>
> >
> >

2019-02-06 10:59:12

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] arm64: dts: qcom: msm8998: Add UFS nodes

On 04/02/2019 18:36, Marc Gonzalez wrote:

> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 6f4f4b79853b..831af20143da 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -711,6 +711,69 @@
> redistributor-stride = <0x0 0x20000>;
> interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> };
> +
> + ufshc: ufshc@1da4000 {
> + compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
> + "jedec,ufs-2.0";
> + reg = <0x01da4000 0x2500>;
> + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&ufsphy_lanes>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + power-domains = <&gcc UFS_GDSC>;
> +
> + clock-names =
> + "core_clk",
> + "bus_aggr_clk",
> + "iface_clk",
> + "core_clk_unipro",
> + "ref_clk",
> + "tx_lane0_sync_clk",
> + "rx_lane0_sync_clk",
> + "rx_lane1_sync_clk";
> + clocks =
> + <&gcc GCC_UFS_AXI_CLK>,
> + <&gcc GCC_AGGRE1_UFS_AXI_CLK>,
> + <&gcc GCC_UFS_AHB_CLK>,
> + <&gcc GCC_UFS_UNIPRO_CORE_CLK>,
> + <&rpmcc RPM_SMD_LN_BB_CLK1>,
> + <&gcc GCC_UFS_TX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_RX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_RX_SYMBOL_1_CLK>;
> + freq-table-hz =
> + <50000000 200000000>,
> + <0 0>,
> + <0 0>,
> + <37500000 150000000>,
> + <0 0>,
> + <0 0>,
> + <0 0>,
> + <0 0>;
> +
> + resets = <&gcc GCC_UFS_BCR>;
> + reset-names = "rst";
> + };

In https://patchwork.kernel.org/patch/10657699/
Bjorn mentioned that the resets prop are useless upstream.

Can, Vivek, Dov:

What is the status on the "scsi: ufs: Add core reset support" patch?
https://patchwork.kernel.org/patch/10652705/

Regards.

2019-02-06 12:56:12

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] phy: qcom-qmp: Add QMP UFS PHY support for msm8998

Hi,

On 04/02/19 11:40 PM, Jeffrey Hugo wrote:
> On 2/4/2019 10:39 AM, Marc Gonzalez wrote:
>> Use same init sequence as sdm845.
>>
>> Signed-off-by: Marc Gonzalez <[email protected]>
>> ---
>
> Reviewed-by: Jeffrey Hugo <[email protected]>

I don't seem to have the dt-binding patch in my inbox. Can you resend them please?

Thanks
Kishon

>
>>   drivers/phy/qualcomm/phy-qcom-qmp.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> index b4006818e1b6..39b9c31b67d0 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> @@ -1735,6 +1735,9 @@ static const struct of_device_id
>> qcom_qmp_phy_of_match_table[] = {
>>       }, {
>>           .compatible = "qcom,msm8996-qmp-usb3-phy",
>>           .data = &msm8996_usb3phy_cfg,
>> +    }, {
>> +        .compatible = "qcom,msm8998-qmp-ufs-phy",
>> +        .data = &sdm845_ufsphy_cfg,
>>       }, {
>>           .compatible = "qcom,ipq8074-qmp-pcie-phy",
>>           .data = &ipq8074_pciephy_cfg,
>>
>
>

2019-02-06 13:02:04

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] phy: qcom-qmp: Add QMP UFS PHY support for msm8998

On 06/02/2019 13:39, Kishon Vijay Abraham I wrote:

> I don't seem to have the dt-binding patch in my inbox. Can you resend them please?

Jeffrey pointed out a few deficiencies in the series.

I'll CC you on the entire upcoming v4 series.

Regards.

2019-02-06 15:27:22

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 05/02/2019 18:51, Marc Gonzalez wrote:

> On 05/02/2019 18:24, Marc Gonzalez wrote:
>
> Silly me. The system crashes in ufshcd_dump_regs() which is a bug
> I fixed myself. Once I cherry-pick the appropriate fix, the board
> no longer reboots, but UFS init does fail.
>
> Full boot log here:
> https://pastebin.ubuntu.com/p/KwpRnWMFw5/

Here's a better failure log, with timestamps:

[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x51af8014]
[ 0.000000] Linux version 5.0.0-rc5-next-20190206 (mgonzalez@venus) (gcc version 7.3.1 20180425 [linaro-7.3-2018.05 revision d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05)) #19 SMP PREEMPT Wed Feb 6 15:42:45 CET 2019
[ 0.000000] Machine model: Qualcomm Technologies, Inc. MSM8998 v1 MTP
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.000000] On node 0 totalpages: 1028544
[ 0.000000] DMA32 zone: 8192 pages used for memmap
[ 0.000000] DMA32 zone: 0 pages reserved
[ 0.000000] DMA32 zone: 511488 pages, LIFO batch:63
[ 0.000000] Normal zone: 8079 pages used for memmap
[ 0.000000] Normal zone: 517056 pages, LIFO batch:63
[ 0.000000] psci: probing for conduit method from DT.
[ 0.000000] psci: PSCIv1.0 detected in firmware.
[ 0.000000] psci: Using standard PSCI v0.2 function IDs
[ 0.000000] psci: MIGRATE_INFO_TYPE not supported.
[ 0.000000] psci: SMC Calling Convention v1.0
[ 0.000000] random: get_random_bytes called from start_kernel+0xa8/0x470 with crng_init=0
[ 0.000000] percpu: Embedded 22 pages/cpu @(____ptrval____) s50184 r8192 d31736 u90112
[ 0.000000] pcpu-alloc: s50184 r8192 d31736 u90112 alloc=22*4096
[ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0] 4 [0] 5 [0] 6 [0] 7
[ 0.000000] Detected VIPT I-cache on CPU0
[ 0.000000] CPU features: detected: Kernel page table isolation (KPTI)
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 1012273
[ 0.000000] Kernel command line: ignore_loglevel androidboot.bootdevice=1da4000.ufshc androidboot.serialno=53733c35 androidboot.baseband=apq mdss_mdp.panel=1:hdmi:16
[ 0.000000] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes)
[ 0.000000] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes)
[ 0.000000] software IO TLB: mapped [mem 0xfbfff000-0xfffff000] (64MB)
[ 0.000000] Memory: 3955464K/4114176K available (3262K kernel code, 410K rwdata, 944K rodata, 6016K init, 1161K bss, 158712K reserved, 0K cma-reserved)
[ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=1
[ 0.000000] ftrace: allocating 12605 entries in 50 pages
[ 0.000000] rcu: Preemptible hierarchical RCU implementation.
[ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=256 to nr_cpu_ids=8.
[ 0.000000] Tasks RCU enabled.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
[ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=8
[ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[ 0.000000] REMAP: PA=17a00000 VA=ffffff8010040000 SIZE=10000
[ 0.000000] REMAP: PA=17b00000 VA=ffffff8010d00000 SIZE=100000
[ 0.000000] GICv3: Distributor has no Range Selector support
[ 0.000000] GICv3: no VLPI support, no direct LPI support
[ 0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000017b00000
[ 0.000000] ITS: No ITS available, not enabling LPIs
[ 0.000000] REMAP: PA=17920000 VA=ffffff8010005000 SIZE=1000
[ 0.000000] REMAP: PA=17921000 VA=ffffff801000d000 SIZE=1000
[ 0.000000] REMAP: PA=17921000 VA=ffffff8010015000 SIZE=1000
[ 0.000000] arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
[ 0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
[ 0.000003] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
[ 0.000061] Console: colour dummy device 80x25
[ 0.000405] printk: console [tty0] enabled
[ 0.000428] Calibrating delay loop (skipped), value calculated using timer frequency.. 38.40 BogoMIPS (lpj=76800)
[ 0.000444] pid_max: default: 32768 minimum: 301
[ 0.000542] Mount-cache hash table entries: 8192 (order: 4, 65536 bytes)
[ 0.000563] Mountpoint-cache hash table entries: 8192 (order: 4, 65536 bytes)
[ 0.000864] *** VALIDATE proc ***
[ 0.023878] ASID allocator initialised with 32768 entries
[ 0.031878] rcu: Hierarchical SRCU implementation.
[ 0.051914] smp: Bringing up secondary CPUs ...
[ 0.086062] Detected VIPT I-cache on CPU1
[ 0.086092] GICv3: CPU1: found redistributor 1 region 0:0x0000000017b20000
[ 0.086135] CPU1: Booted secondary processor 0x0000000001 [0x51af8014]
[ 0.118156] Detected VIPT I-cache on CPU2
[ 0.118178] GICv3: CPU2: found redistributor 2 region 0:0x0000000017b40000
[ 0.118218] CPU2: Booted secondary processor 0x0000000002 [0x51af8014]
[ 0.150457] Detected VIPT I-cache on CPU3
[ 0.150481] GICv3: CPU3: found redistributor 3 region 0:0x0000000017b60000
[ 0.150521] CPU3: Booted secondary processor 0x0000000003 [0x51af8014]
[ 0.183055] Detected VIPT I-cache on CPU4
[ 0.183082] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU4: 0x00000000101122
[ 0.183108] CPU features: Unsupported CPU feature variation detected.
[ 0.183141] GICv3: CPU4: found redistributor 100 region 0:0x0000000017b80000
[ 0.183217] CPU4: Booted secondary processor 0x0000000100 [0x51af8001]
[ 0.215157] Detected VIPT I-cache on CPU5
[ 0.215182] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU5: 0x00000000101122
[ 0.215236] GICv3: CPU5: found redistributor 101 region 0:0x0000000017ba0000
[ 0.215308] CPU5: Booted secondary processor 0x0000000101 [0x51af8001]
[ 0.247502] Detected VIPT I-cache on CPU6
[ 0.247528] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU6: 0x00000000101122
[ 0.247583] GICv3: CPU6: found redistributor 102 region 0:0x0000000017bc0000
[ 0.247656] CPU6: Booted secondary processor 0x0000000102 [0x51af8001]
[ 0.279824] Detected VIPT I-cache on CPU7
[ 0.279850] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU7: 0x00000000101122
[ 0.279908] GICv3: CPU7: found redistributor 103 region 0:0x0000000017be0000
[ 0.279981] CPU7: Booted secondary processor 0x0000000103 [0x51af8001]
[ 0.280178] smp: Brought up 1 node, 8 CPUs
[ 0.280396] SMP: Total of 8 processors activated.
[ 0.280406] CPU features: detected: GIC system register CPU interface
[ 0.280418] CPU features: detected: 32-bit EL0 Support
[ 0.280429] CPU features: detected: CRC32 instructions
[ 0.823309] CPU: All CPU(s) started at EL1
[ 0.823361] alternatives: patching kernel code
[ 0.824150] devtmpfs: initialized
[ 0.827562] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[ 0.827585] futex hash table entries: 2048 (order: 5, 131072 bytes)
[ 0.827708] pinctrl core: initialized pinctrl subsystem
[ 0.829449] vdso: 2 pages (1 code @ (____ptrval____), 1 data @ (____ptrval____))
[ 0.829523] DMA: preallocated 256 KiB pool for atomic allocations
[ 0.830318] REMAP: PA=01f40000 VA=ffffff8011000000 SIZE=20000
[ 0.830863] REMAP: PA=86000000 VA=ffffff8011200000 SIZE=200000
[ 0.832055] REMAP: PA=00100000 VA=ffffff8011100000 SIZE=b0000
[ 0.839431] REMAP: PA=03400000 VA=ffffff8012000000 SIZE=c00000
[ 0.840662] REMAP: PA=17911000 VA=ffffff8010025000 SIZE=1000
[ 0.848961] SCSI subsystem initialized
[ 0.849218] REMAP: PA=00778000 VA=ffffff80116f8000 SIZE=7000
[ 0.850312] clocksource: Switched to clocksource arch_sys_counter
[ 0.859512] s1: supplied by vph_pwr
[ 0.860008] s2: supplied by vph_pwr
[ 0.860291] s3: supplied by vph_pwr
[ 0.860348] s3: Bringing 0uV into 1352000-1352000uV
[ 0.860566] s4: supplied by vph_pwr
[ 0.860699] s4: Bringing 0uV into 1800000-1800000uV
[ 0.860885] s5: supplied by vph_pwr
[ 0.860933] s5: Bringing 0uV into 1904000-1904000uV
[ 0.861115] s6: supplied by vph_pwr
[ 0.861337] s7: supplied by vph_pwr
[ 0.861394] s7: Bringing 0uV into 900000-900000uV
[ 0.861639] s8: supplied by vph_pwr
[ 0.861848] s9: supplied by vph_pwr
[ 0.862055] s10: supplied by vph_pwr
[ 0.862436] s11: supplied by vph_pwr
[ 0.862651] s12: supplied by vph_pwr
[ 0.862903] s13: supplied by vph_pwr
[ 0.863154] l1: supplied by s7
[ 0.863304] l1: Bringing 0uV into 880000-880000uV
[ 0.863518] l2: supplied by s3
[ 0.863573] l2: Bringing 0uV into 1200000-1200000uV
[ 0.863763] l3: supplied by s7
[ 0.863820] l3: Bringing 0uV into 1000000-1000000uV
[ 0.864049] l4: supplied by s7
[ 0.864370] l5: supplied by s7
[ 0.864421] l5: Bringing 0uV into 800000-800000uV
[ 0.864654] l6: supplied by s5
[ 0.864707] l6: Bringing 0uV into 1808000-1808000uV
[ 0.864933] l7: supplied by s5
[ 0.864994] l7: Bringing 0uV into 1800000-1800000uV
[ 0.865269] l8: supplied by s3
[ 0.865329] l8: Bringing 0uV into 1200000-1200000uV
[ 0.865560] l9: Bringing 0uV into 1808000-1808000uV
[ 0.865805] l10: Bringing 0uV into 1808000-1808000uV
[ 0.866041] l11: supplied by s7
[ 0.866098] l11: Bringing 0uV into 1000000-1000000uV
[ 0.866455] l12: supplied by s5
[ 0.866590] l12: Bringing 0uV into 1800000-1800000uV
[ 0.866845] l13: Bringing 0uV into 1808000-1808000uV
[ 0.867109] l14: supplied by s5
[ 0.867178] l14: Bringing 0uV into 1880000-1880000uV
[ 0.867438] l15: supplied by s5
[ 0.867489] l15: Bringing 0uV into 1800000-1800000uV
[ 0.867756] l16: Bringing 0uV into 2704000-2704000uV
[ 0.868118] l17: supplied by s3
[ 0.868200] l17: Bringing 0uV into 1304000-1304000uV
[ 0.868505] l18: Bringing 0uV into 2704000-2704000uV
[ 0.868821] l19: Bringing 0uV into 3008000-3008000uV
[ 0.869127] l20: Bringing 0uV into 2960000-2960000uV
[ 0.869486] l21: Bringing 0uV into 2960000-2960000uV
[ 0.869867] l22: Bringing 0uV into 2864000-2864000uV
[ 0.870433] l23: Bringing 0uV into 3312000-3312000uV
[ 0.870860] l24: Bringing 0uV into 3088000-3088000uV
[ 0.871290] l25: Bringing 0uV into 3104000-3104000uV
[ 0.871742] l26: supplied by s3
[ 0.871793] l26: Bringing 0uV into 1200000-1200000uV
[ 0.872233] l27: supplied by s7
[ 0.872808] l28: Bringing 0uV into 3008000-3008000uV
[ 0.873285] lvs1: supplied by s4
[ 0.873825] lvs2: supplied by s4
[ 0.875080] bob: supplied by vph_pwr
[ 0.875151] bob: Bringing 0uV into 3312000-3312000uV
[ 1.183716] workingset: timestamp_bits=62 max_order=20 bucket_order=0
[ 1.192148] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
[ 1.193995] REMAP: PA=0c010000 VA=ffffff801002d000 SIZE=18c
[ 1.195073] REMAP: PA=0c010200 VA=ffffff8010035200 SIZE=128
[ 1.195106] REMAP: PA=0c010400 VA=ffffff801003d400 SIZE=200
[ 1.195129] REMAP: PA=0c010c00 VA=ffffff8010051c00 SIZE=20c
[ 1.195152] REMAP: PA=0c010600 VA=ffffff8010053600 SIZE=128
[ 1.195179] REMAP: PA=0c010800 VA=ffffff8010055800 SIZE=200
[ 1.196797] qcom-qmp-phy c010000.phy: Registered Qcom-QMP phy
[ 1.197092] REMAP: PA=01da7000 VA=ffffff801005d000 SIZE=18c
[ 1.197411] REMAP: PA=01da7400 VA=ffffff8010065400 SIZE=128
[ 1.197440] REMAP: PA=01da7600 VA=ffffff801006d600 SIZE=1fc
[ 1.197467] REMAP: PA=01da7c00 VA=ffffff8010075c00 SIZE=1dc
[ 1.197492] REMAP: PA=01da7800 VA=ffffff801007d800 SIZE=128
[ 1.197525] REMAP: PA=01da7a00 VA=ffffff8010c1ca00 SIZE=1fc
[ 1.197846] qcom-qmp-phy 1da7000.phy: Registered Qcom-QMP phy
[ 1.202503] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 1.204233] msm_serial c1b0000.serial: msm_serial: detected port #0
[ 1.204305] msm_serial c1b0000.serial: uartclk = 1843200
[ 1.204485] REMAP: PA=0c1b0000 VA=ffffff8010c1e000 SIZE=1000
[ 1.204510] c1b0000.serial: ttyMSM0 at MMIO 0xc1b0000 (irq = 17, base_baud = 115200) is a MSM
[ 1.204674] msm_serial: console setup on port #0
[ 2.240898] printk: console [ttyMSM0] enabled
[ 2.245990] msm_serial: driver initialized
[ 2.253655] REMAP: PA=01da4000 VA=ffffff8010e04000 SIZE=2500
[ 2.254403] ufshcd-qcom 1da4000.ufshc: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled
[ 2.260500] l20: supplied by bob
[ 2.270704] regulator_set_load: vdd_l20_l24 = 750000 uA
[ 2.271116] regulator_set_load: vdd_l26 = 560000 uA
[ 2.273731] regulator_set_load: vdd_s4 = 750000 uA
[ 2.280015] scsi host0: ufshcd
[ 2.319444] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_async_scan | async_run_entry_fn | process_one_work | worker_thread | kthread | ret_from_fork |
[ 2.319830] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 2.332368] l9: supplied by bob
[ 2.345241] l10: supplied by bob
[ 2.348308] l13: supplied by bob
[ 2.351688] l16: supplied by bob
[ 2.354913] l18: supplied by bob
[ 2.358104] l19: supplied by bob
[ 2.361331] l21: supplied by bob
[ 2.364541] l22: supplied by bob
[ 2.367826] l23: supplied by bob
[ 2.370962] l24: supplied by bob
[ 2.374154] l25: supplied by bob
[ 2.377395] l28: supplied by bob
[ 2.405734] regulator_disable: ENTER vdd_l26
[ 2.405958] regulator_disable: EXIT vdd_l26
[ 2.406032] regulator_set_load: vdd_l26 = 0 uA
[ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
[ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
[ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
[ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
[ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
[ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
[ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
[ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
[ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
[ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 37.074399] IGNORE ufshcd_print_host_state
[ 37.079128] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.083144] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 37.104851] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.117239] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 37.598330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 37.598390] IGNORE ufshcd_print_host_state
[ 37.603088] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.607135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 37.628846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.641231] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 38.122332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 38.122392] IGNORE ufshcd_print_host_state
[ 38.127084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.131135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 38.152846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.165228] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 38.646331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 38.646390] IGNORE ufshcd_print_host_state
[ 38.651083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.655135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 38.676845] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.689230] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 39.170331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 39.170391] IGNORE ufshcd_print_host_state
[ 39.175085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.179135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TrX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 39.200847] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.213232] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 39.232940] ufshcd_print_trs | __ufshcd_transfer_req_compl | ufshcd_transfer_req_compl | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.241197] ufshcd-qcom 1da4000.ufshc: UPIU[24] - issue time 15538577 us
[ 39.262687] ufshcd-qcom 1da4000.ufshc: UPIU[24] - complete time 0 us
[ 39.269630] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Transfer Request Descriptor phys@0x1774ba300
[ 39.276009] UPIU TRD: 00000000: 15000000 00000000 0000000f 00000000
[ 39.284387] UPIU TRD: 00000010: 77612000 00000001 00800080 01000001
[ 39.290545] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Request UPIU phys@0x177612000
[ 39.296810] UPIU REQ: 00000000: 18d04001 00000000 00000000 24000000
[ 39.304090] UPIU REQ: 00000010: 00000012 00000024 00000000 00000000
[ 39.310338] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Response UPIU phys@0x177612200
[ 39.316603] UPIU RSP: 00000000: 00000000 00000000 00000000 00000000
[ 39.324233] UPIU RSP: 00000010: 00000000 00000000 00000000 00000000
[ 39.330220] UPIU RSP: 00000020: 00000000 00000000 00000000 00000000
[ 39.336463] UPIU RSP: 00000030: 00000000
[ 39.342703] ufshcd-qcom 1da4000.ufshc: UPIU[24] - PRDT - 1 entries phys@0x177612400
[ 39.346903] UPIU PRDT: 00000000: 77625000 00000001 00000000 00000023
[ 39.474380] ufshcd-qcom 1da4000.ufshc: __ufshcd_issue_tm_cmd: task management cmd 0x08 timed-out
[ 39.474482] ufshcd-qcom 1da4000.ufshc: ufshcd_eh_device_reset_handler: failed with err -110
[ 39.942333] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 39.942391] IGNORE ufshcd_print_host_state
[ 39.947085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 39.951126] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 39.971806] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 39.984356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 40.014355] random: fast init done
[ 40.462330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 40.462392] IGNORE ufshcd_print_host_state
[ 40.467084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.471125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 40.491805] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.504355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 40.982332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 40.982391] IGNORE ufshcd_print_host_state
[ 40.987084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.991125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 41.011807] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.024355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 41.502331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 41.502390] IGNORE ufshcd_print_host_state
[ 41.507083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.511124] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 41.531804] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.544355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 42.022330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 42.022391] IGNORE ufshcd_print_host_state
[ 42.027084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 42.031125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 42.051801] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 42.064356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 42.083047] scsi 0:0:0:49488: Device offlined - not ready after error recovery
[ 42.101655] Freeing unused kernel memory: 6016K

2019-02-06 15:28:50

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

[ Google, stop making email so hard. No, this is not spam, you twat of a Bayesian filter ]

On 05/02/2019 18:51, Marc Gonzalez wrote:

> On 05/02/2019 18:24, Marc Gonzalez wrote:
>
> Silly me. The system crashes in ufshcd_dump_regs() which is a bug
> I fixed myself. Once I cherry-pick the appropriate fix, the board
> no longer reboots, but UFS init does fail.
>
> Full boot log here:
> https://pastebin.ubuntu.com/p/KwpRnWMFw5/

Here's a better failure log, with timestamps:

[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x51af8014]
[ 0.000000] Linux version 5.0.0-rc5-next-20190206 (mgonzalez@venus) (gcc version 7.3.1 20180425 [linaro-7.3-2018.05 revision d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05)) #19 SMP PREEMPT Wed Feb 6 15:42:45 CET 2019
[ 0.000000] Machine model: Qualcomm Technologies, Inc. MSM8998 v1 MTP
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.000000] On node 0 totalpages: 1028544
[ 0.000000] DMA32 zone: 8192 pages used for memmap
[ 0.000000] DMA32 zone: 0 pages reserved
[ 0.000000] DMA32 zone: 511488 pages, LIFO batch:63
[ 0.000000] Normal zone: 8079 pages used for memmap
[ 0.000000] Normal zone: 517056 pages, LIFO batch:63
[ 0.000000] psci: probing for conduit method from DT.
[ 0.000000] psci: PSCIv1.0 detected in firmware.
[ 0.000000] psci: Using standard PSCI v0.2 function IDs
[ 0.000000] psci: MIGRATE_INFO_TYPE not supported.
[ 0.000000] psci: SMC Calling Convention v1.0
[ 0.000000] random: get_random_bytes called from start_kernel+0xa8/0x470 with crng_init=0
[ 0.000000] percpu: Embedded 22 pages/cpu @(____ptrval____) s50184 r8192 d31736 u90112
[ 0.000000] pcpu-alloc: s50184 r8192 d31736 u90112 alloc=22*4096
[ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0] 4 [0] 5 [0] 6 [0] 7
[ 0.000000] Detected VIPT I-cache on CPU0
[ 0.000000] CPU features: detected: Kernel page table isolation (KPTI)
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 1012273
[ 0.000000] Kernel command line: ignore_loglevel androidboot.bootdevice=1da4000.ufshc androidboot.serialno=53733c35 androidboot.baseband=apq mdss_mdp.panel=1:hdmi:16
[ 0.000000] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes)
[ 0.000000] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes)
[ 0.000000] software IO TLB: mapped [mem 0xfbfff000-0xfffff000] (64MB)
[ 0.000000] Memory: 3955464K/4114176K available (3262K kernel code, 410K rwdata, 944K rodata, 6016K init, 1161K bss, 158712K reserved, 0K cma-reserved)
[ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=1
[ 0.000000] ftrace: allocating 12605 entries in 50 pages
[ 0.000000] rcu: Preemptible hierarchical RCU implementation.
[ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=256 to nr_cpu_ids=8.
[ 0.000000] Tasks RCU enabled.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
[ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=8
[ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[ 0.000000] REMAP: PA=17a00000 VA=ffffff8010040000 SIZE=10000
[ 0.000000] REMAP: PA=17b00000 VA=ffffff8010d00000 SIZE=100000
[ 0.000000] GICv3: Distributor has no Range Selector support
[ 0.000000] GICv3: no VLPI support, no direct LPI support
[ 0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000017b00000
[ 0.000000] ITS: No ITS available, not enabling LPIs
[ 0.000000] REMAP: PA=17920000 VA=ffffff8010005000 SIZE=1000
[ 0.000000] REMAP: PA=17921000 VA=ffffff801000d000 SIZE=1000
[ 0.000000] REMAP: PA=17921000 VA=ffffff8010015000 SIZE=1000
[ 0.000000] arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
[ 0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
[ 0.000003] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
[ 0.000061] Console: colour dummy device 80x25
[ 0.000405] printk: console [tty0] enabled
[ 0.000428] Calibrating delay loop (skipped), value calculated using timer frequency.. 38.40 BogoMIPS (lpj=76800)
[ 0.000444] pid_max: default: 32768 minimum: 301
[ 0.000542] Mount-cache hash table entries: 8192 (order: 4, 65536 bytes)
[ 0.000563] Mountpoint-cache hash table entries: 8192 (order: 4, 65536 bytes)
[ 0.000864] *** VALIDATE proc ***
[ 0.023878] ASID allocator initialised with 32768 entries
[ 0.031878] rcu: Hierarchical SRCU implementation.
[ 0.051914] smp: Bringing up secondary CPUs ...
[ 0.086062] Detected VIPT I-cache on CPU1
[ 0.086092] GICv3: CPU1: found redistributor 1 region 0:0x0000000017b20000
[ 0.086135] CPU1: Booted secondary processor 0x0000000001 [0x51af8014]
[ 0.118156] Detected VIPT I-cache on CPU2
[ 0.118178] GICv3: CPU2: found redistributor 2 region 0:0x0000000017b40000
[ 0.118218] CPU2: Booted secondary processor 0x0000000002 [0x51af8014]
[ 0.150457] Detected VIPT I-cache on CPU3
[ 0.150481] GICv3: CPU3: found redistributor 3 region 0:0x0000000017b60000
[ 0.150521] CPU3: Booted secondary processor 0x0000000003 [0x51af8014]
[ 0.183055] Detected VIPT I-cache on CPU4
[ 0.183082] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU4: 0x00000000101122
[ 0.183108] CPU features: Unsupported CPU feature variation detected.
[ 0.183141] GICv3: CPU4: found redistributor 100 region 0:0x0000000017b80000
[ 0.183217] CPU4: Booted secondary processor 0x0000000100 [0x51af8001]
[ 0.215157] Detected VIPT I-cache on CPU5
[ 0.215182] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU5: 0x00000000101122
[ 0.215236] GICv3: CPU5: found redistributor 101 region 0:0x0000000017ba0000
[ 0.215308] CPU5: Booted secondary processor 0x0000000101 [0x51af8001]
[ 0.247502] Detected VIPT I-cache on CPU6
[ 0.247528] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU6: 0x00000000101122
[ 0.247583] GICv3: CPU6: found redistributor 102 region 0:0x0000000017bc0000
[ 0.247656] CPU6: Booted secondary processor 0x0000000102 [0x51af8001]
[ 0.279824] Detected VIPT I-cache on CPU7
[ 0.279850] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU7: 0x00000000101122
[ 0.279908] GICv3: CPU7: found redistributor 103 region 0:0x0000000017be0000
[ 0.279981] CPU7: Booted secondary processor 0x0000000103 [0x51af8001]
[ 0.280178] smp: Brought up 1 node, 8 CPUs
[ 0.280396] SMP: Total of 8 processors activated.
[ 0.280406] CPU features: detected: GIC system register CPU interface
[ 0.280418] CPU features: detected: 32-bit EL0 Support
[ 0.280429] CPU features: detected: CRC32 instructions
[ 0.823309] CPU: All CPU(s) started at EL1
[ 0.823361] alternatives: patching kernel code
[ 0.824150] devtmpfs: initialized
[ 0.827562] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[ 0.827585] futex hash table entries: 2048 (order: 5, 131072 bytes)
[ 0.827708] pinctrl core: initialized pinctrl subsystem
[ 0.829449] vdso: 2 pages (1 code @ (____ptrval____), 1 data @ (____ptrval____))
[ 0.829523] DMA: preallocated 256 KiB pool for atomic allocations
[ 0.830318] REMAP: PA=01f40000 VA=ffffff8011000000 SIZE=20000
[ 0.830863] REMAP: PA=86000000 VA=ffffff8011200000 SIZE=200000
[ 0.832055] REMAP: PA=00100000 VA=ffffff8011100000 SIZE=b0000
[ 0.839431] REMAP: PA=03400000 VA=ffffff8012000000 SIZE=c00000
[ 0.840662] REMAP: PA=17911000 VA=ffffff8010025000 SIZE=1000
[ 0.848961] SCSI subsystem initialized
[ 0.849218] REMAP: PA=00778000 VA=ffffff80116f8000 SIZE=7000
[ 0.850312] clocksource: Switched to clocksource arch_sys_counter
[ 0.859512] s1: supplied by vph_pwr
[ 0.860008] s2: supplied by vph_pwr
[ 0.860291] s3: supplied by vph_pwr
[ 0.860348] s3: Bringing 0uV into 1352000-1352000uV
[ 0.860566] s4: supplied by vph_pwr
[ 0.860699] s4: Bringing 0uV into 1800000-1800000uV
[ 0.860885] s5: supplied by vph_pwr
[ 0.860933] s5: Bringing 0uV into 1904000-1904000uV
[ 0.861115] s6: supplied by vph_pwr
[ 0.861337] s7: supplied by vph_pwr
[ 0.861394] s7: Bringing 0uV into 900000-900000uV
[ 0.861639] s8: supplied by vph_pwr
[ 0.861848] s9: supplied by vph_pwr
[ 0.862055] s10: supplied by vph_pwr
[ 0.862436] s11: supplied by vph_pwr
[ 0.862651] s12: supplied by vph_pwr
[ 0.862903] s13: supplied by vph_pwr
[ 0.863154] l1: supplied by s7
[ 0.863304] l1: Bringing 0uV into 880000-880000uV
[ 0.863518] l2: supplied by s3
[ 0.863573] l2: Bringing 0uV into 1200000-1200000uV
[ 0.863763] l3: supplied by s7
[ 0.863820] l3: Bringing 0uV into 1000000-1000000uV
[ 0.864049] l4: supplied by s7
[ 0.864370] l5: supplied by s7
[ 0.864421] l5: Bringing 0uV into 800000-800000uV
[ 0.864654] l6: supplied by s5
[ 0.864707] l6: Bringing 0uV into 1808000-1808000uV
[ 0.864933] l7: supplied by s5
[ 0.864994] l7: Bringing 0uV into 1800000-1800000uV
[ 0.865269] l8: supplied by s3
[ 0.865329] l8: Bringing 0uV into 1200000-1200000uV
[ 0.865560] l9: Bringing 0uV into 1808000-1808000uV
[ 0.865805] l10: Bringing 0uV into 1808000-1808000uV
[ 0.866041] l11: supplied by s7
[ 0.866098] l11: Bringing 0uV into 1000000-1000000uV
[ 0.866455] l12: supplied by s5
[ 0.866590] l12: Bringing 0uV into 1800000-1800000uV
[ 0.866845] l13: Bringing 0uV into 1808000-1808000uV
[ 0.867109] l14: supplied by s5
[ 0.867178] l14: Bringing 0uV into 1880000-1880000uV
[ 0.867438] l15: supplied by s5
[ 0.867489] l15: Bringing 0uV into 1800000-1800000uV
[ 0.867756] l16: Bringing 0uV into 2704000-2704000uV
[ 0.868118] l17: supplied by s3
[ 0.868200] l17: Bringing 0uV into 1304000-1304000uV
[ 0.868505] l18: Bringing 0uV into 2704000-2704000uV
[ 0.868821] l19: Bringing 0uV into 3008000-3008000uV
[ 0.869127] l20: Bringing 0uV into 2960000-2960000uV
[ 0.869486] l21: Bringing 0uV into 2960000-2960000uV
[ 0.869867] l22: Bringing 0uV into 2864000-2864000uV
[ 0.870433] l23: Bringing 0uV into 3312000-3312000uV
[ 0.870860] l24: Bringing 0uV into 3088000-3088000uV
[ 0.871290] l25: Bringing 0uV into 3104000-3104000uV
[ 0.871742] l26: supplied by s3
[ 0.871793] l26: Bringing 0uV into 1200000-1200000uV
[ 0.872233] l27: supplied by s7
[ 0.872808] l28: Bringing 0uV into 3008000-3008000uV
[ 0.873285] lvs1: supplied by s4
[ 0.873825] lvs2: supplied by s4
[ 0.875080] bob: supplied by vph_pwr
[ 0.875151] bob: Bringing 0uV into 3312000-3312000uV
[ 1.183716] workingset: timestamp_bits=62 max_order=20 bucket_order=0
[ 1.192148] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
[ 1.193995] REMAP: PA=0c010000 VA=ffffff801002d000 SIZE=18c
[ 1.195073] REMAP: PA=0c010200 VA=ffffff8010035200 SIZE=128
[ 1.195106] REMAP: PA=0c010400 VA=ffffff801003d400 SIZE=200
[ 1.195129] REMAP: PA=0c010c00 VA=ffffff8010051c00 SIZE=20c
[ 1.195152] REMAP: PA=0c010600 VA=ffffff8010053600 SIZE=128
[ 1.195179] REMAP: PA=0c010800 VA=ffffff8010055800 SIZE=200
[ 1.196797] qcom-qmp-phy c010000.phy: Registered Qcom-QMP phy
[ 1.197092] REMAP: PA=01da7000 VA=ffffff801005d000 SIZE=18c
[ 1.197411] REMAP: PA=01da7400 VA=ffffff8010065400 SIZE=128
[ 1.197440] REMAP: PA=01da7600 VA=ffffff801006d600 SIZE=1fc
[ 1.197467] REMAP: PA=01da7c00 VA=ffffff8010075c00 SIZE=1dc
[ 1.197492] REMAP: PA=01da7800 VA=ffffff801007d800 SIZE=128
[ 1.197525] REMAP: PA=01da7a00 VA=ffffff8010c1ca00 SIZE=1fc
[ 1.197846] qcom-qmp-phy 1da7000.phy: Registered Qcom-QMP phy
[ 1.202503] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 1.204233] msm_serial c1b0000.serial: msm_serial: detected port #0
[ 1.204305] msm_serial c1b0000.serial: uartclk = 1843200
[ 1.204485] REMAP: PA=0c1b0000 VA=ffffff8010c1e000 SIZE=1000
[ 1.204510] c1b0000.serial: ttyMSM0 at MMIO 0xc1b0000 (irq = 17, base_baud = 115200) is a MSM
[ 1.204674] msm_serial: console setup on port #0
[ 2.240898] printk: console [ttyMSM0] enabled
[ 2.245990] msm_serial: driver initialized
[ 2.253655] REMAP: PA=01da4000 VA=ffffff8010e04000 SIZE=2500
[ 2.254403] ufshcd-qcom 1da4000.ufshc: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled
[ 2.260500] l20: supplied by bob
[ 2.270704] regulator_set_load: vdd_l20_l24 = 750000 uA
[ 2.271116] regulator_set_load: vdd_l26 = 560000 uA
[ 2.273731] regulator_set_load: vdd_s4 = 750000 uA
[ 2.280015] scsi host0: ufshcd
[ 2.319444] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_async_scan | async_run_entry_fn | process_one_work | worker_thread | kthread | ret_from_fork |
[ 2.319830] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 2.332368] l9: supplied by bob
[ 2.345241] l10: supplied by bob
[ 2.348308] l13: supplied by bob
[ 2.351688] l16: supplied by bob
[ 2.354913] l18: supplied by bob
[ 2.358104] l19: supplied by bob
[ 2.361331] l21: supplied by bob
[ 2.364541] l22: supplied by bob
[ 2.367826] l23: supplied by bob
[ 2.370962] l24: supplied by bob
[ 2.374154] l25: supplied by bob
[ 2.377395] l28: supplied by bob
[ 2.405734] regulator_disable: ENTER vdd_l26
[ 2.405958] regulator_disable: EXIT vdd_l26
[ 2.406032] regulator_set_load: vdd_l26 = 0 uA
[ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
[ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
[ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
[ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
[ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
[ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
[ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
[ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
[ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
[ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 37.074399] IGNORE ufshcd_print_host_state
[ 37.079128] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.083144] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 37.104851] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.117239] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 37.598330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 37.598390] IGNORE ufshcd_print_host_state
[ 37.603088] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.607135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 37.628846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 37.641231] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 38.122332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 38.122392] IGNORE ufshcd_print_host_state
[ 38.127084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.131135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 38.152846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.165228] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 38.646331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 38.646390] IGNORE ufshcd_print_host_state
[ 38.651083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.655135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 38.676845] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 38.689230] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 39.170331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 39.170391] IGNORE ufshcd_print_host_state
[ 39.175085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.179135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TrX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 39.200847] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.213232] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 39.232940] ufshcd_print_trs | __ufshcd_transfer_req_compl | ufshcd_transfer_req_compl | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
[ 39.241197] ufshcd-qcom 1da4000.ufshc: UPIU[24] - issue time 15538577 us
[ 39.262687] ufshcd-qcom 1da4000.ufshc: UPIU[24] - complete time 0 us
[ 39.269630] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Transfer Request Descriptor phys@0x1774ba300
[ 39.276009] UPIU TRD: 00000000: 15000000 00000000 0000000f 00000000
[ 39.284387] UPIU TRD: 00000010: 77612000 00000001 00800080 01000001
[ 39.290545] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Request UPIU phys@0x177612000
[ 39.296810] UPIU REQ: 00000000: 18d04001 00000000 00000000 24000000
[ 39.304090] UPIU REQ: 00000010: 00000012 00000024 00000000 00000000
[ 39.310338] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Response UPIU phys@0x177612200
[ 39.316603] UPIU RSP: 00000000: 00000000 00000000 00000000 00000000
[ 39.324233] UPIU RSP: 00000010: 00000000 00000000 00000000 00000000
[ 39.330220] UPIU RSP: 00000020: 00000000 00000000 00000000 00000000
[ 39.336463] UPIU RSP: 00000030: 00000000
[ 39.342703] ufshcd-qcom 1da4000.ufshc: UPIU[24] - PRDT - 1 entries phys@0x177612400
[ 39.346903] UPIU PRDT: 00000000: 77625000 00000001 00000000 00000023
[ 39.474380] ufshcd-qcom 1da4000.ufshc: __ufshcd_issue_tm_cmd: task management cmd 0x08 timed-out
[ 39.474482] ufshcd-qcom 1da4000.ufshc: ufshcd_eh_device_reset_handler: failed with err -110
[ 39.942333] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 39.942391] IGNORE ufshcd_print_host_state
[ 39.947085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 39.951126] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 39.971806] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 39.984356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 40.014355] random: fast init done
[ 40.462330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 40.462392] IGNORE ufshcd_print_host_state
[ 40.467084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.471125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 40.491805] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.504355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 40.982332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 40.982391] IGNORE ufshcd_print_host_state
[ 40.987084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 40.991125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 41.011807] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.024355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 41.502331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 41.502390] IGNORE ufshcd_print_host_state
[ 41.507083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.511124] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 41.531804] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 41.544355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 42.022330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
[ 42.022391] IGNORE ufshcd_print_host_state
[ 42.027084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 42.031125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
[ 42.051801] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
[ 42.064356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
[ 42.083047] scsi 0:0:0:49488: Device offlined - not ready after error recovery
[ 42.101655] Freeing unused kernel memory: 6016K

2019-02-06 15:55:35

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"



On 06/02/19 1:16 AM, Bjorn Andersson wrote:
> On Tue 05 Feb 02:52 PST 2019, Alim Akhtar wrote:
>
>> Hi Bjorn,
>>
>> On 05/02/19 11:57 AM, Bjorn Andersson wrote:
>>> On Mon 04 Feb 20:58 PST 2019, Alim Akhtar wrote:
>>>
>>>> Hi Marc,
>>>>
>>>> On 04/02/19 11:12 PM, Marc Gonzalez wrote:
>>>>> This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.
>>>>>
>>>>> Calling ufshcd_set_vccq_rail_unused hangs my system.
>>>>> It seems vccq is not *not* needed.
>>>>>
>>>>> Signed-off-by: Marc Gonzalez <[email protected]>
>>>>> ---
>>>>
>>>> AFAIK Samsung and Toshiba UFS devices does not use VCCQ (this pin is
>>>> either floating or connected to Ground, at least on the devices that I
>>>> have worked on).
>>>
>>> But why does such system define a vccq-supply? If the system doesn't
>>> have a regulator connected to VCCQ, then the UFS driver shouldn't be
>>> told that there is one. And if VCCQ is optional the UFS driver should
>>> support the fact that this regulator might not be supplied (i.e. call
>>> regulator_get_optional() and handle the error indicating that the supply
>>> isn't specified).
>>>
>> As per JESD220C, chapter 6.1, it does says "VCCQ - Supply voltage used
>> typically for the memory controller and optionally for the PHY
>> interface, the memory IO, and any other internal very low voltage block"
>> And we have VCCQ2 - which serve the pretty much same purpose. The
>> voltage range for VCCQ and VCCQ2 are different, VCCQ has a lower voltage
>> suitable to some low voltage block inside UFS device. I think this is
>> design consideration which allow some vendor to use one less physical
>> pin may be. And also depends on the voltage requirements of some of the
>> internal circuit.
>>
>
> This looks to me that you are required to have a VCCQ. But you said that
> you do not have a regulator supplying VCCQ on your board, and if that
> really is the case then you should not specify vccq-supply.
>
> The patch Marc is reverting states that for devices that does not have
> VCCQ connected, some unrelated regulator should be assigned to the UFS
> driver so that it can turn it off.
>
>
> If you have a regulator connected to VCCQ then it should go in
> vccq-supply, if you have a regulator connected to VCCQ2 the is should go
> in vccq2-supply. If you don't have these pins connected then there
> shouldn't be any regulators specified here!
>
Yes, that's correct, it should be like what you are suggesting, DT
entries should match the board schematic.

> Regards,
> Bjorn
>
>>
>>> Regards,
>>> Bjorn
>>>
>>>> You said your system hanged, I believe you have set UFS_DEVICE_NO_VCCQ
>>>> quirks, in that case VCCQ regulator should having been disabled.
>>>> So you mean your system hanged because vccq regulator got disabled?
>>>>
>>>>> drivers/scsi/ufs/ufs.h | 1 -
>>>>> drivers/scsi/ufs/ufshcd.c | 59 +++------------------------------------
>>>>> 2 files changed, 4 insertions(+), 56 deletions(-)
>>>>>
>>>>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>>>>> index dd65fea07687..7da7318eb6a6 100644
>>>>> --- a/drivers/scsi/ufs/ufs.h
>>>>> +++ b/drivers/scsi/ufs/ufs.h
>>>>> @@ -514,7 +514,6 @@ struct ufs_vreg {
>>>>> struct regulator *reg;
>>>>> const char *name;
>>>>> bool enabled;
>>>>> - bool unused;
>>>>> int min_uV;
>>>>> int max_uV;
>>>>> int min_uA;
>>>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>>>> index 9ba7671b84f8..8b9a01073d62 100644
>>>>> --- a/drivers/scsi/ufs/ufshcd.c
>>>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>>>> @@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
>>>>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
>>>>> bool skip_ref_clk);
>>>>> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
>>>>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
>>>>> static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
>>>>> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
>>>>> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
>>>>> @@ -6819,11 +6818,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>>>>> ufs_fixup_device_setup(hba, &card);
>>>>> ufshcd_tune_unipro_params(hba);
>>>>>
>>>>> - ret = ufshcd_set_vccq_rail_unused(hba,
>>>>> - (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
>>>>> - if (ret)
>>>>> - goto out;
>>>>> -
>>>>> /* UFS device is also active now */
>>>>> ufshcd_set_ufs_dev_active(hba);
>>>>> ufshcd_force_reset_auto_bkops(hba);
>>>>> @@ -7007,24 +7001,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
>>>>> static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
>>>>> struct ufs_vreg *vreg)
>>>>> {
>>>>> - if (!vreg)
>>>>> - return 0;
>>>>> - else if (vreg->unused)
>>>>> - return 0;
>>>>> - else
>>>>> - return ufshcd_config_vreg_load(hba->dev, vreg,
>>>>> - UFS_VREG_LPM_LOAD_UA);
>>>>> + return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
>>>>> }
>>>>>
>>>>> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>>>>> struct ufs_vreg *vreg)
>>>>> {
>>>>> - if (!vreg)
>>>>> - return 0;
>>>>> - else if (vreg->unused)
>>>>> - return 0;
>>>>> - else
>>>>> - return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
>>>>> + return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
>>>>> }
>>>>>
>>>>> static int ufshcd_config_vreg(struct device *dev,
>>>>> @@ -7062,9 +7045,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>>>> {
>>>>> int ret = 0;
>>>>>
>>>>> - if (!vreg)
>>>>> - goto out;
>>>>> - else if (vreg->enabled || vreg->unused)
>>>>> + if (!vreg || vreg->enabled)
>>>>> goto out;
>>>>>
>>>>> ret = ufshcd_config_vreg(dev, vreg, true);
>>>>> @@ -7084,9 +7065,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>>>> {
>>>>> int ret = 0;
>>>>>
>>>>> - if (!vreg)
>>>>> - goto out;
>>>>> - else if (!vreg->enabled || vreg->unused)
>>>>> + if (!vreg || !vreg->enabled)
>>>>> goto out;
>>>>>
>>>>> ret = regulator_disable(vreg->reg);
>>>>> @@ -7192,36 +7171,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> -static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
>>>>> -{
>>>>> - int ret = 0;
>>>>> - struct ufs_vreg_info *info = &hba->vreg_info;
>>>>> -
>>>>> - if (!info)
>>>>> - goto out;
>>>>> - else if (!info->vccq)
>>>>> - goto out;
>>>>> -
>>>>> - if (unused) {
>>>>> - /* shut off the rail here */
>>>>> - ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
>>>>> - /*
>>>>> - * Mark this rail as no longer used, so it doesn't get enabled
>>>>> - * later by mistake
>>>>> - */
>>>>> - if (!ret)
>>>>> - info->vccq->unused = true;
>>>>> - } else {
>>>>> - /*
>>>>> - * rail should have been already enabled hence just make sure
>>>>> - * that unused flag is cleared.
>>>>> - */
>>>>> - info->vccq->unused = false;
>>>>> - }
>>>>> -out:
>>>>> - return ret;
>>>>> -}
>>>>> -
>>>>> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
>>>>> bool skip_ref_clk)
>>>>> {
>>>>>
>>>
>>>
>
>

2019-02-06 15:57:58

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 06/02/2019 16:27, Alim Akhtar wrote:

> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>
>> [ 2.405734] regulator_disable: ENTER vdd_l26
>> [ 2.405958] regulator_disable: EXIT vdd_l26
>> [ 2.406032] regulator_set_load: vdd_l26 = 0 uA
>> [ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>> [ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>> [ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>> [ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
>> [ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
>> [ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
>> [ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
>> [ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
>> [ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
>> [ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>> [ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>> [ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>> [ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
>> [ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>> [ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>> [ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>> [ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
>> [ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
>> [ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>
> Can you check if your UFS device RESET_N is asserted correctly. It might
> be connected to some regulator and may be you can try keeping that
> regulator as "regulator-always-on" from your DT node.

How do I check RESET_N? In software or hardware?

Do you think it is not a good idea to revert 60f0187031c05e04cbadffb62f557d0ff3564490 ?

Regards.

2019-02-06 15:57:59

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Hi Marc,

On 06/02/19 8:29 PM, Marc Gonzalez wrote:
> [ Google, stop making email so hard. No, this is not spam, you twat of a Bayesian filter ]
>
> On 05/02/2019 18:51, Marc Gonzalez wrote:
>
>> On 05/02/2019 18:24, Marc Gonzalez wrote:
>>
>> Silly me. The system crashes in ufshcd_dump_regs() which is a bug
>> I fixed myself. Once I cherry-pick the appropriate fix, the board
>> no longer reboots, but UFS init does fail.
>>
>> Full boot log here:
>> https://pastebin.ubuntu.com/p/KwpRnWMFw5/
>
> Here's a better failure log, with timestamps:
>
> [ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x51af8014]
> [ 0.000000] Linux version 5.0.0-rc5-next-20190206 (mgonzalez@venus) (gcc version 7.3.1 20180425 [linaro-7.3-2018.05 revision d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05)) #19 SMP PREEMPT Wed Feb 6 15:42:45 CET 2019
> [ 0.000000] Machine model: Qualcomm Technologies, Inc. MSM8998 v1 MTP
> [ 0.000000] printk: debug: ignoring loglevel setting.
> [ 0.000000] On node 0 totalpages: 1028544
> [ 0.000000] DMA32 zone: 8192 pages used for memmap
> [ 0.000000] DMA32 zone: 0 pages reserved
> [ 0.000000] DMA32 zone: 511488 pages, LIFO batch:63
> [ 0.000000] Normal zone: 8079 pages used for memmap
> [ 0.000000] Normal zone: 517056 pages, LIFO batch:63
> [ 0.000000] psci: probing for conduit method from DT.
> [ 0.000000] psci: PSCIv1.0 detected in firmware.
> [ 0.000000] psci: Using standard PSCI v0.2 function IDs
> [ 0.000000] psci: MIGRATE_INFO_TYPE not supported.
> [ 0.000000] psci: SMC Calling Convention v1.0
> [ 0.000000] random: get_random_bytes called from start_kernel+0xa8/0x470 with crng_init=0
> [ 0.000000] percpu: Embedded 22 pages/cpu @(____ptrval____) s50184 r8192 d31736 u90112
> [ 0.000000] pcpu-alloc: s50184 r8192 d31736 u90112 alloc=22*4096
> [ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0] 4 [0] 5 [0] 6 [0] 7
> [ 0.000000] Detected VIPT I-cache on CPU0
> [ 0.000000] CPU features: detected: Kernel page table isolation (KPTI)
> [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 1012273
> [ 0.000000] Kernel command line: ignore_loglevel androidboot.bootdevice=1da4000.ufshc androidboot.serialno=53733c35 androidboot.baseband=apq mdss_mdp.panel=1:hdmi:16
> [ 0.000000] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes)
> [ 0.000000] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes)
> [ 0.000000] software IO TLB: mapped [mem 0xfbfff000-0xfffff000] (64MB)
> [ 0.000000] Memory: 3955464K/4114176K available (3262K kernel code, 410K rwdata, 944K rodata, 6016K init, 1161K bss, 158712K reserved, 0K cma-reserved)
> [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=1
> [ 0.000000] ftrace: allocating 12605 entries in 50 pages
> [ 0.000000] rcu: Preemptible hierarchical RCU implementation.
> [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=256 to nr_cpu_ids=8.
> [ 0.000000] Tasks RCU enabled.
> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
> [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=8
> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> [ 0.000000] REMAP: PA=17a00000 VA=ffffff8010040000 SIZE=10000
> [ 0.000000] REMAP: PA=17b00000 VA=ffffff8010d00000 SIZE=100000
> [ 0.000000] GICv3: Distributor has no Range Selector support
> [ 0.000000] GICv3: no VLPI support, no direct LPI support
> [ 0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000017b00000
> [ 0.000000] ITS: No ITS available, not enabling LPIs
> [ 0.000000] REMAP: PA=17920000 VA=ffffff8010005000 SIZE=1000
> [ 0.000000] REMAP: PA=17921000 VA=ffffff801000d000 SIZE=1000
> [ 0.000000] REMAP: PA=17921000 VA=ffffff8010015000 SIZE=1000
> [ 0.000000] arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
> [ 0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
> [ 0.000003] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
> [ 0.000061] Console: colour dummy device 80x25
> [ 0.000405] printk: console [tty0] enabled
> [ 0.000428] Calibrating delay loop (skipped), value calculated using timer frequency.. 38.40 BogoMIPS (lpj=76800)
> [ 0.000444] pid_max: default: 32768 minimum: 301
> [ 0.000542] Mount-cache hash table entries: 8192 (order: 4, 65536 bytes)
> [ 0.000563] Mountpoint-cache hash table entries: 8192 (order: 4, 65536 bytes)
> [ 0.000864] *** VALIDATE proc ***
> [ 0.023878] ASID allocator initialised with 32768 entries
> [ 0.031878] rcu: Hierarchical SRCU implementation.
> [ 0.051914] smp: Bringing up secondary CPUs ...
> [ 0.086062] Detected VIPT I-cache on CPU1
> [ 0.086092] GICv3: CPU1: found redistributor 1 region 0:0x0000000017b20000
> [ 0.086135] CPU1: Booted secondary processor 0x0000000001 [0x51af8014]
> [ 0.118156] Detected VIPT I-cache on CPU2
> [ 0.118178] GICv3: CPU2: found redistributor 2 region 0:0x0000000017b40000
> [ 0.118218] CPU2: Booted secondary processor 0x0000000002 [0x51af8014]
> [ 0.150457] Detected VIPT I-cache on CPU3
> [ 0.150481] GICv3: CPU3: found redistributor 3 region 0:0x0000000017b60000
> [ 0.150521] CPU3: Booted secondary processor 0x0000000003 [0x51af8014]
> [ 0.183055] Detected VIPT I-cache on CPU4
> [ 0.183082] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU4: 0x00000000101122
> [ 0.183108] CPU features: Unsupported CPU feature variation detected.
> [ 0.183141] GICv3: CPU4: found redistributor 100 region 0:0x0000000017b80000
> [ 0.183217] CPU4: Booted secondary processor 0x0000000100 [0x51af8001]
> [ 0.215157] Detected VIPT I-cache on CPU5
> [ 0.215182] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU5: 0x00000000101122
> [ 0.215236] GICv3: CPU5: found redistributor 101 region 0:0x0000000017ba0000
> [ 0.215308] CPU5: Booted secondary processor 0x0000000101 [0x51af8001]
> [ 0.247502] Detected VIPT I-cache on CPU6
> [ 0.247528] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU6: 0x00000000101122
> [ 0.247583] GICv3: CPU6: found redistributor 102 region 0:0x0000000017bc0000
> [ 0.247656] CPU6: Booted secondary processor 0x0000000102 [0x51af8001]
> [ 0.279824] Detected VIPT I-cache on CPU7
> [ 0.279850] CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64MMFR0_EL1. Boot CPU: 0x00000000001122, CPU7: 0x00000000101122
> [ 0.279908] GICv3: CPU7: found redistributor 103 region 0:0x0000000017be0000
> [ 0.279981] CPU7: Booted secondary processor 0x0000000103 [0x51af8001]
> [ 0.280178] smp: Brought up 1 node, 8 CPUs
> [ 0.280396] SMP: Total of 8 processors activated.
> [ 0.280406] CPU features: detected: GIC system register CPU interface
> [ 0.280418] CPU features: detected: 32-bit EL0 Support
> [ 0.280429] CPU features: detected: CRC32 instructions
> [ 0.823309] CPU: All CPU(s) started at EL1
> [ 0.823361] alternatives: patching kernel code
> [ 0.824150] devtmpfs: initialized
> [ 0.827562] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
> [ 0.827585] futex hash table entries: 2048 (order: 5, 131072 bytes)
> [ 0.827708] pinctrl core: initialized pinctrl subsystem
> [ 0.829449] vdso: 2 pages (1 code @ (____ptrval____), 1 data @ (____ptrval____))
> [ 0.829523] DMA: preallocated 256 KiB pool for atomic allocations
> [ 0.830318] REMAP: PA=01f40000 VA=ffffff8011000000 SIZE=20000
> [ 0.830863] REMAP: PA=86000000 VA=ffffff8011200000 SIZE=200000
> [ 0.832055] REMAP: PA=00100000 VA=ffffff8011100000 SIZE=b0000
> [ 0.839431] REMAP: PA=03400000 VA=ffffff8012000000 SIZE=c00000
> [ 0.840662] REMAP: PA=17911000 VA=ffffff8010025000 SIZE=1000
> [ 0.848961] SCSI subsystem initialized
> [ 0.849218] REMAP: PA=00778000 VA=ffffff80116f8000 SIZE=7000
> [ 0.850312] clocksource: Switched to clocksource arch_sys_counter
> [ 0.859512] s1: supplied by vph_pwr
> [ 0.860008] s2: supplied by vph_pwr
> [ 0.860291] s3: supplied by vph_pwr
> [ 0.860348] s3: Bringing 0uV into 1352000-1352000uV
> [ 0.860566] s4: supplied by vph_pwr
> [ 0.860699] s4: Bringing 0uV into 1800000-1800000uV
> [ 0.860885] s5: supplied by vph_pwr
> [ 0.860933] s5: Bringing 0uV into 1904000-1904000uV
> [ 0.861115] s6: supplied by vph_pwr
> [ 0.861337] s7: supplied by vph_pwr
> [ 0.861394] s7: Bringing 0uV into 900000-900000uV
> [ 0.861639] s8: supplied by vph_pwr
> [ 0.861848] s9: supplied by vph_pwr
> [ 0.862055] s10: supplied by vph_pwr
> [ 0.862436] s11: supplied by vph_pwr
> [ 0.862651] s12: supplied by vph_pwr
> [ 0.862903] s13: supplied by vph_pwr
> [ 0.863154] l1: supplied by s7
> [ 0.863304] l1: Bringing 0uV into 880000-880000uV
> [ 0.863518] l2: supplied by s3
> [ 0.863573] l2: Bringing 0uV into 1200000-1200000uV
> [ 0.863763] l3: supplied by s7
> [ 0.863820] l3: Bringing 0uV into 1000000-1000000uV
> [ 0.864049] l4: supplied by s7
> [ 0.864370] l5: supplied by s7
> [ 0.864421] l5: Bringing 0uV into 800000-800000uV
> [ 0.864654] l6: supplied by s5
> [ 0.864707] l6: Bringing 0uV into 1808000-1808000uV
> [ 0.864933] l7: supplied by s5
> [ 0.864994] l7: Bringing 0uV into 1800000-1800000uV
> [ 0.865269] l8: supplied by s3
> [ 0.865329] l8: Bringing 0uV into 1200000-1200000uV
> [ 0.865560] l9: Bringing 0uV into 1808000-1808000uV
> [ 0.865805] l10: Bringing 0uV into 1808000-1808000uV
> [ 0.866041] l11: supplied by s7
> [ 0.866098] l11: Bringing 0uV into 1000000-1000000uV
> [ 0.866455] l12: supplied by s5
> [ 0.866590] l12: Bringing 0uV into 1800000-1800000uV
> [ 0.866845] l13: Bringing 0uV into 1808000-1808000uV
> [ 0.867109] l14: supplied by s5
> [ 0.867178] l14: Bringing 0uV into 1880000-1880000uV
> [ 0.867438] l15: supplied by s5
> [ 0.867489] l15: Bringing 0uV into 1800000-1800000uV
> [ 0.867756] l16: Bringing 0uV into 2704000-2704000uV
> [ 0.868118] l17: supplied by s3
> [ 0.868200] l17: Bringing 0uV into 1304000-1304000uV
> [ 0.868505] l18: Bringing 0uV into 2704000-2704000uV
> [ 0.868821] l19: Bringing 0uV into 3008000-3008000uV
> [ 0.869127] l20: Bringing 0uV into 2960000-2960000uV
> [ 0.869486] l21: Bringing 0uV into 2960000-2960000uV
> [ 0.869867] l22: Bringing 0uV into 2864000-2864000uV
> [ 0.870433] l23: Bringing 0uV into 3312000-3312000uV
> [ 0.870860] l24: Bringing 0uV into 3088000-3088000uV
> [ 0.871290] l25: Bringing 0uV into 3104000-3104000uV
> [ 0.871742] l26: supplied by s3
> [ 0.871793] l26: Bringing 0uV into 1200000-1200000uV
> [ 0.872233] l27: supplied by s7
> [ 0.872808] l28: Bringing 0uV into 3008000-3008000uV
> [ 0.873285] lvs1: supplied by s4
> [ 0.873825] lvs2: supplied by s4
> [ 0.875080] bob: supplied by vph_pwr
> [ 0.875151] bob: Bringing 0uV into 3312000-3312000uV
> [ 1.183716] workingset: timestamp_bits=62 max_order=20 bucket_order=0
> [ 1.192148] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
> [ 1.193995] REMAP: PA=0c010000 VA=ffffff801002d000 SIZE=18c
> [ 1.195073] REMAP: PA=0c010200 VA=ffffff8010035200 SIZE=128
> [ 1.195106] REMAP: PA=0c010400 VA=ffffff801003d400 SIZE=200
> [ 1.195129] REMAP: PA=0c010c00 VA=ffffff8010051c00 SIZE=20c
> [ 1.195152] REMAP: PA=0c010600 VA=ffffff8010053600 SIZE=128
> [ 1.195179] REMAP: PA=0c010800 VA=ffffff8010055800 SIZE=200
> [ 1.196797] qcom-qmp-phy c010000.phy: Registered Qcom-QMP phy
> [ 1.197092] REMAP: PA=01da7000 VA=ffffff801005d000 SIZE=18c
> [ 1.197411] REMAP: PA=01da7400 VA=ffffff8010065400 SIZE=128
> [ 1.197440] REMAP: PA=01da7600 VA=ffffff801006d600 SIZE=1fc
> [ 1.197467] REMAP: PA=01da7c00 VA=ffffff8010075c00 SIZE=1dc
> [ 1.197492] REMAP: PA=01da7800 VA=ffffff801007d800 SIZE=128
> [ 1.197525] REMAP: PA=01da7a00 VA=ffffff8010c1ca00 SIZE=1fc
> [ 1.197846] qcom-qmp-phy 1da7000.phy: Registered Qcom-QMP phy
> [ 1.202503] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> [ 1.204233] msm_serial c1b0000.serial: msm_serial: detected port #0
> [ 1.204305] msm_serial c1b0000.serial: uartclk = 1843200
> [ 1.204485] REMAP: PA=0c1b0000 VA=ffffff8010c1e000 SIZE=1000
> [ 1.204510] c1b0000.serial: ttyMSM0 at MMIO 0xc1b0000 (irq = 17, base_baud = 115200) is a MSM
> [ 1.204674] msm_serial: console setup on port #0
> [ 2.240898] printk: console [ttyMSM0] enabled
> [ 2.245990] msm_serial: driver initialized
> [ 2.253655] REMAP: PA=01da4000 VA=ffffff8010e04000 SIZE=2500
> [ 2.254403] ufshcd-qcom 1da4000.ufshc: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled
> [ 2.260500] l20: supplied by bob
> [ 2.270704] regulator_set_load: vdd_l20_l24 = 750000 uA
> [ 2.271116] regulator_set_load: vdd_l26 = 560000 uA
> [ 2.273731] regulator_set_load: vdd_s4 = 750000 uA
> [ 2.280015] scsi host0: ufshcd
> [ 2.319444] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_async_scan | async_run_entry_fn | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 2.319830] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 2.332368] l9: supplied by bob
> [ 2.345241] l10: supplied by bob
> [ 2.348308] l13: supplied by bob
> [ 2.351688] l16: supplied by bob
> [ 2.354913] l18: supplied by bob
> [ 2.358104] l19: supplied by bob
> [ 2.361331] l21: supplied by bob
> [ 2.364541] l22: supplied by bob
> [ 2.367826] l23: supplied by bob
> [ 2.370962] l24: supplied by bob
> [ 2.374154] l25: supplied by bob
> [ 2.377395] l28: supplied by bob
> [ 2.405734] regulator_disable: ENTER vdd_l26
> [ 2.405958] regulator_disable: EXIT vdd_l26
> [ 2.406032] regulator_set_load: vdd_l26 = 0 uA
> [ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
> [ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
> [ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
> [ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
> [ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
> [ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
> [ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
> [ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
> [ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
> [ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
> [ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
> [ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
> [ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
> [ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
> [ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
> [ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
> [ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
> [ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
> [ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
Here the UFS link startup has failed, so no point looking below logs.
Can you check if your UFS device RESET_N is asserted correctly. It might
be connected to some regulator and may be you can try keeping that
regulator as "regulator-always-on" from your DT node.
> [ 37.074399] IGNORE ufshcd_print_host_state
> [ 37.079128] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 37.083144] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 37.104851] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 37.117239] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 37.598330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 37.598390] IGNORE ufshcd_print_host_state
> [ 37.603088] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 37.607135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 37.628846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 37.641231] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 38.122332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 38.122392] IGNORE ufshcd_print_host_state
> [ 38.127084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 38.131135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 38.152846] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 38.165228] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 38.646331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 38.646390] IGNORE ufshcd_print_host_state
> [ 38.651083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 38.655135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 38.676845] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 38.689230] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 39.170331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 39.170391] IGNORE ufshcd_print_host_state
> [ 39.175085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 39.179135] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TrX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 39.200847] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 39.213232] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 39.232940] ufshcd_print_trs | __ufshcd_transfer_req_compl | ufshcd_transfer_req_compl | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | ufshcd_abort | scmd_eh_abort_handler | process_one_work | worker_thread | kthread | ret_from_fork |
> [ 39.241197] ufshcd-qcom 1da4000.ufshc: UPIU[24] - issue time 15538577 us
> [ 39.262687] ufshcd-qcom 1da4000.ufshc: UPIU[24] - complete time 0 us
> [ 39.269630] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Transfer Request Descriptor phys@0x1774ba300
> [ 39.276009] UPIU TRD: 00000000: 15000000 00000000 0000000f 00000000
> [ 39.284387] UPIU TRD: 00000010: 77612000 00000001 00800080 01000001
> [ 39.290545] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Request UPIU phys@0x177612000
> [ 39.296810] UPIU REQ: 00000000: 18d04001 00000000 00000000 24000000
> [ 39.304090] UPIU REQ: 00000010: 00000012 00000024 00000000 00000000
> [ 39.310338] ufshcd-qcom 1da4000.ufshc: UPIU[24] - Response UPIU phys@0x177612200
> [ 39.316603] UPIU RSP: 00000000: 00000000 00000000 00000000 00000000
> [ 39.324233] UPIU RSP: 00000010: 00000000 00000000 00000000 00000000
> [ 39.330220] UPIU RSP: 00000020: 00000000 00000000 00000000 00000000
> [ 39.336463] UPIU RSP: 00000030: 00000000
> [ 39.342703] ufshcd-qcom 1da4000.ufshc: UPIU[24] - PRDT - 1 entries phys@0x177612400
> [ 39.346903] UPIU PRDT: 00000000: 77625000 00000001 00000000 00000023
> [ 39.474380] ufshcd-qcom 1da4000.ufshc: __ufshcd_issue_tm_cmd: task management cmd 0x08 timed-out
> [ 39.474482] ufshcd-qcom 1da4000.ufshc: ufshcd_eh_device_reset_handler: failed with err -110
> [ 39.942333] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 39.942391] IGNORE ufshcd_print_host_state
> [ 39.947085] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 39.951126] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 39.971806] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 39.984356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 40.014355] random: fast init done
> [ 40.462330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 40.462392] IGNORE ufshcd_print_host_state
> [ 40.467084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 40.471125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 40.491805] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 40.504355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 40.982332] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 40.982391] IGNORE ufshcd_print_host_state
> [ 40.987084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 40.991125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 41.011807] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 41.024355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 41.502331] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 41.502390] IGNORE ufshcd_print_host_state
> [ 41.507083] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 41.511124] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 41.531804] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 41.544355] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 42.022330] ufshcd-qcom 1da4000.ufshc: link startup failed 1
> [ 42.022391] IGNORE ufshcd_print_host_state
> [ 42.027084] ufshcd_print_pwr_info | ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 42.031125] ufshcd-qcom 1da4000.ufshc: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
> [ 42.051801] ufshcd_probe_hba | ufshcd_host_reset_and_restore | ufshcd_reset_and_restore | ufshcd_eh_host_reset_handler | scsi_try_host_reset | scsi_eh_ready_devs | scsi_error_handler | kthread | ret_from_fork |
> [ 42.064356] ufshcd-qcom 1da4000.ufshc: ufshcd_host_reset_and_restore: Host init failed 1
> [ 42.083047] scsi 0:0:0:49488: Device offlined - not ready after error recovery
> [ 42.101655] Freeing unused kernel memory: 6016K
>
>

2019-02-07 09:10:04

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Hi Marc,

On 06/02/19 9:22 PM, Marc Gonzalez wrote:
> On 06/02/2019 16:27, Alim Akhtar wrote:
>
>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>
>>> [ 2.405734] regulator_disable: ENTER vdd_l26
>>> [ 2.405958] regulator_disable: EXIT vdd_l26
>>> [ 2.406032] regulator_set_load: vdd_l26 = 0 uA
>>> [ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>> [ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>> [ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>> [ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
>>> [ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
>>> [ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
>>> [ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
>>> [ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
>>> [ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
>>> [ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>> [ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>> [ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>> [ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
>>> [ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>> [ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>> [ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>> [ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
>>> [ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
>>> [ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>
>> Can you check if your UFS device RESET_N is asserted correctly. It might
>> be connected to some regulator and may be you can try keeping that
>> regulator as "regulator-always-on" from your DT node.
>
> How do I check RESET_N? In software or hardware?
>
RST_N is the reset logic for UFS device core logic and it is input to
the device from UFS host controller.So, in your platform please check if
this line somehow connected to (pulled up) a PMIC supply. If that is the
case, please keep that regulator ON and see if this issue is resolved.
> Do you think it is not a good idea to revert 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>
Please hold on till we understand the real cause of this issue. Or we
have a consensuses for reverting the said commit.
Thanks!

> Regards.
>
>

2019-02-07 14:54:48

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/7/2019 1:50 AM, Alim Akhtar wrote:
> Hi Marc,
>
> On 06/02/19 9:22 PM, Marc Gonzalez wrote:
>> On 06/02/2019 16:27, Alim Akhtar wrote:
>>
>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>>
>>>> [ 2.405734] regulator_disable: ENTER vdd_l26
>>>> [ 2.405958] regulator_disable: EXIT vdd_l26
>>>> [ 2.406032] regulator_set_load: vdd_l26 = 0 uA
>>>> [ 3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>>> [ 5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>>> [ 6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
>>>> [ 6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
>>>> [ 6.946959] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
>>>> [ 6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
>>>> [ 6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
>>>> [ 6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
>>>> [ 6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
>>>> [ 8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>>> [ 10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>>> [ 11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
>>>> [ 11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
>>>> [ 13.050354] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>>> [ 14.554313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>>> [ 16.058313] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
>>>> [ 16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
>>>> [ 16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
>>>> [ 37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>>
>>> Can you check if your UFS device RESET_N is asserted correctly. It might
>>> be connected to some regulator and may be you can try keeping that
>>> regulator as "regulator-always-on" from your DT node.
>>
>> How do I check RESET_N? In software or hardware?
>>
> RST_N is the reset logic for UFS device core logic and it is input to
> the device from UFS host controller.So, in your platform please check if
> this line somehow connected to (pulled up) a PMIC supply. If that is the
> case, please keep that regulator ON and see if this issue is resolved.

The reset line is routed though the global clock controller (GCC), and
must be explicitly asserted within the GCC to trigger a reset. As far
as I am aware, Linux is not touching this.

Additionally, I fail to see how if this was a reset issue, reverting
60f0187031c0 would have any impact (which doing so addresses our issue)

>> Do you think it is not a good idea to revert 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>>
> Please hold on till we understand the real cause of this issue. Or we
> have a consensuses for reverting the said commit.
> Thanks!

Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
powers components within the host controller, and by not setting load on
the regulator properly, we are likely undervolting those components due
to the current draw?

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-08 09:29:12

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Hi Jeffrey,

On 07/02/19 8:22 PM, Jeffrey Hugo wrote:
> On 2/7/2019 1:50 AM, Alim Akhtar wrote:
>> Hi Marc,
>>
>> On 06/02/19 9:22 PM, Marc Gonzalez wrote:
>>> On 06/02/2019 16:27, Alim Akhtar wrote:
>>>
>>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>>>
>>>>> [    2.405734] regulator_disable: ENTER vdd_l26
>>>>> [    2.405958] regulator_disable: EXIT vdd_l26
>>>>> [    2.406032]   regulator_set_load: vdd_l26 = 0 uA
>>>>> [    3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>> [    5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>> [    6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>> [    6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry:
>>>>> query attribute, idn 13, failed with error -11 after 3 retires
>>>>> [    6.946959] ufshcd-qcom 1da4000.ufshc:
>>>>> ufshcd_disable_auto_bkops: failed to enable exception event -11
>>>>> [    6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>> 0x1587 failed 3 retries
>>>>> [    6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>> 0x1586 failed 3 retries
>>>>> [    6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode:
>>>>> invalid max pwm tx gear read = 0
>>>>> [    6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed
>>>>> getting max supported power mode
>>>>> [    8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>> Sending flag query for idn 3 failed, err = -11
>>>>> [   10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>> Sending flag query for idn 3 failed, err = -11
>>>>> [   11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>> Sending flag query for idn 3 failed, err = -11
>>>>> [   11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry:
>>>>> query attribute, opcode 5, idn 3, failed with error -11 after 3
>>>>> retires
>>>>> [   13.050354] ufshcd-qcom 1da4000.ufshc:
>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>> err = -11
>>>>> [   14.554313] ufshcd-qcom 1da4000.ufshc:
>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>> err = -11
>>>>> [   16.058313] ufshcd-qcom 1da4000.ufshc:
>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>> err = -11
>>>>> [   16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param:
>>>>> Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0,
>>>>> ret -11
>>>>> [   16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels:
>>>>> Failed reading power descriptor.len = 98 ret = -11
>>>>> [   37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>>>
>>>> Can you check if your UFS device RESET_N is asserted correctly. It
>>>> might
>>>> be connected to some regulator and may be you can try keeping that
>>>> regulator as "regulator-always-on" from your DT node.
>>>
>>> How do I check RESET_N? In software or hardware?
>>>
>> RST_N is the reset logic for UFS device core logic and it is input to
>> the device from UFS host controller.So, in your platform please check if
>> this line somehow connected to (pulled up) a PMIC supply. If that is the
>> case, please keep that regulator ON and see if this issue is resolved.
>
> The reset line is routed though the global clock controller (GCC), and
> must be explicitly asserted within the GCC to trigger a reset.  As far
> as I am aware, Linux is not touching this.
>
> Additionally, I fail to see how if this was a reset issue, reverting
> 60f0187031c0 would have any impact (which doing so addresses our issue)
>
OK, that's again implementation dependent and your platform used that
way. My point was to make sure that reset part is ok, if reset/power is
not proper to the UFS device core logic this kind of issues comes.

>>> Do you think it is not a good idea to revert
>>> 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>>>
>> Please hold on till we understand the real cause of this issue. Or we
>> have a consensuses for reverting the said commit.
>> Thanks!
>
> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
> powers components within the host controller, and by not setting load on
> the regulator properly, we are likely undervolting those components due
> to the current draw?
>
In theory may be true. But looks like we dont have a solid evidence yet
(correct me if I am wrong or misunderstood anything here)
So that means its some short of hardware/board quirk, right?
Can you please recheck the schematic and see what Bjorn is telling
(about having right entries in the DT for regulator) resolve your issue?

Marc, Can you disabled pmic on that board (hope your board boots with
default PMIC supply) and see if this issue still occurs?
I am just trying to understand and see what is the real cause.

@Yaniv Gardi, will you be able to comment on reason for adding
60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)?

2019-02-08 10:05:19

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

+ Yaniv (just in case)
+ Hannes (if he remembers)

We're discussing commit 60f0187031c05e04cbadffb62f557d0ff3564490

On 08/02/2019 10:09, Alim Akhtar wrote:

> On 07/02/19 8:22 PM, Jeffrey Hugo wrote:
>
>> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
>> powers components within the host controller, and by not setting load on
>> the regulator properly, we are likely undervolting those components due
>> to the current draw?
>
> In theory may be true. But looks like we dont have a solid evidence yet
> (correct me if I am wrong or misunderstood anything here)
> So that means its some short of hardware/board quirk, right?
> Can you please recheck the schematic and see what Bjorn is telling
> (about having right entries in the DT for regulator) resolve your issue?

I think there might be a misunderstanding on your side.

Basically, Bjorn is saying that code to disable a regulator is not needed.
(Therefore he is in favor of the patch to revert.)

If a board does not require some power rail, then it should simply *not*
list it in the DT. My board *does* require that power rail, so I must
list it in the DT. The driver should not attempt to work around that
simple fact (by disabling a regulator that was erroneously listed).

> Marc, Can you disabled pmic on that board (hope your board boots with
> default PMIC supply) and see if this issue still occurs?
> I am just trying to understand and see what is the real cause.

I don't know how to disable the PMIC.

> @Yaniv Gardi, will you be able to comment on reason for adding
> 60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)?

AFAIU, Yaniv no longer works for CA, so he is unlikely to reply.

I'll resend the series and CC the regulators maintainers so they
can share their thoughts.

Regards.

2019-02-08 15:00:26

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/8/2019 2:09 AM, Alim Akhtar wrote:
> Hi Jeffrey,
>
> On 07/02/19 8:22 PM, Jeffrey Hugo wrote:
>> On 2/7/2019 1:50 AM, Alim Akhtar wrote:
>>> Hi Marc,
>>>
>>> On 06/02/19 9:22 PM, Marc Gonzalez wrote:
>>>> On 06/02/2019 16:27, Alim Akhtar wrote:
>>>>
>>>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>>>>
>>>>>> [    2.405734] regulator_disable: ENTER vdd_l26
>>>>>> [    2.405958] regulator_disable: EXIT vdd_l26
>>>>>> [    2.406032]   regulator_set_load: vdd_l26 = 0 uA
>>>>>> [    3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>> [    5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>> [    6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>> [    6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry:
>>>>>> query attribute, idn 13, failed with error -11 after 3 retires
>>>>>> [    6.946959] ufshcd-qcom 1da4000.ufshc:
>>>>>> ufshcd_disable_auto_bkops: failed to enable exception event -11
>>>>>> [    6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>> 0x1587 failed 3 retries
>>>>>> [    6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>> 0x1586 failed 3 retries
>>>>>> [    6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode:
>>>>>> invalid max pwm tx gear read = 0
>>>>>> [    6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed
>>>>>> getting max supported power mode
>>>>>> [    8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>> [   10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>> [   11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>> [   11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry:
>>>>>> query attribute, opcode 5, idn 3, failed with error -11 after 3
>>>>>> retires
>>>>>> [   13.050354] ufshcd-qcom 1da4000.ufshc:
>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>> err = -11
>>>>>> [   14.554313] ufshcd-qcom 1da4000.ufshc:
>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>> err = -11
>>>>>> [   16.058313] ufshcd-qcom 1da4000.ufshc:
>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>> err = -11
>>>>>> [   16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param:
>>>>>> Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0,
>>>>>> ret -11
>>>>>> [   16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels:
>>>>>> Failed reading power descriptor.len = 98 ret = -11
>>>>>> [   37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>>>>
>>>>> Can you check if your UFS device RESET_N is asserted correctly. It
>>>>> might
>>>>> be connected to some regulator and may be you can try keeping that
>>>>> regulator as "regulator-always-on" from your DT node.
>>>>
>>>> How do I check RESET_N? In software or hardware?
>>>>
>>> RST_N is the reset logic for UFS device core logic and it is input to
>>> the device from UFS host controller.So, in your platform please check if
>>> this line somehow connected to (pulled up) a PMIC supply. If that is the
>>> case, please keep that regulator ON and see if this issue is resolved.
>>
>> The reset line is routed though the global clock controller (GCC), and
>> must be explicitly asserted within the GCC to trigger a reset.  As far
>> as I am aware, Linux is not touching this.
>>
>> Additionally, I fail to see how if this was a reset issue, reverting
>> 60f0187031c0 would have any impact (which doing so addresses our issue)
>>
> OK, that's again implementation dependent and your platform used that
> way. My point was to make sure that reset part is ok, if reset/power is
> not proper to the UFS device core logic this kind of issues comes.

We are following the Hardware Programming Guide written by the platform
designers with regard to UFS, including the reset logic. I really don't
think the reset logic is an issue here.

>
>>>> Do you think it is not a good idea to revert
>>>> 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>>>>
>>> Please hold on till we understand the real cause of this issue. Or we
>>> have a consensuses for reverting the said commit.
>>> Thanks!
>>
>> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
>> powers components within the host controller, and by not setting load on
>> the regulator properly, we are likely undervolting those components due
>> to the current draw?
>>
> In theory may be true. But looks like we dont have a solid evidence yet
> (correct me if I am wrong or misunderstood anything here
The evidence seems simple. We have properly described in DT all the
regulators that are consumed by the UFS host controller, and by
extension, the UFS storage chip as well.

By default, with no kernel changes, UFS does not work.

Marc and I debugged the issue, and found that the VCCQ regulator was not
being handled properly, and reverting the change we are discussing fixes
the the VCCQ regulator issue, and as a result UFS works.

Again, despite the fact that we may have a Samsung UFS storage chip,
which triggers the quirk, the UFS host controller which talks to that
chip requires VCCQ, therefore this quirk breaks us.

> So that means its some short of hardware/board quirk, right?

No

> Can you please recheck the schematic and see what Bjorn is telling
> (about having right entries in the DT for regulator) resolve your issue?

Already done. The schematic defines vcc, vccq, and vccq2. All of those
are listed in DT, and have been since Marc and I have been trying to
utilize UFS.

>
> Marc, Can you disabled pmic on that board (hope your board boots with
> default PMIC supply) and see if this issue still occurs?

The PMIC is required the boot the board. I doubt the board will be
functional with the PMIC disabled.

> I am just trying to understand and see what is the real cause.

Our analysis is that VCCQ is required and
60f0187031c05e04cbadffb62f557d0ff3564490 prevents the proper
configuration of VCCQ, thus a required resource (VCCQ) is not in the
proper state.

>
> @Yaniv Gardi, will you be able to comment on reason for adding
> 60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)?
>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2019-02-09 09:02:39

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"



On 08/02/19 8:29 PM, Jeffrey Hugo wrote:
> On 2/8/2019 2:09 AM, Alim Akhtar wrote:
>> Hi Jeffrey,
>>
>> On 07/02/19 8:22 PM, Jeffrey Hugo wrote:
>>> On 2/7/2019 1:50 AM, Alim Akhtar wrote:
>>>> Hi Marc,
>>>>
>>>> On 06/02/19 9:22 PM, Marc Gonzalez wrote:
>>>>> On 06/02/2019 16:27, Alim Akhtar wrote:
>>>>>
>>>>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>>>>>
>>>>>>> [    2.405734] regulator_disable: ENTER vdd_l26
>>>>>>> [    2.405958] regulator_disable: EXIT vdd_l26
>>>>>>> [    2.406032]   regulator_set_load: vdd_l26 = 0 uA
>>>>>>> [    3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>> [    5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>> [    6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>> [    6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry:
>>>>>>> query attribute, idn 13, failed with error -11 after 3 retires
>>>>>>> [    6.946959] ufshcd-qcom 1da4000.ufshc:
>>>>>>> ufshcd_disable_auto_bkops: failed to enable exception event -11
>>>>>>> [    6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>>> 0x1587 failed 3 retries
>>>>>>> [    6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>>> 0x1586 failed 3 retries
>>>>>>> [    6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode:
>>>>>>> invalid max pwm tx gear read = 0
>>>>>>> [    6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed
>>>>>>> getting max supported power mode
>>>>>>> [    8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>> [   10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>> [   11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>> [   11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry:
>>>>>>> query attribute, opcode 5, idn 3, failed with error -11 after 3
>>>>>>> retires
>>>>>>> [   13.050354] ufshcd-qcom 1da4000.ufshc:
>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>> err = -11
>>>>>>> [   14.554313] ufshcd-qcom 1da4000.ufshc:
>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>> err = -11
>>>>>>> [   16.058313] ufshcd-qcom 1da4000.ufshc:
>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>> err = -11
>>>>>>> [   16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param:
>>>>>>> Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0,
>>>>>>> ret -11
>>>>>>> [   16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels:
>>>>>>> Failed reading power descriptor.len = 98 ret = -11
>>>>>>> [   37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>>>>>
>>>>>> Can you check if your UFS device RESET_N is asserted correctly. It
>>>>>> might
>>>>>> be connected to some regulator and may be you can try keeping that
>>>>>> regulator as "regulator-always-on" from your DT node.
>>>>>
>>>>> How do I check RESET_N? In software or hardware?
>>>>>
>>>> RST_N is the reset logic for UFS device core logic and it is input to
>>>> the device from UFS host controller.So, in your platform please
>>>> check if
>>>> this line somehow connected to (pulled up) a PMIC supply. If that is
>>>> the
>>>> case, please keep that regulator ON and see if this issue is resolved.
>>>
>>> The reset line is routed though the global clock controller (GCC), and
>>> must be explicitly asserted within the GCC to trigger a reset.  As far
>>> as I am aware, Linux is not touching this.
>>>
>>> Additionally, I fail to see how if this was a reset issue, reverting
>>> 60f0187031c0 would have any impact (which doing so addresses our issue)
>>>
>> OK, that's again implementation dependent and your platform used that
>> way. My point was to make sure that reset part is ok, if reset/power is
>> not proper to the UFS device core logic this kind of issues comes.
>
> We are following the Hardware Programming Guide written by the platform
> designers with regard to UFS, including the reset logic.  I really don't
> think the reset logic is an issue here.
>
>>
>>>>> Do you think it is not a good idea to revert
>>>>> 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>>>>>
>>>> Please hold on till we understand the real cause of this issue. Or we
>>>> have a consensuses for reverting the said commit.
>>>> Thanks!
>>>
>>> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
>>> powers components within the host controller, and by not setting load on
>>> the regulator properly, we are likely undervolting those components due
>>> to the current draw?
>>>
>> In theory may be true. But looks like we dont have a solid evidence yet
>> (correct me if I am wrong or misunderstood anything here
> The evidence seems simple.  We have properly described in DT all the
> regulators that are consumed by the UFS host controller, and by
> extension, the UFS storage chip as well.
>
> By default, with no kernel changes, UFS does not work.
>
> Marc and I debugged the issue, and found that the VCCQ regulator was not
> being handled properly, and reverting the change we are discussing fixes
> the the VCCQ regulator issue, and as a result UFS works.
>
Ok, fair, before we revert this patch, Marc can you try below patch, or
let me know if you have already tried this and share your
result/observation:

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index 50e9033aa7f6..b08e8d1ea0f3 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -212,6 +212,7 @@
vreg_l26a_1p2: l26 {
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
+ regulator-always-on;
};
vreg_l28_3p0: l28 {
regulator-min-microvolt = <3008000>;
---

I believe "vreg_l26a_1p2" is supply for ufs's vccq.

check the result of this patch /wo reverting
60f0187031c05e04cbadffb62f557d0ff3564490

> Again, despite the fact that we may have a Samsung UFS storage chip,
> which triggers the quirk, the UFS host controller which talks to that
> chip requires VCCQ, therefore this quirk breaks us.
>
>> So that means its some short of hardware/board quirk, right?
>
> No
>
>> Can you please recheck the schematic and see what Bjorn is telling
>> (about having right entries in the DT for regulator) resolve your issue?
>
> Already done.  The schematic defines vcc, vccq, and vccq2.  All of those
> are listed in DT, and have been since Marc and I have been trying to
> utilize UFS.
>
>>
>> Marc, Can you disabled pmic on that board (hope your board boots with
>> default PMIC supply) and see if this issue still occurs?
>
> The PMIC is required the boot the board.  I doubt the board will be
> functional with the PMIC disabled.
>
>> I am just trying to understand and see what is the real cause.
>
> Our analysis is that VCCQ is required and
> 60f0187031c05e04cbadffb62f557d0ff3564490 prevents the proper
> configuration of VCCQ, thus a required resource (VCCQ) is not in the
> proper state.
>
Not in proper state or vccq regulator is disabled?
>>
>> @Yaniv Gardi, will you be able to comment on reason for adding
>> 60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)?
>>
>
>

2019-02-09 11:55:03

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

Adding DT & regulator maintainers.

FTR, we are discussing the revert of patch 60f0187031c0
in the UFSHC driver.

On 09/02/2019 09:42, Alim Akhtar wrote:

> On 08/02/19 8:29 PM, Jeffrey Hugo wrote:
>
>> The evidence seems simple.? We have properly described in DT all the
>> regulators that are consumed by the UFS host controller, and by
>> extension, the UFS storage chip as well.
>>
>> By default, with no kernel changes, UFS does not work.
>>
>> Marc and I debugged the issue, and found that the VCCQ regulator was not
>> being handled properly, and reverting the change we are discussing fixes
>> the VCCQ regulator issue, and, as a result, UFS works.
>
> OK, fair, before we revert this patch, Marc can you try below patch,
> or let me know if you have already tried this and share your
> result/observation:
>
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -212,6 +212,7 @@
> vreg_l26a_1p2: l26 {
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> + regulator-always-on;

This property will make _regulator_disable() be (mostly)
a NOP for vreg_l26a_1p2. So the UFSHC driver will not
be able to disable vccq, and UFS will work.

I tested something similar by making regulator_disable()
a NOP which returns immediately. That's actually how I
found the issue in the UFSHC driver ;-)

But this is not a proper solution. This makes it impossible
to disable l26, even when there is no UFS driver, or when
the UFSHC goes into sleep mode.


>> Our analysis is that VCCQ is required and 60f0187031c05e
>> prevents the proper configuration of VCCQ, thus a required
>> resource (VCCQ) is not in the proper state.
>
> Not in proper state or vccq regulator is disabled?

The improper state is being disabled, instead of enabled.

Regards.

2019-02-10 15:59:18

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

On 2/9/2019 1:42 AM, Alim Akhtar wrote:
>
>
> On 08/02/19 8:29 PM, Jeffrey Hugo wrote:
>> On 2/8/2019 2:09 AM, Alim Akhtar wrote:
>>> Hi Jeffrey,
>>>
>>> On 07/02/19 8:22 PM, Jeffrey Hugo wrote:
>>>> On 2/7/2019 1:50 AM, Alim Akhtar wrote:
>>>>> Hi Marc,
>>>>>
>>>>> On 06/02/19 9:22 PM, Marc Gonzalez wrote:
>>>>>> On 06/02/2019 16:27, Alim Akhtar wrote:
>>>>>>
>>>>>>> On 06/02/19 8:29 PM, Marc Gonzalez wrote:
>>>>>>>
>>>>>>>> [    2.405734] regulator_disable: ENTER vdd_l26
>>>>>>>> [    2.405958] regulator_disable: EXIT vdd_l26
>>>>>>>> [    2.406032]   regulator_set_load: vdd_l26 = 0 uA
>>>>>>>> [    3.930447] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>>> [    5.434358] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>>> [    6.938318] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode
>>>>>>>> 0x04 for idn 13 failed, index 0, err = -11
>>>>>>>> [    6.938414] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry:
>>>>>>>> query attribute, idn 13, failed with error -11 after 3 retires
>>>>>>>> [    6.946959] ufshcd-qcom 1da4000.ufshc:
>>>>>>>> ufshcd_disable_auto_bkops: failed to enable exception event -11
>>>>>>>> [    6.958523] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>>>> 0x1587 failed 3 retries
>>>>>>>> [    6.967730] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id
>>>>>>>> 0x1586 failed 3 retries
>>>>>>>> [    6.975576] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode:
>>>>>>>> invalid max pwm tx gear read = 0
>>>>>>>> [    6.983306] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed
>>>>>>>> getting max supported power mode
>>>>>>>> [    8.506314] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>>> [   10.010352] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>>> [   11.514313] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag:
>>>>>>>> Sending flag query for idn 3 failed, err = -11
>>>>>>>> [   11.514412] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry:
>>>>>>>> query attribute, opcode 5, idn 3, failed with error -11 after 3
>>>>>>>> retires
>>>>>>>> [   13.050354] ufshcd-qcom 1da4000.ufshc:
>>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>>> err = -11
>>>>>>>> [   14.554313] ufshcd-qcom 1da4000.ufshc:
>>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>>> err = -11
>>>>>>>> [   16.058313] ufshcd-qcom 1da4000.ufshc:
>>>>>>>> __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0,
>>>>>>>> err = -11
>>>>>>>> [   16.058421] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param:
>>>>>>>> Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0,
>>>>>>>> ret -11
>>>>>>>> [   16.067654] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels:
>>>>>>>> Failed reading power descriptor.len = 98 ret = -11
>>>>>>>> [   37.074334] ufshcd-qcom 1da4000.ufshc: link startup failed 1
>>>>>>>
>>>>>>> Can you check if your UFS device RESET_N is asserted correctly. It
>>>>>>> might
>>>>>>> be connected to some regulator and may be you can try keeping that
>>>>>>> regulator as "regulator-always-on" from your DT node.
>>>>>>
>>>>>> How do I check RESET_N? In software or hardware?
>>>>>>
>>>>> RST_N is the reset logic for UFS device core logic and it is input to
>>>>> the device from UFS host controller.So, in your platform please
>>>>> check if
>>>>> this line somehow connected to (pulled up) a PMIC supply. If that is
>>>>> the
>>>>> case, please keep that regulator ON and see if this issue is resolved.
>>>>
>>>> The reset line is routed though the global clock controller (GCC), and
>>>> must be explicitly asserted within the GCC to trigger a reset.  As far
>>>> as I am aware, Linux is not touching this.
>>>>
>>>> Additionally, I fail to see how if this was a reset issue, reverting
>>>> 60f0187031c0 would have any impact (which doing so addresses our issue)
>>>>
>>> OK, that's again implementation dependent and your platform used that
>>> way. My point was to make sure that reset part is ok, if reset/power is
>>> not proper to the UFS device core logic this kind of issues comes.
>>
>> We are following the Hardware Programming Guide written by the platform
>> designers with regard to UFS, including the reset logic.  I really don't
>> think the reset logic is an issue here.
>>
>>>
>>>>>> Do you think it is not a good idea to revert
>>>>>> 60f0187031c05e04cbadffb62f557d0ff3564490 ?
>>>>>>
>>>>> Please hold on till we understand the real cause of this issue. Or we
>>>>> have a consensuses for reverting the said commit.
>>>>> Thanks!
>>>>
>>>> Did you see https://lkml.org/lkml/2019/2/5/659 where I indicated VCCQ
>>>> powers components within the host controller, and by not setting load on
>>>> the regulator properly, we are likely undervolting those components due
>>>> to the current draw?
>>>>
>>> In theory may be true. But looks like we dont have a solid evidence yet
>>> (correct me if I am wrong or misunderstood anything here
>> The evidence seems simple.  We have properly described in DT all the
>> regulators that are consumed by the UFS host controller, and by
>> extension, the UFS storage chip as well.
>>
>> By default, with no kernel changes, UFS does not work.
>>
>> Marc and I debugged the issue, and found that the VCCQ regulator was not
>> being handled properly, and reverting the change we are discussing fixes
>> the the VCCQ regulator issue, and as a result UFS works.
>>
> Ok, fair, before we revert this patch, Marc can you try below patch, or
> let me know if you have already tried this and share your
> result/observation:
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 50e9033aa7f6..b08e8d1ea0f3 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -212,6 +212,7 @@
> vreg_l26a_1p2: l26 {
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> + regulator-always-on;
> };
> vreg_l28_3p0: l28 {
> regulator-min-microvolt = <3008000>;
> ---
>
> I believe "vreg_l26a_1p2" is supply for ufs's vccq.
>
> check the result of this patch /wo reverting
> 60f0187031c05e04cbadffb62f557d0ff3564490
>
>> Again, despite the fact that we may have a Samsung UFS storage chip,
>> which triggers the quirk, the UFS host controller which talks to that
>> chip requires VCCQ, therefore this quirk breaks us.
>>
>>> So that means its some short of hardware/board quirk, right?
>>
>> No
>>
>>> Can you please recheck the schematic and see what Bjorn is telling
>>> (about having right entries in the DT for regulator) resolve your issue?
>>
>> Already done.  The schematic defines vcc, vccq, and vccq2.  All of those
>> are listed in DT, and have been since Marc and I have been trying to
>> utilize UFS.
>>
>>>
>>> Marc, Can you disabled pmic on that board (hope your board boots with
>>> default PMIC supply) and see if this issue still occurs?
>>
>> The PMIC is required the boot the board.  I doubt the board will be
>> functional with the PMIC disabled.
>>
>>> I am just trying to understand and see what is the real cause.
>>
>> Our analysis is that VCCQ is required and
>> 60f0187031c05e04cbadffb62f557d0ff3564490 prevents the proper
>> configuration of VCCQ, thus a required resource (VCCQ) is not in the
>> proper state.
>>
> Not in proper state or vccq regulator is disabled?

Proper state. The PHY also consumes VCCQ, so it should be on, however
the regulators have multiple power savings states based on the amount of
load expressed. The amount of load the PHY puts on the regulator is
minimal. The load the controller puts on the regulator is significant
(which would result in a non-default state that would consume more
power), however the quirk prevents that load from being expressed, and
thus the regulator is in the wrong state for the system needs.

>>>
>>> @Yaniv Gardi, will you be able to comment on reason for adding
>>> 60f0187031c05e04cbadffb62f557d0ff3564490 (any issue faced)?
>>>
>>
>>


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.