RE: [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7

["Alim Akhtar" <alim.akhtar@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 8139 bytes --]

Hi Pawel,

> -----Original Message-----
> From: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> Sent: 03 April 2020 22:22
> To: Alim Akhtar <alim.akhtar@samsung.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-scsi@vger.kernel.org
> Cc: krzk@kernel.org; avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7
> 
> Hi Alim
> 
> Looking at vendor sources, my device is using the same gpios for
> urfs_rst_n and ufs_refclk_out like Espresso (with one difference -
> ufs_rst_n shouldn't be pulled up).
> 
> About regulators (it would be easier if dts would have all regulators).
> It's also using s2mps15 as Espresso, but it vendor dts had only 8 (of
> 10 possible bucks, one missing was for UFS) and 14 ldos (of 27
> possible), where almost all rails are connected to something.
> 
> I'm wondering how it's working on Espresso, because when adding correct
> regulators for ufs (vccq = buck10 from s2mps15, always enabled for
> testing plus vccq2 and vccq = two regulators enabled by one gpio,
> enabled at boot by firmware), ufs wasn't still working because it was
> then failing at defer probe (s2mps15 was probed after ufs)
> 
> [    0.962482] exynos-ufshc 15570000.ufs: ufshcd_get_vreg: vccq get
> failed, err=-517
>
As I said, this is very specific to the board, on Espresso we have LDO12 connected to UFS_RESETn.
Either make all of them as always-on, or just disabled s2mps15 
(default voltage supply should be ok, unless bootloader on your board does have messed too much with PMIC)
 
> After that boot would just stop/hang.
> 
> After making a "dirty fix" by making s2mps15 regulator driver use
> subsys_initcall (like in vendor sources) and ufs late_initcall (to give
> it more time to setup and get it working and solve it later),
> i had to mark following clocks as CLK_IGNORE_UNUSED to be able to bring
> link up (it replicates setting done by vendor kernel, which enables
> them on boot):
> - "phyclk_ufs20_rx1_symbol_user"
> - "phyclk_ufs20_rx0_symbol_user"
> - "phyclk_ufs20_tx0_symbol_user"
> 
Coming to these clocks, all these are supplied by default, my best guess is since you are using an actual product (S6 edge), they might have optimized for power saving 
And most likely all clock might be  gated initially. In my case all are set to default.
I have attached a small change in the exynos7 dts and phy driver clock handling, please try this attached patch and let me know if this helps in removing some of your hacks.
In the later SoCs these clocks are not in this form, so I didn't included in my current patch set, If this works for your, will add as an optional for exynos7/7420.
I also assume you are using clk-exynos7.c and my posted ufs driver.

> Now it's able to bring both device and link, but it fails at
> ufshcd_uic_change_pwr_mode.
> 
Can you please use the exact ufs and ufs-phy device node as in my patch?

> [    1.411547] exynos-ufshc 15570000.ufs: ufshcd_init_clocks: clk:
> core_clk, rate: 100000000
> [    1.419698] exynos-ufshc 15570000.ufs: ufshcd_init_clocks: clk:
> sclk_unipro_main, rate: 167000000
> [    1.428550] exynos-ufshc 15570000.ufs: __ufshcd_setup_clocks: clk:
> core_clk enabled
> [    1.436200] exynos-ufshc 15570000.ufs: __ufshcd_setup_clocks: clk:
> sclk_unipro_main enabled
> [    1.445704] scsi host0: ufshcd
> [    1.465684] exynos-ufshc 15570000.ufs: ufshcd_print_pwr_info:[RX,
> TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate
> =
> 0
> [    2.023699] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd041 val
> 0x1fff error code 1
> [    2.023846] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd041 val
> 0x1fff failed 0 retries
> [    2.024025] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd042 val
> 0xffff error code 1
> [    2.025457] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd042 val
> 0xffff failed 0 retries
> [    2.033777] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd043 val
> 0x7fff error code 1
> [    2.041607] exynos-ufshc 15570000.ufs: dme-set: attr-id 0xd043 val
> 0x7fff failed 0 retries
> [    2.067809] exynos-ufshc 15570000.ufs: pwr ctrl cmd 0x2 failed, host
> upmcrs:0x5
> [    2.067953] exynos-ufshc 15570000.ufs: UFS Host state=0
> [    2.068056] exynos-ufshc 15570000.ufs: outstanding reqs=0x0
> tasks=0x0
> [    2.068759] exynos-ufshc 15570000.ufs: saved_err=0x0,
> saved_uic_err=0x0
> [    2.075368] exynos-ufshc 15570000.ufs: Device power mode=1, UIC link
> state=1
> [    2.082392] exynos-ufshc 15570000.ufs: PM in progress=0, sys.
> suspended=0
> [    2.089158] exynos-ufshc 15570000.ufs: Auto BKOPS=0, Host self-
> block=0
> [    2.095667] exynos-ufshc 15570000.ufs: Clk gate=1
> [    2.100354] exynos-ufshc 15570000.ufs: error handling flags=0x0,
> req. abort count=0
> [    2.107987] exynos-ufshc 15570000.ufs: Host capabilities=0x383ff0f,
> caps=0x0
> [    2.115018] exynos-ufshc 15570000.ufs: quirks=0x780, dev.
> quirks=0xc4
> [    2.121443] exynos-ufshc 15570000.ufs: ufshcd_print_pwr_info:[RX,
> TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate
> =
> 0
> [    2.133960] host_regs: 00000000: 0383ff0f 00000000 00000200 00000000
> [    2.140268] host_regs: 00000010: 00000101 00007fce 00000000 00000000
> [    2.146604] host_regs: 00000020: 00000000 00030a75 00000000 00000000
> [    2.152940] host_regs: 00000030: 0000050f 00000000 80000010 00000000
> [    2.159271] host_regs: 00000040: 00000000 00000000 00000000 00000000
> [    2.165609] host_regs: 00000050: f9587000 00000000 00000000 00000000
> [    2.171944] host_regs: 00000060: 00000001 00000000 00000000 00000000
> [    2.178278] host_regs: 00000070: f958a000 00000000 00000000 00000000
> [    2.184609] host_regs: 00000080: 00000001 00000000 00000000 00000000
> [    2.190945] host_regs: 00000090: 00000002 15710000 00000000 00000000
> [    2.197282] exynos-ufshc 15570000.ufs: hba->ufs_version = 0x200,
> hba->capabilities = 0x383ff0f
> [    2.205869] exynos-ufshc 15570000.ufs: hba->outstanding_reqs = 0x0,
> hba->outstanding_tasks = 0x0
> [    2.214636] exynos-ufshc 15570000.ufs: last_hibern8_exit_tstamp at 0
> us, hibern8_exit_cnt = 0
> [    2.223141] exynos-ufshc 15570000.ufs: No record of pa_err
> [    2.228606] exynos-ufshc 15570000.ufs: No record of dl_err
> [    2.234071] exynos-ufshc 15570000.ufs: No record of nl_err
> [    2.239540] exynos-ufshc 15570000.ufs: No record of tl_err
> [    2.245007] exynos-ufshc 15570000.ufs: No record of dme_err
> [    2.250558] exynos-ufshc 15570000.ufs: No record of auto_hibern8_err
> [    2.256895] exynos-ufshc 15570000.ufs: No record of fatal_err
> [    2.262624] exynos-ufshc 15570000.ufs: No record of
> link_startup_fail
> [    2.269044] exynos-ufshc 15570000.ufs: No record of resume_fail
> [    2.274942] exynos-ufshc 15570000.ufs: No record of suspend_fail
> [    2.280931] exynos-ufshc 15570000.ufs: No record of dev_reset
> [    2.286659] exynos-ufshc 15570000.ufs: No record of host_reset
> [    2.292475] exynos-ufshc 15570000.ufs: No record of task_abort
> [    2.298290] exynos-ufshc 15570000.ufs: ufshcd_change_power_mode:
> power mode change failed 5
> [    2.306619] exynos-ufshc 15570000.ufs: ufshcd_probe_hba: Failed
> setting power mode, err = 5
> [    2.315144] exynos-ufshc 15570000.ufs: __ufshcd_setup_clocks: clk:
> core_clk disabled
> 
> And here boot would just stop/hang.
> 
> Thanks for all hints.
> 
> >
> >
> > > > > Also looking at clk-exynos7 driver seems to confirm this.
> > > > >
> > > > > > +		};
> > > > > > +
> > > > > >  		usbdrd_phy: phy@15500000 {
> > > > > >  			compatible = "samsung,exynos7-usbdrd-phy";
> > > > > >  			reg = <0x15500000 0x100>;
> >
> >



[-- Attachment #2: ufs_phy_clk.patch --]
[-- Type: application/octet-stream, Size: 4939 bytes --]

diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index a9531c4bf22f..9a4dd9ae5cdf 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -665,9 +665,14 @@
 			reg-names = "phy-pma";
 			samsung,pmu-syscon = <&pmu_system_controller>;
 			#phy-cells = <0>;
-			clocks = <&clock_fsys1 MOUT_FSYS1_PHYCLK_SEL1>,
-				<&clock_top1 CLK_SCLK_PHY_FSYS1_26M>;
-			clock-names = "ref_clk_parent", "ref_clk";
+			clocks = <&clock_top1 DOUT_SCLK_PHY_FSYS1_26M>,
+				 <&clock_fsys1 SCLK_COMBO_PHY_EMBEDDED_26M>,
+				 <&clock_fsys1 PHYCLK_UFS20_RX1_SYMBOL_USER>,
+				 <&clock_fsys1 PHYCLK_UFS20_RX0_SYMBOL_USER>,
+				 <&clock_fsys1 PHYCLK_UFS20_TX0_SYMBOL_USER>;
+			clock-names = "ref_clk_parent", "ref_clk",
+					"rx1_symbol_clk", "rx0_symbol_clk",
+					"tx0_symbol_clk";
 		};
 
 		usbdrd_phy: phy@15500000 {
diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
index 572e40e72776..2d5dcd670927 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.c
+++ b/drivers/phy/samsung/phy-samsung-ufs.c
@@ -136,9 +136,63 @@ int samsung_ufs_phy_calibrate(struct phy *phy)
 	return err;
 }
 
+static int samsung_ufs_phy_symbol_clk_init(struct samsung_ufs_phy *phy)
+{
+	struct clk *clk;
+	int ret = 0;
+
+	clk = devm_clk_get(phy->dev, "tx0_symbol_clk");
+	if (IS_ERR(clk)) {
+		dev_err(phy->dev, "failed to get tx0_symbol_clk clock\n");
+		goto out;
+	} else {
+		phy->tx0_symbol_clk = clk;
+	}
+
+	clk = devm_clk_get(phy->dev, "rx0_symbol_clk");
+	if (IS_ERR(clk)) {
+		dev_err(phy->dev, "failed to get rx0_symbol_clk clock\n");
+		goto out;
+	} else {
+		phy->rx0_symbol_clk = clk;
+	}
+
+	clk = devm_clk_get(phy->dev, "rx1_symbol_clk");
+	if (IS_ERR(clk)) {
+		dev_err(phy->dev, "failed to get rx1_symbol_clk clock\n");
+		goto out;
+	} else {
+		phy->rx1_symbol_clk = clk;
+	}
+
+	ret = clk_prepare_enable(phy->tx0_symbol_clk);
+	if (ret) {
+		dev_err(phy->dev, "%s: tx0_symbol_clk enable failed %d\n",
+				__func__, ret);
+		goto out;
+	}
+	ret = clk_prepare_enable(phy->rx0_symbol_clk);
+	if (ret) {
+		dev_err(phy->dev, "%s: rx0_symbol_clk enable failed %d\n",
+				__func__, ret);
+		goto out;
+	}
+	ret = clk_prepare_enable(phy->rx1_symbol_clk);
+	if (ret) {
+		dev_err(phy->dev, "%s: rx1_symbol_clk enable failed %d\n",
+				__func__, ret);
+		goto out;
+	}
+out:
+	return ret;
+}
+
 static int samsung_ufs_phy_clks_init(struct samsung_ufs_phy *phy)
 {
 	struct clk *child, *parent;
+	u32 phy_clk_rate, phy_parent_rate;
+	int ret;
+
 
 	child = devm_clk_get(phy->dev, "ref_clk");
 	if (IS_ERR(child))
@@ -146,18 +200,32 @@ static int samsung_ufs_phy_clks_init(struct samsung_ufs_phy *phy)
 	else
 		phy->ref_clk = child;
 
+	ret = clk_prepare_enable(phy->ref_clk);
+	if (ret) {
+		dev_err(phy->dev, "%s: ref_clk enable failed %d\n",
+				__func__, ret);
+		return ret;
+	}
+
+	phy_clk_rate = clk_get_rate(child);
+	dev_info(phy->dev, "MPHY ref_clk_rate = %d\n", phy_clk_rate);
+
 	parent = devm_clk_get(phy->dev, "ref_clk_parent");
 	if (IS_ERR(parent))
 		dev_err(phy->dev, "failed to get ref_clk_parent clock\n");
 	else
 		phy->ref_clk_parent = parent;
 
+	phy_parent_rate = clk_get_rate(parent);
+	dev_info(phy->dev, "MPHY ref_parent_clk_rate = %d\n", phy_parent_rate);
+
 	return clk_set_parent(child, parent);
 }
 
 static int samsung_ufs_phy_init(struct phy *phy)
 {
 	struct samsung_ufs_phy *_phy = get_samsung_ufs_phy(phy);
+	int ret;
 
 	_phy->lane_cnt = phy->attrs.bus_width;
 	_phy->ufs_phy_state = CFG_PRE_INIT;
@@ -167,7 +235,13 @@ static int samsung_ufs_phy_init(struct phy *phy)
 	_phy->is_pre_pmc = false;
 	_phy->is_post_pmc = false;
 
-	samsung_ufs_phy_clks_init(_phy);
+	ret = samsung_ufs_phy_symbol_clk_init(_phy);
+	if (ret)
+		dev_err(_phy->dev, "failed to set ufs phy symbol clocks\n");
+
+	ret = samsung_ufs_phy_clks_init(_phy);
+	if (!ret)
+		dev_err(_phy->dev, "failed to set ufs phy  clocks\n");
 
 	samsung_ufs_phy_calibrate(phy);
 
@@ -177,14 +251,6 @@ static int samsung_ufs_phy_init(struct phy *phy)
 static int samsung_ufs_phy_power_on(struct phy *phy)
 {
 	struct samsung_ufs_phy *_phy = get_samsung_ufs_phy(phy);
-	int ret;
-
-	ret = clk_prepare_enable(_phy->ref_clk);
-	if (ret) {
-		dev_err(_phy->dev, "%s: ref_clk enable failed %d\n",
-				__func__, ret);
-		return ret;
-	}
 
 	samsung_ufs_phy_ctrl_isol(_phy, false);
 	return 0;
diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index 971d67ae7f80..27dc1b573469 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -110,6 +110,9 @@ struct samsung_ufs_phy {
 	struct regmap *reg_pmu;
 	struct clk *ref_clk;
 	struct clk *ref_clk_parent;
+	struct clk *tx0_symbol_clk;
+	struct clk *rx0_symbol_clk;
+	struct clk *rx1_symbol_clk;
 	const struct samsung_ufs_phy_drvdata *drvdata;
 	struct samsung_ufs_phy_cfg **cfg;
 	const struct pmu_isol *isol;