Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
From: cang@codeaurora.org
To: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	asutoshd@codeaurora.org, nguyenb@codeaurora.org,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-scsi@vger.kernel.org, kernel-team@android.com,
	saravanak@google.com, salyzyn@google.com,
	Andy Gross <agross@kernel.org>,
	Alim Akhtar <alim.akhtar@samsung.com>,
	Avri Altman <avri.altman@wdc.com>,
	Pedro Sousa <pedrom.sousa@synopsys.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	"open list:ARM/QUALCOMM SUPPORT" <linux-arm-msm@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 2/7] scsi: ufs-qcom: Add reset control support for host controller
Date: Tue, 17 Dec 2019 08:37:14 +0800
Message-ID: <091562cbe7d88ca1c30638bc10197074@codeaurora.org> (raw)
In-Reply-To: <CAOCk7NpAp+DHBp-owyKGgJFLRajfSQR6ff1XMmAj6A4nM3VnMQ@mail.gmail.com>

On 2019-12-17 03:12, Jeffrey Hugo wrote:
> On Mon, Dec 16, 2019 at 12:05 PM Vinod Koul <vkoul@kernel.org> wrote:
>> 
>> Hi Can,
>> 
>> On 14-11-19, 22:09, Can Guo wrote:
>> > Add reset control for host controller so that host controller can be reset
>> > as required in its power up sequence.
>> 
>> I am seeing a regression on UFS on SM8150-mtp with this patch. I think
>> Jeff is seeing same one lenove laptop on 8998.
> 
> Confirmed.
> 
>> 
>> 845 does not seem to have this issue and only thing I can see is that 
>> on
>> sm8150 and 8998 we define reset as:
>> 
>>                         resets = <&gcc GCC_UFS_BCR>;
>>                         reset-names = "rst";
>> 
>> Thanks
>> 
>> >
>> > Signed-off-by: Can Guo <cang@codeaurora.org>
>> > ---
>> >  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
>> 
>> --
>> ~Vinod

Hi Jeffrey and Vinod,

Thanks for reporting this. May I know what kind of regression do you see 
on
8150 and 8998?
BTW, do you have reset control for UFS PHY in your DT?
See 71278b058a9f8752e51030e363b7a7306938f64e.

FYI, we use reset control on all of our platforms and it is
a must during our power up sequence.

Thanks,
Can Guo.

  reply index

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-15  6:09 [PATCH v5 0/7] UFS driver general fixes bundle 3 Can Guo
2019-11-15  6:09 ` [PATCH v5 1/7] scsi: ufs: Add device reset in link recovery path Can Guo
2019-11-15  6:25   ` Stanley Chu
2019-11-15  6:09 ` [PATCH v5 2/7] scsi: ufs-qcom: Add reset control support for host controller Can Guo
2019-11-15 10:08   ` Avri Altman
2019-12-16 19:04   ` Vinod Koul
2019-12-16 19:12     ` Jeffrey Hugo
2019-12-17  0:37       ` cang [this message]
2019-12-17  4:13         ` Vinod Koul
2019-12-17  7:10           ` cang
2019-12-17  9:24             ` Vinod Koul
2019-12-17 10:09               ` cang
2019-12-17 15:08                 ` Vinod Koul
2019-12-17 16:00                   ` Jeffrey Hugo
2019-12-17 18:44                     ` cang
2019-12-18  4:12                       ` Vinod Koul
2019-12-19  7:12                         ` cang
2019-12-19 14:21                           ` Vinod Koul
2019-12-19 14:25                             ` Jeffrey Hugo
2019-12-19 14:52                               ` Vinod Koul
2019-12-20  0:30                                 ` cang
2019-11-15  6:09 ` [PATCH v5 3/7] scsi: ufs: Fix up auto hibern8 enablement Can Guo
2019-11-15  6:35   ` Stanley Chu
2019-11-15  7:03     ` Can Guo
2019-11-15  7:18       ` Stanley Chu
2019-11-15 12:27         ` Can Guo
2019-11-15 13:46           ` Stanley Chu
2019-11-15  6:09 ` [PATCH v5 4/7] scsi: ufs: Fix register dump caused sleep in atomic context Can Guo
2019-11-15  6:09 ` [PATCH v5 5/7] scsi: ufs: Fix irq return code Can Guo
2019-11-15  6:09 ` [PATCH v5 6/7] scsi: ufs: Abort gating if clock on request is pending Can Guo
2019-11-15  6:09 ` [PATCH v5 7/7] scsi: ufs: Fix error handing during hibern8 enter Can Guo
2019-11-15 10:12   ` Avri Altman
2019-11-17 16:36   ` [EXT] " Bean Huo (beanhuo)
2019-11-19  4:28 ` [PATCH v5 0/7] UFS driver general fixes bundle 3 Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=091562cbe7d88ca1c30638bc10197074@codeaurora.org \
    --to=cang@codeaurora.org \
    --cc=agross@kernel.org \
    --cc=alim.akhtar@samsung.com \
    --cc=asutoshd@codeaurora.org \
    --cc=avri.altman@wdc.com \
    --cc=jeffrey.l.hugo@gmail.com \
    --cc=jejb@linux.ibm.com \
    --cc=kernel-team@android.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nguyenb@codeaurora.org \
    --cc=pedrom.sousa@synopsys.com \
    --cc=rnayak@codeaurora.org \
    --cc=salyzyn@google.com \
    --cc=saravanak@google.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git