scsi: ufs: add 2 lane support
PATCH 1/1 scsi: ufs: add 2 lane support
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
drivers/scsi/ufs/ufs-qcom.c +9 -11
@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; -if (host->hba->lanes_per_direction >1 ) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk);
@@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)if (err) goto disable_tx_l0; - err =ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , - host->tx_l1_sync_clk); -if (err) - goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this optional */ +if (host->tx_l1_sync_clk) +ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , + host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out; -disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:
@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */ +ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk); } out:return err;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Can,
Quoted 30 lines · Can Guo
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
Signed-off-by Venkat Gopalakrishnan <[email protected]>Signed-off-by Subhash Jadavani <[email protected]>Signed-off-by Can Guo <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-2@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; -if (host->hba->lanes_per_direction >1 ) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk);
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host) · line 150
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +1
-4@@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)if (err) goto disable_tx_l0; - err =ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , - host->tx_l1_sync_clk); -if (err) - goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this optional */
You need a similar change for "rx_l1_sync_clk" also.
And also get rid of 'lanes_per_direction' flag as well for ufs_qcom_enable_lane_clks() and ufs_qcom_init_lane_clks() too, as you are doing in ufs_qcom_disable_lane_clks().
Quoted 15 lines
drivers/scsi/ufs/ufs-qcom.c +3
-3+if (host->tx_l1_sync_clk) +ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , + host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out; -disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host) · line 191
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +3
-2@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */ +ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk);
same here.
Best regards
Vivek
Quoted 3 lines
drivers/scsi/ufs/ufs-qcom.c +0
-0} out:return err;
Quoted 39 lines · Vivek Gautam
Hi Can,
Quoted 35 lines · Can Guo
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
Signed-off-by Venkat Gopalakrishnan <[email protected]>Signed-off-by Subhash Jadavani <[email protected]>Signed-off-by Can Guo <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-1@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; - if (host->hba->lanes_per_direction > 1) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk); @@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct
ufs_qcom_host*host)
if (err)
gotodisable_tx_l0;
- err = ufs_qcom_host_clk_enable(dev, "tx_lane1_sync_clk",drivers/scsi/ufs/ufs-qcom.c +0
-2- host->tx_l1_sync_clk); -if (err)
Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host) · line 150
Quoted 2 lines
drivers/scsi/ufs/ufs-qcom.c +1
-1- goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this optional */ You need a similar change for "
rx_l1_sync_clk" also. And also get rid of 'lanes_per_direction' flag as well forufs_qcom_enable_lane_clks()
andufs_qcom_init_lane_clks()too, as you are doing inufs_qcom_disable_lane_clks().
Sure, got it.
Thanks
Can
Quoted 24 lines
Quoted 13 lines
drivers/scsi/ufs/ufs-qcom.c +3
-2+if (host->tx_l1_sync_clk) +ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , + host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out; -disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_init_lane_clks(struct · line 192
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +3
-1@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */ +ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk);same here.
Quoted 3 lines
Best regards
Vivek
Shall do as well.
Thanks
Can
Quoted 3 lines
Quoted 3 lines
drivers/scsi/ufs/ufs-qcom.c +0
-0} out:return err;
Hi,
Quoted 5 lines · Can Guo
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
I'm reviving this old chestnut, sorry I missed it initially. This description is a little terse, and I'm actually confused about it. The description makes it sounds like this patch is adding support for 2-lane UFS controllers, but the patch itself appears to only make the UFS controller tolerant of a missing lane (or more specifically, a missing lane clock). Can you describe a little more about what's going on here, and perhaps fix the description?
I notice that the global clock controller has clocks for TX symbol 0, and RX symbol 1, but seems to be missing GCC_UFS_PHY_TX_SYMBOL_1_CLK. Was that an oversight, or is it really not there?
Quoted 26 lines
Signed-off-by Venkat Gopalakrishnan <[email protected]>Signed-off-by Subhash Jadavani <[email protected]>Signed-off-by Can Guo <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-2@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; -if (host->hba->lanes_per_direction >1 ) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk);
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host) · line 150
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +1
-4@@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)if (err) goto disable_tx_l0; - err =ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , - host->tx_l1_sync_clk); -if (err) - goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this optional */
I'm confused by this comment. What do you mean the clock could be muxed?
Quoted 26 lines
drivers/scsi/ufs/ufs-qcom.c +6
-5+if (host->tx_l1_sync_clk) +ufs_qcom_host_clk_enable (dev,"tx_lane1_sync_clk" , + host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out; -disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */ +ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk); } out:return err;
Quoted 52 lines · Evan Green
Hi,
Quoted 5 lines · Can Guo
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
I'm reviving this old chestnut, sorry I missed it initially. This description is a little terse, and I'm actually confused about it. The description makes it sounds like this patch is adding support for 2-lane UFS controllers, but the patch itself appears to only make the UFS controller tolerant of a missing lane (or more specifically, a missing lane clock). Can you describe a little more about what's going on here, and perhaps fix the description?
I notice that the global clock controller has clocks for TX symbol 0, and RX symbol 1, but seems to be missing
GCC_UFS_PHY_TX_SYMBOL_1_CLK. Was that an oversight, or is it really not there?Quoted 31 lines
Signed-off-by Venkat Gopalakrishnan <[email protected]>Signed-off-by Subhash Jadavani <[email protected]>Signed-off-by Can Guo <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-2@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; -if (host->hba->lanes_per_direction >1 ) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk);@@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)if (err) goto disable_tx_l0;
Quoted 10 lines
Quoted 7 lines
drivers/scsi/ufs/ufs-qcom.c +1
-4- err =ufs_qcom_host_clk_enable (dev, "tx_lane1_sync_clk", - host->tx_l1_sync_clk); -if (err) - goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this optional */
I'm confused by this comment. What do you mean the clock could be muxed?
Hi Evan, sorry for the late response, I totally missed this mail. Here it means Tx Lane1 clock can be muxed with Tx Lane0 clock. As by design, Tx Lane1 clock derives from Tx Lane0 clock.
I pushed a new change of it as I change the commit name. Please help review the new patch. Sorry for the inconveience.
Quoted 30 lines
Quoted 30 lines
drivers/scsi/ufs/ufs-qcom.c +2
-0+if (host->tx_l1_sync_clk) + ufs_qcom_host_clk_enable(dev,"
tx_lane1_sync_clk",
+host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out;drivers/scsi/ufs/ufs-qcom.c +1
-5-disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */
drivers/scsi/ufs/ufs-qcom.c +2
-0+ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk); } out:return err;
Hi Evan,
Quoted 17 lines · Evan Green
Hi,
Quoted 5 lines · Can Guo
From: Venkat Gopalakrishnan <[email protected]>
Qcom ufs controller v3.1.0 supports 2 lanes, add support to configure 2 lanes during phy initialization.
I'm reviving this old chestnut, sorry I missed it initially. This description is a little terse, and I'm actually confused about it. The description makes it sounds like this patch is adding support for 2-lane UFS controllers, but the patch itself appears to only make the UFS controller tolerant of a missing lane (or more specifically, a missing lane clock). Can you describe a little more about what's going on here, and perhaps fix the description?
Quoted 4 lines
I notice that the global clock controller has clocks for TX symbol 0, and RX symbol 1, but seems to be missing
GCC_UFS_PHY_TX_SYMBOL_1_CLK. Was that an oversight, or is it really not there?
You are right. The name and commit message are not representing itself correctly as most of the original commit has been upstreamed already. I uploaded a new patch to address it.
As per Qcom's design Tx Lane1 clock derives from Tx Lane0. So only one Tx Lane0 clock would make 2 Tx lanes work.
Quoted 71 lines
Quoted 37 lines
Signed-off-by Venkat Gopalakrishnan <[email protected]>Signed-off-by Subhash Jadavani <[email protected]>Signed-off-by Can Guo <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)drivers/scsi/ufs/ufs-qcom.c +3
-6@@ -113,10 +113,10 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)if (!host->is_lane_clks_enabled)return ; -if (host->hba->lanes_per_direction >1 ) +if (host->tx_l1_sync_clk)clk_disable_unprepare (host->tx_l1_sync_clk);clk_disable_unprepare (host->tx_l0_sync_clk); -if (host->hba->lanes_per_direction >1 ) +if (host->rx_l1_sync_clk)clk_disable_unprepare (host->rx_l1_sync_clk);clk_disable_unprepare (host->rx_l0_sync_clk);@@ -147,18 +147,15 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)if (err) goto disable_tx_l0; - err =ufs_qcom_host_clk_enable (dev, "tx_lane1_sync_clk", - host->tx_l1_sync_clk); -if (err) - goto disable_rx_l1; +/* The tx lane1 clk could be muxed, hence keep this Quoted 1 line
optional */
I'm confused by this comment. What do you mean the clock could be muxed?
Quoted 30 lines
drivers/scsi/ufs/ufs-qcom.c +2
-0+if (host->tx_l1_sync_clk) + ufs_qcom_host_clk_enable(dev,"
tx_lane1_sync_clk",
+host->tx_l1_sync_clk); } host->is_lane_clks_enabled = true; goto out;drivers/scsi/ufs/ufs-qcom.c +1
-5-disable_rx_l1: -if (host->hba->lanes_per_direction >1 ) -clk_disable_unprepare (host->rx_l1_sync_clk); disable_tx_l0:clk_disable_unprepare (host->tx_l0_sync_clk); disable_rx_l0:@@ -189,8 +186,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)if (err) goto out; - err =ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , - &host->tx_l1_sync_clk); +/* The tx lane1 clk could be muxed, hence keep this optional */
drivers/scsi/ufs/ufs-qcom.c +2
-0+ufs_qcom_host_clk_get (dev,"tx_lane1_sync_clk" , + &host->tx_l1_sync_clk); } out:return err;