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

Jeffrey Hugo
2019-12-19T14:25:32Z

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?