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

[Vinod Koul <vkoul@kernel.org>]

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