linux-kernel - Re: [PATCH v2 3/3] phy: qcom-qmp: Add SM8150 QMP UFS PHY support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190918173740.GQ4392@vkoul-mobl>
Date:   Wed, 18 Sep 2019 23:07:40 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Marc Gonzalez <marc.w.gonzalez@...e.fr>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        MSM <linux-arm-msm@...r.kernel.org>,
        Andy Gross <agross@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH v2 3/3] phy: qcom-qmp: Add SM8150 QMP UFS PHY support

On 18-09-19, 17:08, Marc Gonzalez wrote:
> On 06/09/2019 07:10, Vinod Koul wrote:

> > +	QMP_PHY_INIT_CFG(QSERDES_COM_V4_LOCK_CMP2_MODE1, 0x0f),
> > +	QMP_PHY_INIT_CFG(QSERDES_COM_V4_BIN_VCOCAL_CMP_CODE1_MODE1, 0xdd),
> > +	QMP_PHY_INIT_CFG(QSERDES_COM_V4_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
> > +
> > +	/* Rate B */
> > +	QMP_PHY_INIT_CFG(QSERDES_COM_V4_VCO_TUNE_MAP, 0x06),
> 
> IMO, the name of the symbolic constants should be QSERDES_V4_COM*
> (like below for QSERDES_V4_TX* and QSERDES_V4_RX*)

Agreed this should have been QSERDES_V4_COM_*
 
> > +static const char * const sm8150_ufs_phy_clk_l[] = {
> > +	"ref", "ref_aux",
> > +};
> > +
> 
> Why not just reuse sdm845_ufs_phy_clk_l?

I think that is a good idea, will do

> > +static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
> > +	.type			= PHY_TYPE_UFS,
> > +	.nlanes			= 2,
> > +
> > +	.serdes_tbl		= sm8150_ufsphy_serdes_tbl,
> > +	.serdes_tbl_num		= ARRAY_SIZE(sm8150_ufsphy_serdes_tbl),
> > +	.tx_tbl			= sm8150_ufsphy_tx_tbl,
> > +	.tx_tbl_num		= ARRAY_SIZE(sm8150_ufsphy_tx_tbl),
> > +	.rx_tbl			= sm8150_ufsphy_rx_tbl,
> > +	.rx_tbl_num		= ARRAY_SIZE(sm8150_ufsphy_rx_tbl),
> > +	.pcs_tbl		= sm8150_ufsphy_pcs_tbl,
> > +	.pcs_tbl_num		= ARRAY_SIZE(sm8150_ufsphy_pcs_tbl),
> > +	.clk_list		= sm8150_ufs_phy_clk_l,
> > +	.num_clks		= ARRAY_SIZE(sm8150_ufs_phy_clk_l),
> > +	.vreg_list		= qmp_phy_vreg_l,
> > +	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
> > +	.regs			= sm8150_ufsphy_regs_layout,
> > +
> > +	.start_ctrl		= SERDES_START,
> > +	.pwrdn_ctrl		= SW_PWRDN,
> > +	.mask_pcs_ready		= PCS_READY,
> 
> I think you need to rework your patch on top of
> "phy: qcom-qmp: Correct ready status, again"
> (it removed this field)

Yes will rebase on rc1 (when out) and resend, thanks for pointing

-- 
~Vinod