Re: [PATCH V3 2/2] phy: qcom-qmp-ufs: Add Phy Configuration support for SC7280

[Nitin Rawat]

On 9/16/2023 12:03 AM, Dmitry Baryshkov wrote:
> On Fri, 15 Sept 2023 at 19:14, Nitin Rawat <quic_nitirawa@xxxxxxxxxxx> wrote:
> > On 9/6/2023 1:34 AM, Dmitry Baryshkov wrote:
> > > On 23/08/2023 12:17, Nitin Rawat wrote:
> > > > Add SC7280 specific register layout and table configs.
> > > >
> > > > Co-developed-by: Manish Pandey <quic_mapa@xxxxxxxxxxx>
> > > > Signed-off-by: Manish Pandey <quic_mapa@xxxxxxxxxxx>
> > > > Signed-off-by: Nitin Rawat <quic_nitirawa@xxxxxxxxxxx>
> > > > ---
> > > >    drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 142 ++++++++++++++++++++++++
> > > >    1 file changed, 142 insertions(+)
> > > >
> > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> > > > b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> > > > index 3927eba8e468..514fa14df634 100644
> > > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> > > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> > >
> > > [skipped tables programming]
> > >
> > > 4),
> > Sorry I quite didn't get this comment. what exactly is skipped ?Please
> > can you help explain?
>
> I skipped them, as I didn't have comments for them.
>
> > > > @@ -888,6 +993,40 @@ static const struct qmp_phy_cfg
> > > > sa8775p_ufsphy_cfg = {
> > > >        .regs            = ufsphy_v5_regs_layout,
> > > >    };
> > > >
> > > > +static const struct qmp_phy_cfg sc7280_ufsphy_cfg = {
> > > > +    .lanes                  = 2,
> > > > +
> > > > +    .offsets                = &qmp_ufs_offsets,
> > > > +
> > > > +    .tbls = {
> > > > +        .serdes         = sm8150_ufsphy_serdes,
> > > > +        .serdes_num     = ARRAY_SIZE(sm8150_ufsphy_serdes),
> > > > +        .tx             = sc7280_ufsphy_tx,
> > > > +        .tx_num         = ARRAY_SIZE(sc7280_ufsphy_tx),
> > > > +        .rx             = sc7280_ufsphy_rx,
> > > > +        .rx_num         = ARRAY_SIZE(sc7280_ufsphy_rx),
> > > > +        .pcs            = sc7280_ufsphy_pcs,
> > > > +        .pcs_num        = ARRAY_SIZE(sc7280_ufsphy_pcs),
> > > > +    },
> > > > +    .tbls_hs_b = {
> > > > +        .serdes         = sm8150_ufsphy_hs_b_serdes,
> > > > +        .serdes_num     = ARRAY_SIZE(sm8150_ufsphy_hs_b_serdes),
> > > > +    },
> > > > +    .tbls_hs_g4 = {
> > > > +        .tx             = sm8250_ufsphy_hs_g4_tx,
> > > > +        .tx_num         = ARRAY_SIZE(sm8250_ufsphy_hs_g4_tx),
> > > > +        .rx             = sc7280_ufsphy_hs_g4_rx,
> > > > +        .rx_num         = ARRAY_SIZE(sc7280_ufsphy_hs_g4_rx),
> > > > +        .pcs            = sm8150_ufsphy_hs_g4_pcs,
> > > > +        .pcs_num        = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
> > > > +    },
> > > > +    .clk_list               = sm8450_ufs_phy_clk_l,
> > > > +    .num_clks               = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
> > >
> > > This doesn't correspond to the bindings. This array has 3 enries, while
> > > in the bindings you have opted for two clocks for this PHY.
> > Sure. I'll update the bindings.
>
> Are you sure about the third clock? Neither sm8150 nor sm8250 used the
> qref clock. Or is that an omission on our side?

Hi Dmitry,

For SC7280 we need all the three clocks for this target. Same is being 
used in downstream code as well. Hence I would need to update the 
binding as well.

Thanks,
Nitin


> > > > +    .vreg_list              = qmp_phy_vreg_l,
> > > > +    .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
> > > > +    .regs                   = ufsphy_v4_regs_layout,
> > > > +};
> > > > +
> > > >    static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
> > > >        .lanes            = 2,
> > > >
> > > > @@ -1648,6 +1787,9 @@ static const struct of_device_id
> > > > qmp_ufs_of_match_table[] = {
> > > >        }, {
> > > >            .compatible = "qcom,sa8775p-qmp-ufs-phy",
> > > >            .data = &sa8775p_ufsphy_cfg,
> > > > +    }, {
> > > > +        .compatible = "qcom,sc7280-qmp-ufs-phy",
> > > > +        .data = &sc7280_ufsphy_cfg,
> > > >        }, {
> > > >            .compatible = "qcom,sc8180x-qmp-ufs-phy",
> > > >            .data = &sm8150_ufsphy_cfg,
> > > > --
> > > > 2.17.1
> > Thanks,
> > Nitin