Re: [PATCH v5 2/7] scsi: ufs-qcom: Add reset control support for host controller

From: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: Can Guo <cang@codeaurora.org>,
	asutoshd@codeaurora.org, nguyenb@codeaurora.org,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-scsi@vger.kernel.org, kernel-team@android.com,
	saravanak@google.com, Mark Salyzyn <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: Thu, 19 Dec 2019 07:25:19 -0700	[thread overview]
Message-ID: <CAOCk7NrKRXsTffNyQFt_tQmdNq7+SaH+kAJVk8AAPJWJjPxYYw@mail.gmail.com> (raw)
In-Reply-To: <20191219142145.GV2536@vkoul-mobl>

On Thu, Dec 19, 2019 at 7:21 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 19-12-19, 15:12, cang@codeaurora.org wrote:
> > On 2019-12-18 12:12, Vinod Koul wrote:
> > > On 18-12-19, 02:44, cang@codeaurora.org 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?