[PATCH v7 08/12] phy: qcom-qmp-pcie: Add support for SM8550 g3x2 and g4x2 PCIEs

[Abel Vesa]

On 23-02-03 10:33:14, Johan Hovold wrote:
> On Fri, Feb 03, 2023 at 10:18:03AM +0200, Abel Vesa wrote:
> > Add the SM8550 both g4 and g3 configurations. In addition, there is a
> > new "lane shared" table that needs to be configured for g4, along with
> > the No-CSR list of resets.
> 
> Could you add a comment about the new nocsr reset and how it is used
> here?
>  
> > Co-developed-by: Neil Armstrong <neil.armstrong at linaro.org>
> > Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
> > Signed-off-by: Abel Vesa <abel.vesa at linaro.org>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> > 
> > This patchset relies on the following patchset:
> > https://lore.kernel.org/all/20230117224148.1914627-1-abel.vesa@linaro.org/
> > 
> > The v6 of this patch is:
> > https://lore.kernel.org/all/20230202123902.3831491-9-abel.vesa@linaro.org/
> > 
> > Changes since v6:
> >  * none
> > 
> > Changes since v5:
> >  * renmaed the no-CSR reset to "phy_nocsr" as discussed off-list with
> >    Bjorn and Johan
> > 
> > Changes since v4:
> >  * dropped _serdes infix from ln_shrd table name and from every ln_shrd
> >    variable name
> >  * added hyphen between "no CSR" in both places
> >  * dropped has_ln_shrd_serdes_tbl
> >  * reordered qmp_pcie_offsets_v6_20 by struct members
> >  * added rollback for no-CSR reset in qmp_pcie_init fail path
> >  * moved ln_shrd offset calculation after port_b
> > 
> > Changes since v3:
> >  * added Dmitry's R-b tag
> > 
> > Changes since v2:
> >  * none
> > 
> > Changes since v1:
> >  * split all the offsets into separate patches, like Vinod suggested
> > 
> > 
> >  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 367 ++++++++++++++++++++++-
> >  1 file changed, 365 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > index 907f3f236f05..ff6c0b526fde 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > @@ -1506,6 +1506,234 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_ep_pcs_misc_tbl[] =
> >  	QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5, 0x08),
> >  };
> 
[...]
> 
> >  
> > @@ -2214,6 +2469,68 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
> >  	.phy_status		= PHYSTATUS_4_20,
> >  };
> >  
> > +static const struct qmp_phy_cfg sm8550_qmp_gen3x2_pciephy_cfg = {
> > +	.lanes = 2,
> > +
> > +	.offsets		= &qmp_pcie_offsets_v5,
> 
> Did you really intend to use the v5 offsets here? It seems you use v6.20
> defines in the tables below. This may work but it looks a little strange
> and does not match how we name and use these resources for the other
> SoCs (e.g. reusing structures and defines from older IP revisions is
> fine, but not necessarily the other way round).

So here is what is happening here. The actual IP block version is 6 for
the g3x2. The offsets of the tables are the same as on v5, but the
actual offsets of some of the registers within those tables are
entirely different. Now, if you compare the PCS PCIe offsets (v5 vs v6)
you'll notice that all v6 registers currently added are the same as v5
(both names and values). With that in mind, we still need to keep the v6
offsets for the case when a new register, that might not be in v5, might
be added later on. As for the table offsets, since they look the same we
should probably not add a dedicated v6 one.
> 
> I assume this means that the gen3 PHY is really is really v5 and using
> a subset of the v6.20 defines happens to works as they are in fact
> identical with respect to that subset?
> 
> As you have dedicated gen3x2 tables, perhaps those should use the v5
> defines?
> 
> And at least add a comment about this in the commit message.
> 
> > +
> > +	.tbls = {
> > +		.serdes		= sm8550_qmp_gen3x2_pcie_serdes_tbl,
> > +		.serdes_num	= ARRAY_SIZE(sm8550_qmp_gen3x2_pcie_serdes_tbl),
> > +		.tx		= sm8550_qmp_gen3x2_pcie_tx_tbl,
> > +		.tx_num		= ARRAY_SIZE(sm8550_qmp_gen3x2_pcie_tx_tbl),
> > +		.rx		= sm8550_qmp_gen3x2_pcie_rx_tbl,
> > +		.rx_num		= ARRAY_SIZE(sm8550_qmp_gen3x2_pcie_rx_tbl),
> > +		.pcs		= sm8550_qmp_gen3x2_pcie_pcs_tbl,
> > +		.pcs_num	= ARRAY_SIZE(sm8550_qmp_gen3x2_pcie_pcs_tbl),
> > +		.pcs_misc	= sm8550_qmp_gen3x2_pcie_pcs_misc_tbl,
> > +		.pcs_misc_num	= ARRAY_SIZE(sm8550_qmp_gen3x2_pcie_pcs_misc_tbl),
> > +	},
> > +	.clk_list		= sc8280xp_pciephy_clk_l,
> > +	.num_clks		= ARRAY_SIZE(sc8280xp_pciephy_clk_l),
> > +	.reset_list		= sdm845_pciephy_reset_l,
> > +	.num_resets		= ARRAY_SIZE(sdm845_pciephy_reset_l),
> > +	.vreg_list		= qmp_phy_vreg_l,
> > +	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
> > +	.regs			= pciephy_v5_regs_layout,
> > +
> > +	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
> > +	.phy_status		= PHYSTATUS,
> > +};
> > +

[...]