On Mon, Feb 06, 2023 at 04:05:02PM +0200, Abel Vesa wrote: > 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@xxxxxxxxxx> > > > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > > > Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > > --- > > > > > > This patchset relies on the following patchset: > > > https://lore.kernel.org/all/20230117224148.1914627-1-abel.vesa@xxxxxxxxxx/ > > > > > > The v6 of this patch is: > > > https://lore.kernel.org/all/20230202123902.3831491-9-abel.vesa@xxxxxxxxxx/ > > > > > > 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. Ok, makes sense. Johan
