Add reset control for host controller so that host controller can be reset
as required in its power up sequence.
Signed-off-by: Can Guo <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/ufs-qcom.h | 3 +++
2 files changed, 56 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index a5b7148..c69c29a1c 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -246,6 +246,44 @@ static void ufs_qcom_select_unipro_mode(struct ufs_qcom_host *host)
mb();
}
+/**
+ * ufs_qcom_host_reset - reset host controller and PHY
+ */
+static int ufs_qcom_host_reset(struct ufs_hba *hba)
+{
+ int ret = 0;
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+ if (!host->core_reset) {
+ dev_warn(hba->dev, "%s: reset control not set\n", __func__);
+ goto out;
+ }
+
+ ret = reset_control_assert(host->core_reset);
+ if (ret) {
+ dev_err(hba->dev, "%s: core_reset assert failed, err = %d\n",
+ __func__, ret);
+ goto out;
+ }
+
+ /*
+ * The hardware requirement for delay between assert/deassert
+ * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
+ * ~125us (4/32768). To be on the safe side add 200us delay.
+ */
+ usleep_range(200, 210);
+
+ ret = reset_control_deassert(host->core_reset);
+ if (ret)
+ dev_err(hba->dev, "%s: core_reset deassert failed, err = %d\n",
+ __func__, ret);
+
+ usleep_range(1000, 1100);
+
+out:
+ return ret;
+}
+
static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
{
struct ufs_qcom_host *host = ufshcd_get_variant(hba);
@@ -254,6 +292,12 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
? true : false;
+ /* Reset UFS Host Controller and PHY */
+ ret = ufs_qcom_host_reset(hba);
+ if (ret)
+ dev_warn(hba->dev, "%s: host reset returned %d\n",
+ __func__, ret);
+
if (is_rate_B)
phy_set_mode(phy, PHY_MODE_UFS_HS_B);
@@ -1101,6 +1145,15 @@ static int ufs_qcom_init(struct ufs_hba *hba)
host->hba = hba;
ufshcd_set_variant(hba, host);
+ /* Setup the reset control of HCI */
+ host->core_reset = devm_reset_control_get(hba->dev, "rst");
+ if (IS_ERR(host->core_reset)) {
+ err = PTR_ERR(host->core_reset);
+ dev_warn(dev, "Failed to get reset control %d\n", err);
+ host->core_reset = NULL;
+ err = 0;
+ }
+
/* Fire up the reset controller. Failure here is non-fatal. */
host->rcdev.of_node = dev->of_node;
host->rcdev.ops = &ufs_qcom_reset_ops;
diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
index d401f17..2d95e7c 100644
--- a/drivers/scsi/ufs/ufs-qcom.h
+++ b/drivers/scsi/ufs/ufs-qcom.h
@@ -6,6 +6,7 @@
#define UFS_QCOM_H_
#include <linux/reset-controller.h>
+#include <linux/reset.h>
#define MAX_UFS_QCOM_HOSTS 1
#define MAX_U32 (~(u32)0)
@@ -233,6 +234,8 @@ struct ufs_qcom_host {
u32 dbg_print_en;
struct ufs_qcom_testbus testbus;
+ /* Reset control of HCI */
+ struct reset_control *core_reset;
struct reset_controller_dev rcdev;
struct gpio_desc *device_reset;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
>
> Add reset control for host controller so that host controller can be reset as
> required in its power up sequence.
>
> Signed-off-by: Can Guo <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
On 2019-12-18 12:12, Vinod Koul wrote:
> On 18-12-19, 02:44, [email protected] wrote:
>
>> Hi Vinod and Jeffrey,
>>
>> Let me summary here, now the 1000000us timeout works for both 845 and
>> 8998.
>> However, 8150 still fails.
>>
>> > > The bigger question is why is the reset causing the timeout to be
>> > > increased for sdm845 and not to work in case of sm8150! (Vinod)
>>
>> I would not say this patch increases the timeout. With this patch,
>> the PCS polling timeout, per my profiling, the PCS ready usually needs
>> less than 5000us, which is the actual time needed for PCS bit to be
>> ready.
>>
>> The reason why 1000us worked for you is because, w/o the patch, UFS
>> PHY
>> registers are retained from pre-kernel stage (bootloader i.e.), the
>> PCS
>> ready
>> bit was set to 1 in pre-kernel stage, so when kernel driver reads it,
>> it
>> returns
>> 1, not even to be polled at all. It may seem "faster", but not the
>> right
>> thing to do, because kernel stage may need different PHY settings than
>> pre-kernel stage, keeping the settings configured in pre-kernel stage
>> is not
>> always right, so this patch is needed. And increasing 1000us to
>> 1000000us
>> is the right thing to do, but not a hack.
>>
>> As reg for the phy initialization timeout on 8150, I found there is
>> something
>> wrong with its settings in /drivers/phy/qualcomm/phy-qcom-qmp.c
>>
>> static const struct qmp_phy_init_tbl sm8150_ufsphy_serdes_tbl[] = {
>> QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x01),
>> QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_EN_SEL, 0xd9),
>>
>> "QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x01)" should NOT appear in
>> the
>> serdes
>> table! I haven't check who made this change, but please have a try
>> after
>> remove
>> this line from sm8150_ufsphy_serdes_tbl.
>
> That is me :) Looks like I made an error while porting from downstream.
> I
> did a quick check to remove this and it doesn't work yet, let me
> recheck
> the settings again ...
>
> Thanks for your help!
Hi Vinod,
Indeed, you need to tweak your settings. I spent some time help you
figure this out. Try below change and please let me know if it can
resolve your problem.
I would not say this is a regression caused by my patch, it is just
my patch reveals something incorrect in the settings.
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 8e642a6..0cc9044 100755
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -66,7 +66,7 @@
/* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
#define CLAMP_EN BIT(0) /* enables i/o
clamp_n */
-#define PHY_INIT_COMPLETE_TIMEOUT 1000
+#define PHY_INIT_COMPLETE_TIMEOUT 1000000
#define POWER_DOWN_DELAY_US_MIN 10
#define POWER_DOWN_DELAY_US_MAX 11
@@ -166,6 +166,7 @@ static const unsigned int
sdm845_ufsphy_regs_layout[] = {
};
static const unsigned int sm8150_ufsphy_regs_layout[] = {
+ [QPHY_SW_RESET] = 0x08,
[QPHY_START_CTRL] = 0x00,
[QPHY_PCS_READY_STATUS] = 0x180,
};
@@ -885,7 +886,6 @@ static const struct qmp_phy_init_tbl
msm8998_usb3_pcs_tbl[] = {
};
static const struct qmp_phy_init_tbl sm8150_ufsphy_serdes_tbl[] = {
- QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x01),
QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_EN_SEL, 0xd9),
QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x11),
QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_HS_SWITCH_SEL, 0x00),
@@ -1390,7 +1390,6 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg
= {
.pwrdn_ctrl = SW_PWRDN,
.is_dual_lane_phy = true,
- .no_pcs_sw_reset = true,
};
static void qcom_qmp_phy_configure(void __iomem *base,
---
Aside of the phy settings, your DT needs some modifications too,
seems you copied most of them from sdm845.
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=3834a2e92229ef26d30de28acb698b2b23d3e397
<--snip-->
> + ufs_mem_phy: phy@1d87000 {
> + compatible = "qcom,sm8150-qmp-ufs-phy";
> + reg = <0 0x01d87000 0 0x18c>;
The size 0x18c is wrong, in the code you are even accessing registers
whose offsets are beyond 0x18c, see
#define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0 0x1ac
#define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0
#define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4
#define QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc
#define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8
FYI, the total size of serdes registers is 0x1c0.
<--snip-->
> + ufs_mem_phy_lanes: lanes@1d87400 {
> + reg = <0 0x01d87400 0 0x108>,
> + <0 0x01d87600 0 0x1e0>,
> + <0 0x01d87c00 0 0x1dc>,
Same as above, see
#define QPHY_V4_MULTI_LANE_CTRL1 0x1e0
FYI, the total size of PCS registers is 0x200
> + <0 0x01d87800 0 0x108>,
> + <0 0x01d87a00 0 0x1e0>;
> + #phy-cells = <0>;
> + };
<--snip-->
On 19-12-19, 15:12, [email protected] wrote:
> On 2019-12-18 12:12, Vinod Koul wrote:
> > On 18-12-19, 02:44, [email protected] wrote:
>
> Aside of the phy settings, your DT needs some modifications too,
> seems you copied most of them from sdm845.
> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=3834a2e92229ef26d30de28acb698b2b23d3e397
>
> <--snip-->
> > + ufs_mem_phy: phy@1d87000 {
> > + compatible = "qcom,sm8150-qmp-ufs-phy";
> > + reg = <0 0x01d87000 0 0x18c>;
>
> The size 0x18c is wrong, in the code you are even accessing registers
> whose offsets are beyond 0x18c, see
>
> #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0 0x1ac
> #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0
> #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4
> #define QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc
> #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8
>
> FYI, the total size of serdes registers is 0x1c0.
Yeah I will update it to 0x1c0
>
> <--snip-->
> > + ufs_mem_phy_lanes: lanes@1d87400 {
> > + reg = <0 0x01d87400 0 0x108>,
> > + <0 0x01d87600 0 0x1e0>,
> > + <0 0x01d87c00 0 0x1dc>,
>
> Same as above, see
>
> #define QPHY_V4_MULTI_LANE_CTRL1 0x1e0
>
> FYI, the total size of PCS registers is 0x200
>
> > + <0 0x01d87800 0 0x108>,
> > + <0 0x01d87a00 0 0x1e0>;
> > + #phy-cells = <0>;
> > + };
> <--snip-->
So I managed to fix it by configuring QPHY_SW_RESET in
qcom_qmp_phy_com_init() before invoking the configuration. That makes it
work for me. Will send patches shortly
--
~Vinod
On Thu, Dec 19, 2019 at 7:21 AM Vinod Koul <[email protected]> wrote:
>
> On 19-12-19, 15:12, [email protected] wrote:
> > On 2019-12-18 12:12, Vinod Koul wrote:
> > > On 18-12-19, 02:44, [email protected] wrote:
>
> >
> > Aside of the phy settings, your DT needs some modifications too,
> > seems you copied most of them from sdm845.
> > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=3834a2e92229ef26d30de28acb698b2b23d3e397
> >
> > <--snip-->
> > > + ufs_mem_phy: phy@1d87000 {
> > > + compatible = "qcom,sm8150-qmp-ufs-phy";
> > > + reg = <0 0x01d87000 0 0x18c>;
> >
> > The size 0x18c is wrong, in the code you are even accessing registers
> > whose offsets are beyond 0x18c, see
> >
> > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0 0x1ac
> > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0
> > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4
> > #define QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc
> > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8
> >
> > FYI, the total size of serdes registers is 0x1c0.
>
> Yeah I will update it to 0x1c0
>
> >
> > <--snip-->
> > > + ufs_mem_phy_lanes: lanes@1d87400 {
> > > + reg = <0 0x01d87400 0 0x108>,
> > > + <0 0x01d87600 0 0x1e0>,
> > > + <0 0x01d87c00 0 0x1dc>,
> >
> > Same as above, see
> >
> > #define QPHY_V4_MULTI_LANE_CTRL1 0x1e0
> >
> > FYI, the total size of PCS registers is 0x200
> >
> > > + <0 0x01d87800 0 0x108>,
> > > + <0 0x01d87a00 0 0x1e0>;
> > > + #phy-cells = <0>;
> > > + };
> > <--snip-->
>
> So I managed to fix it by configuring QPHY_SW_RESET in
> qcom_qmp_phy_com_init() before invoking the configuration. That makes it
> work for me. Will send patches shortly
So, you are going to send some fixes to make sm8150 work. We also
need the extended timeout for all platforms, yes? Who is going to
send up the patch for the timeout? All of this should be -rc material
since Can's change caused these issues to appear, and thus impact
users, no?
On 19-12-19, 07:25, Jeffrey Hugo wrote:
> On Thu, Dec 19, 2019 at 7:21 AM Vinod Koul <[email protected]> wrote:
> >
> > On 19-12-19, 15:12, [email protected] wrote:
> > > On 2019-12-18 12:12, Vinod Koul wrote:
> > > > On 18-12-19, 02:44, [email protected] wrote:
> >
> > >
> > > Aside of the phy settings, your DT needs some modifications too,
> > > seems you copied most of them from sdm845.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=3834a2e92229ef26d30de28acb698b2b23d3e397
> > >
> > > <--snip-->
> > > > + ufs_mem_phy: phy@1d87000 {
> > > > + compatible = "qcom,sm8150-qmp-ufs-phy";
> > > > + reg = <0 0x01d87000 0 0x18c>;
> > >
> > > The size 0x18c is wrong, in the code you are even accessing registers
> > > whose offsets are beyond 0x18c, see
> > >
> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0 0x1ac
> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0
> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4
> > > #define QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc
> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8
> > >
> > > FYI, the total size of serdes registers is 0x1c0.
> >
> > Yeah I will update it to 0x1c0
> >
> > >
> > > <--snip-->
> > > > + ufs_mem_phy_lanes: lanes@1d87400 {
> > > > + reg = <0 0x01d87400 0 0x108>,
> > > > + <0 0x01d87600 0 0x1e0>,
> > > > + <0 0x01d87c00 0 0x1dc>,
> > >
> > > Same as above, see
> > >
> > > #define QPHY_V4_MULTI_LANE_CTRL1 0x1e0
> > >
> > > FYI, the total size of PCS registers is 0x200
> > >
> > > > + <0 0x01d87800 0 0x108>,
> > > > + <0 0x01d87a00 0 0x1e0>;
> > > > + #phy-cells = <0>;
> > > > + };
> > > <--snip-->
> >
> > So I managed to fix it by configuring QPHY_SW_RESET in
> > qcom_qmp_phy_com_init() before invoking the configuration. That makes it
> > work for me. Will send patches shortly
>
> So, you are going to send some fixes to make sm8150 work. We also
> need the extended timeout for all platforms, yes? Who is going to
> send up the patch for the timeout? All of this should be -rc material
> since Can's change caused these issues to appear, and thus impact
> users, no?
yeah I have tested a timeout of 10ms and seems to look good for me on
sm8150 and sdm845. I will be sending the patches in few minutes :) and
yes the timeout should be marked to 5.5 fixes
Thanks
--
~Vinod
On 2019-12-19 22:52, Vinod Koul wrote:
> On 19-12-19, 07:25, Jeffrey Hugo wrote:
>> On Thu, Dec 19, 2019 at 7:21 AM Vinod Koul <[email protected]> wrote:
>> >
>> > On 19-12-19, 15:12, [email protected] wrote:
>> > > On 2019-12-18 12:12, Vinod Koul wrote:
>> > > > On 18-12-19, 02:44, [email protected] wrote:
>> >
>> > >
>> > > Aside of the phy settings, your DT needs some modifications too,
>> > > seems you copied most of them from sdm845.
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=3834a2e92229ef26d30de28acb698b2b23d3e397
>> > >
>> > > <--snip-->
>> > > > + ufs_mem_phy: phy@1d87000 {
>> > > > + compatible = "qcom,sm8150-qmp-ufs-phy";
>> > > > + reg = <0 0x01d87000 0 0x18c>;
>> > >
>> > > The size 0x18c is wrong, in the code you are even accessing registers
>> > > whose offsets are beyond 0x18c, see
>> > >
>> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0 0x1ac
>> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0 0x1b0
>> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1 0x1b4
>> > > #define QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL 0x1bc
>> > > #define QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1 0x1b8
>> > >
>> > > FYI, the total size of serdes registers is 0x1c0.
>> >
>> > Yeah I will update it to 0x1c0
>> >
>> > >
>> > > <--snip-->
>> > > > + ufs_mem_phy_lanes: lanes@1d87400 {
>> > > > + reg = <0 0x01d87400 0 0x108>,
>> > > > + <0 0x01d87600 0 0x1e0>,
>> > > > + <0 0x01d87c00 0 0x1dc>,
>> > >
>> > > Same as above, see
>> > >
>> > > #define QPHY_V4_MULTI_LANE_CTRL1 0x1e0
>> > >
>> > > FYI, the total size of PCS registers is 0x200
>> > >
>> > > > + <0 0x01d87800 0 0x108>,
>> > > > + <0 0x01d87a00 0 0x1e0>;
>> > > > + #phy-cells = <0>;
>> > > > + };
>> > > <--snip-->
>> >
>> > So I managed to fix it by configuring QPHY_SW_RESET in
>> > qcom_qmp_phy_com_init() before invoking the configuration. That makes it
>> > work for me. Will send patches shortly
>>
>> So, you are going to send some fixes to make sm8150 work. We also
>> need the extended timeout for all platforms, yes? Who is going to
>> send up the patch for the timeout? All of this should be -rc material
>> since Can's change caused these issues to appear, and thus impact
>> users, no?
>
> yeah I have tested a timeout of 10ms and seems to look good for me on
> sm8150 and sdm845. I will be sending the patches in few minutes :) and
> yes the timeout should be marked to 5.5 fixes
>
> Thanks
Hi Vinod,
Good to know it works for you. Vivek Gautam, who is the author QCOM UFS
PHY
driver, has left QCOM. Please add Asutosh Das([email protected]),
Bao D. Nguyen([email protected]) and me for QCOM UFS PHY changes.
Actually, without this change, your PHY is not even re-initialized at
all
during kernel bootup, it just retains whatever it was configured in
UEFI,
yet you are still saying this is a regression. The extended timeout is
what
UFS PHY has to take for a full initialization.
Regards,
Can Guo.