| Message ID | 20220924160302.285875-2-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: Support using the same PHY for both RC and EP | expand |
On Sat, Sep 24, 2022 at 07:02:57PM +0300, Dmitry Baryshkov wrote: > SM8250 configuration tables are split into two parts: the common one and > the PHY-specific tables. Make this split more formal. Rather than having > a blind renamed copy of all QMP table fields, add separate struct > qmp_phy_cfg_tables and add two instances of this structure to the struct > qmp_phy_cfg. Later on this will be used to support different PHY modes > (RC vs EP). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- > 1 file changed, 77 insertions(+), 52 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index 7aff3f9940a5..30806816c8b0 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { > QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), > }; > > -/* struct qmp_phy_cfg - per-PHY initialization config */ > -struct qmp_phy_cfg { > - int lanes; > - > - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ > +struct qmp_phy_cfg_tables { > const struct qmp_phy_init_tbl *serdes_tbl; > int serdes_tbl_num; > - const struct qmp_phy_init_tbl *serdes_tbl_sec; > - int serdes_tbl_num_sec; > const struct qmp_phy_init_tbl *tx_tbl; > int tx_tbl_num; > - const struct qmp_phy_init_tbl *tx_tbl_sec; > - int tx_tbl_num_sec; > const struct qmp_phy_init_tbl *rx_tbl; > int rx_tbl_num; > - const struct qmp_phy_init_tbl *rx_tbl_sec; > - int rx_tbl_num_sec; > const struct qmp_phy_init_tbl *pcs_tbl; > int pcs_tbl_num; > - const struct qmp_phy_init_tbl *pcs_tbl_sec; > - int pcs_tbl_num_sec; > const struct qmp_phy_init_tbl *pcs_misc_tbl; > int pcs_misc_tbl_num; > - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; > - int pcs_misc_tbl_num_sec; > +}; > + > +/* struct qmp_phy_cfg - per-PHY initialization config */ > +struct qmp_phy_cfg { > + int lanes; > + > + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ > + struct qmp_phy_cfg_tables common; > + /* > + * Additional init sequence for PHY blocks, providing additional > + * register programming. Unless required it can be left omitted. > + */ > + struct qmp_phy_cfg_tables *extra; > > /* clock ids to be requested */ > const char * const *clk_list; > @@ -1949,31 +1974,31 @@ static int qmp_pcie_power_on(struct phy *phy) > } > > /* Tx, Rx, and PCS configurations */ > - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num, 1); > - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl_sec, cfg->tx_tbl_num_sec, 1); > + qmp_pcie_configure_lane(tx, cfg->regs, cfg->common.tx_tbl, cfg->common.tx_tbl_num, 1); > + qmp_pcie_configure_lane(tx, cfg->regs, cfg->extra->tx_tbl, cfg->extra->tx_tbl_num, 1); Hmm. How did you test this? With your later versions of this series, cfg->extra is generally NULL so this would dereference a NULL pointer. Johan
On Sat, Sep 24, 2022 at 07:02:57PM +0300, Dmitry Baryshkov wrote: > SM8250 configuration tables are split into two parts: the common one and > the PHY-specific tables. Make this split more formal. Rather than having > a blind renamed copy of all QMP table fields, add separate struct > qmp_phy_cfg_tables and add two instances of this structure to the struct > qmp_phy_cfg. Later on this will be used to support different PHY modes > (RC vs EP). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- > 1 file changed, 77 insertions(+), 52 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index 7aff3f9940a5..30806816c8b0 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { > QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), > }; > > -/* struct qmp_phy_cfg - per-PHY initialization config */ > -struct qmp_phy_cfg { > - int lanes; > - > - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ > +struct qmp_phy_cfg_tables { > const struct qmp_phy_init_tbl *serdes_tbl; > int serdes_tbl_num; So I still think you should drop the now redundant "tbl" suffix and infix. > - const struct qmp_phy_init_tbl *serdes_tbl_sec; > - int serdes_tbl_num_sec; > const struct qmp_phy_init_tbl *tx_tbl; > int tx_tbl_num; > - const struct qmp_phy_init_tbl *tx_tbl_sec; > - int tx_tbl_num_sec; > const struct qmp_phy_init_tbl *rx_tbl; > int rx_tbl_num; > - const struct qmp_phy_init_tbl *rx_tbl_sec; > - int rx_tbl_num_sec; > const struct qmp_phy_init_tbl *pcs_tbl; > int pcs_tbl_num; > - const struct qmp_phy_init_tbl *pcs_tbl_sec; > - int pcs_tbl_num_sec; > const struct qmp_phy_init_tbl *pcs_misc_tbl; > int pcs_misc_tbl_num; > - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; > - int pcs_misc_tbl_num_sec; > +}; > + > +/* struct qmp_phy_cfg - per-PHY initialization config */ > +struct qmp_phy_cfg { > + int lanes; > + > + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ > + struct qmp_phy_cfg_tables common; And this could be "tbls_common". > + /* > + * Additional init sequence for PHY blocks, providing additional > + * register programming. Unless required it can be left omitted. > + */ > + struct qmp_phy_cfg_tables *extra; And "tbls_extra". I guess this table should be const as well. > > /* clock ids to be requested */ > const char * const *clk_list; > @@ -1459,6 +1458,7 @@ static const char * const sdm845_pciephy_reset_l[] = { > static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { > .lanes = 1, > > + .common = { > .serdes_tbl = ipq8074_pcie_serdes_tbl, > .serdes_tbl_num = ARRAY_SIZE(ipq8074_pcie_serdes_tbl), > .tx_tbl = ipq8074_pcie_tx_tbl, > @@ -1467,6 +1467,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { > .rx_tbl_num = ARRAY_SIZE(ipq8074_pcie_rx_tbl), > .pcs_tbl = ipq8074_pcie_pcs_tbl, > .pcs_tbl_num = ARRAY_SIZE(ipq8074_pcie_pcs_tbl), > + }, Shouldn't you indent the members now? The above looks unnecessarily hard to read. > .clk_list = ipq8074_pciephy_clk_l, > .num_clks = ARRAY_SIZE(ipq8074_pciephy_clk_l), > .reset_list = ipq8074_pciephy_reset_l, @@ -1603,24 +1612,28 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { > static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { > .lanes = 1, > > + .common = { > .serdes_tbl = sm8250_qmp_pcie_serdes_tbl, > .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl), > - .serdes_tbl_sec = sm8250_qmp_gen3x1_pcie_serdes_tbl, > - .serdes_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), > .tx_tbl = sm8250_qmp_pcie_tx_tbl, > .tx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl), > .rx_tbl = sm8250_qmp_pcie_rx_tbl, > .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl), > - .rx_tbl_sec = sm8250_qmp_gen3x1_pcie_rx_tbl, > - .rx_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), > .pcs_tbl = sm8250_qmp_pcie_pcs_tbl, > .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl), > - .pcs_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_tbl, > - .pcs_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), > .pcs_misc_tbl = sm8250_qmp_pcie_pcs_misc_tbl, > .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl), > - .pcs_misc_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, > - .pcs_misc_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), > + }, > + .extra = &(struct qmp_phy_cfg_tables) { const structure? > + .serdes_tbl = sm8250_qmp_gen3x1_pcie_serdes_tbl, > + .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), > + .rx_tbl = sm8250_qmp_gen3x1_pcie_rx_tbl, > + .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), > + .pcs_tbl = sm8250_qmp_gen3x1_pcie_pcs_tbl, > + .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), > + .pcs_misc_tbl = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, > + .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), Indentation. > + }, > .clk_list = sdm845_pciephy_clk_l, > .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), > .reset_list = sdm845_pciephy_reset_l, > @@ -1854,11 +1881,9 @@ static int qmp_pcie_serdes_init(struct qmp_phy *qphy) > { > const struct qmp_phy_cfg *cfg = qphy->cfg; > void __iomem *serdes = qphy->serdes; > - const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl; > - int serdes_tbl_num = cfg->serdes_tbl_num; > > - qmp_pcie_configure(serdes, cfg->regs, serdes_tbl, serdes_tbl_num); > - qmp_pcie_configure(serdes, cfg->regs, cfg->serdes_tbl_sec, cfg->serdes_tbl_num_sec); > + qmp_pcie_configure(serdes, cfg->regs, cfg->common.serdes_tbl, cfg->common.serdes_tbl_num); > + qmp_pcie_configure(serdes, cfg->regs, cfg->extra->serdes_tbl, cfg->extra->serdes_tbl_num); I already mentioned the NULL-derefs as cfg->extra can be NULL. > > return 0; > } > if (IS_ERR(qphy->pcs_misc)) { > - if (cfg->pcs_misc_tbl || cfg->pcs_misc_tbl_sec) > + if (cfg->common.pcs_misc_tbl || cfg->extra->pcs_misc_tbl) Here too. > return PTR_ERR(qphy->pcs_misc); > } Johan
On 26/09/2022 09:32, Johan Hovold wrote: > On Sat, Sep 24, 2022 at 07:02:57PM +0300, Dmitry Baryshkov wrote: >> SM8250 configuration tables are split into two parts: the common one and >> the PHY-specific tables. Make this split more formal. Rather than having >> a blind renamed copy of all QMP table fields, add separate struct >> qmp_phy_cfg_tables and add two instances of this structure to the struct >> qmp_phy_cfg. Later on this will be used to support different PHY modes >> (RC vs EP). >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- >> 1 file changed, 77 insertions(+), 52 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> index 7aff3f9940a5..30806816c8b0 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { >> QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), >> }; >> >> -/* struct qmp_phy_cfg - per-PHY initialization config */ >> -struct qmp_phy_cfg { >> - int lanes; >> - >> - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ >> +struct qmp_phy_cfg_tables { >> const struct qmp_phy_init_tbl *serdes_tbl; >> int serdes_tbl_num; >> - const struct qmp_phy_init_tbl *serdes_tbl_sec; >> - int serdes_tbl_num_sec; >> const struct qmp_phy_init_tbl *tx_tbl; >> int tx_tbl_num; >> - const struct qmp_phy_init_tbl *tx_tbl_sec; >> - int tx_tbl_num_sec; >> const struct qmp_phy_init_tbl *rx_tbl; >> int rx_tbl_num; >> - const struct qmp_phy_init_tbl *rx_tbl_sec; >> - int rx_tbl_num_sec; >> const struct qmp_phy_init_tbl *pcs_tbl; >> int pcs_tbl_num; >> - const struct qmp_phy_init_tbl *pcs_tbl_sec; >> - int pcs_tbl_num_sec; >> const struct qmp_phy_init_tbl *pcs_misc_tbl; >> int pcs_misc_tbl_num; >> - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; >> - int pcs_misc_tbl_num_sec; >> +}; >> + >> +/* struct qmp_phy_cfg - per-PHY initialization config */ >> +struct qmp_phy_cfg { >> + int lanes; >> + >> + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ >> + struct qmp_phy_cfg_tables common; >> + /* >> + * Additional init sequence for PHY blocks, providing additional >> + * register programming. Unless required it can be left omitted. >> + */ >> + struct qmp_phy_cfg_tables *extra; >> >> /* clock ids to be requested */ >> const char * const *clk_list; > >> @@ -1949,31 +1974,31 @@ static int qmp_pcie_power_on(struct phy *phy) >> } >> >> /* Tx, Rx, and PCS configurations */ >> - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num, 1); >> - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl_sec, cfg->tx_tbl_num_sec, 1); >> + qmp_pcie_configure_lane(tx, cfg->regs, cfg->common.tx_tbl, cfg->common.tx_tbl_num, 1); >> + qmp_pcie_configure_lane(tx, cfg->regs, cfg->extra->tx_tbl, cfg->extra->tx_tbl_num, 1); > > Hmm. How did you test this? > > With your later versions of this series, cfg->extra is generally NULL so > this would dereference a NULL pointer. I must admit, I tested this only on sm8450. Mea culpa. > > Johan
On 26/09/2022 10:27, Johan Hovold wrote: > On Sat, Sep 24, 2022 at 07:02:57PM +0300, Dmitry Baryshkov wrote: >> SM8250 configuration tables are split into two parts: the common one and >> the PHY-specific tables. Make this split more formal. Rather than having >> a blind renamed copy of all QMP table fields, add separate struct >> qmp_phy_cfg_tables and add two instances of this structure to the struct >> qmp_phy_cfg. Later on this will be used to support different PHY modes >> (RC vs EP). >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- >> 1 file changed, 77 insertions(+), 52 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> index 7aff3f9940a5..30806816c8b0 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { >> QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), >> }; >> >> -/* struct qmp_phy_cfg - per-PHY initialization config */ >> -struct qmp_phy_cfg { >> - int lanes; >> - >> - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ >> +struct qmp_phy_cfg_tables { >> const struct qmp_phy_init_tbl *serdes_tbl; >> int serdes_tbl_num; > > So I still think you should drop the now redundant "tbl" suffix and > infix. > >> - const struct qmp_phy_init_tbl *serdes_tbl_sec; >> - int serdes_tbl_num_sec; >> const struct qmp_phy_init_tbl *tx_tbl; >> int tx_tbl_num; >> - const struct qmp_phy_init_tbl *tx_tbl_sec; >> - int tx_tbl_num_sec; >> const struct qmp_phy_init_tbl *rx_tbl; >> int rx_tbl_num; >> - const struct qmp_phy_init_tbl *rx_tbl_sec; >> - int rx_tbl_num_sec; >> const struct qmp_phy_init_tbl *pcs_tbl; >> int pcs_tbl_num; >> - const struct qmp_phy_init_tbl *pcs_tbl_sec; >> - int pcs_tbl_num_sec; >> const struct qmp_phy_init_tbl *pcs_misc_tbl; >> int pcs_misc_tbl_num; >> - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; >> - int pcs_misc_tbl_num_sec; >> +}; >> + >> +/* struct qmp_phy_cfg - per-PHY initialization config */ >> +struct qmp_phy_cfg { >> + int lanes; >> + >> + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ >> + struct qmp_phy_cfg_tables common; > > And this could be "tbls_common". I'd go for common_tables, if you don't mind. > >> + /* >> + * Additional init sequence for PHY blocks, providing additional >> + * register programming. Unless required it can be left omitted. >> + */ >> + struct qmp_phy_cfg_tables *extra; > > And "tbls_extra". > > I guess this table should be const as well. Ack > >> >> /* clock ids to be requested */ >> const char * const *clk_list; >> @@ -1459,6 +1458,7 @@ static const char * const sdm845_pciephy_reset_l[] = { >> static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { >> .lanes = 1, >> >> + .common = { >> .serdes_tbl = ipq8074_pcie_serdes_tbl, >> .serdes_tbl_num = ARRAY_SIZE(ipq8074_pcie_serdes_tbl), >> .tx_tbl = ipq8074_pcie_tx_tbl, >> @@ -1467,6 +1467,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { >> .rx_tbl_num = ARRAY_SIZE(ipq8074_pcie_rx_tbl), >> .pcs_tbl = ipq8074_pcie_pcs_tbl, >> .pcs_tbl_num = ARRAY_SIZE(ipq8074_pcie_pcs_tbl), >> + }, > > Shouldn't you indent the members now? The above looks unnecessarily hard > to read. I wanted to keep the indentation to make the patch small enough, but let's indent these lines (while dropping the _tbl from names as you insisted). > >> .clk_list = ipq8074_pciephy_clk_l, >> .num_clks = ARRAY_SIZE(ipq8074_pciephy_clk_l), >> .reset_list = ipq8074_pciephy_reset_l, > > @@ -1603,24 +1612,28 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { >> static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { >> .lanes = 1, >> >> + .common = { >> .serdes_tbl = sm8250_qmp_pcie_serdes_tbl, >> .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl), >> - .serdes_tbl_sec = sm8250_qmp_gen3x1_pcie_serdes_tbl, >> - .serdes_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), >> .tx_tbl = sm8250_qmp_pcie_tx_tbl, >> .tx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl), >> .rx_tbl = sm8250_qmp_pcie_rx_tbl, >> .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl), >> - .rx_tbl_sec = sm8250_qmp_gen3x1_pcie_rx_tbl, >> - .rx_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), >> .pcs_tbl = sm8250_qmp_pcie_pcs_tbl, >> .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl), >> - .pcs_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_tbl, >> - .pcs_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), >> .pcs_misc_tbl = sm8250_qmp_pcie_pcs_misc_tbl, >> .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl), >> - .pcs_misc_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, >> - .pcs_misc_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), >> + }, >> + .extra = &(struct qmp_phy_cfg_tables) { > > const structure? Ack > >> + .serdes_tbl = sm8250_qmp_gen3x1_pcie_serdes_tbl, >> + .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), >> + .rx_tbl = sm8250_qmp_gen3x1_pcie_rx_tbl, >> + .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), >> + .pcs_tbl = sm8250_qmp_gen3x1_pcie_pcs_tbl, >> + .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), >> + .pcs_misc_tbl = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, >> + .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), > > Indentation. > >> + }, >> .clk_list = sdm845_pciephy_clk_l, >> .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), >> .reset_list = sdm845_pciephy_reset_l, > >> @@ -1854,11 +1881,9 @@ static int qmp_pcie_serdes_init(struct qmp_phy *qphy) >> { >> const struct qmp_phy_cfg *cfg = qphy->cfg; >> void __iomem *serdes = qphy->serdes; >> - const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl; >> - int serdes_tbl_num = cfg->serdes_tbl_num; >> >> - qmp_pcie_configure(serdes, cfg->regs, serdes_tbl, serdes_tbl_num); >> - qmp_pcie_configure(serdes, cfg->regs, cfg->serdes_tbl_sec, cfg->serdes_tbl_num_sec); >> + qmp_pcie_configure(serdes, cfg->regs, cfg->common.serdes_tbl, cfg->common.serdes_tbl_num); >> + qmp_pcie_configure(serdes, cfg->regs, cfg->extra->serdes_tbl, cfg->extra->serdes_tbl_num); > > I already mentioned the NULL-derefs as cfg->extra can be NULL. > >> >> return 0; >> } > >> if (IS_ERR(qphy->pcs_misc)) { >> - if (cfg->pcs_misc_tbl || cfg->pcs_misc_tbl_sec) >> + if (cfg->common.pcs_misc_tbl || cfg->extra->pcs_misc_tbl) > > Here too. > >> return PTR_ERR(qphy->pcs_misc); >> } > > Johan
On Mon, Sep 26, 2022 at 02:35:07PM +0300, Dmitry Baryshkov wrote: > On 26/09/2022 10:27, Johan Hovold wrote: > > On Sat, Sep 24, 2022 at 07:02:57PM +0300, Dmitry Baryshkov wrote: > >> SM8250 configuration tables are split into two parts: the common one and > >> the PHY-specific tables. Make this split more formal. Rather than having > >> a blind renamed copy of all QMP table fields, add separate struct > >> qmp_phy_cfg_tables and add two instances of this structure to the struct > >> qmp_phy_cfg. Later on this will be used to support different PHY modes > >> (RC vs EP). > >> > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> --- > >> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- > >> 1 file changed, 77 insertions(+), 52 deletions(-) > >> > >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> index 7aff3f9940a5..30806816c8b0 100644 > >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > >> @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { > >> QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), > >> }; > >> > >> -/* struct qmp_phy_cfg - per-PHY initialization config */ > >> -struct qmp_phy_cfg { > >> - int lanes; > >> - > >> - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ > >> +struct qmp_phy_cfg_tables { > >> const struct qmp_phy_init_tbl *serdes_tbl; > >> int serdes_tbl_num; > > > > So I still think you should drop the now redundant "tbl" suffix and > > infix. > > > >> - const struct qmp_phy_init_tbl *serdes_tbl_sec; > >> - int serdes_tbl_num_sec; > >> const struct qmp_phy_init_tbl *tx_tbl; > >> int tx_tbl_num; > >> - const struct qmp_phy_init_tbl *tx_tbl_sec; > >> - int tx_tbl_num_sec; > >> const struct qmp_phy_init_tbl *rx_tbl; > >> int rx_tbl_num; > >> - const struct qmp_phy_init_tbl *rx_tbl_sec; > >> - int rx_tbl_num_sec; > >> const struct qmp_phy_init_tbl *pcs_tbl; > >> int pcs_tbl_num; > >> - const struct qmp_phy_init_tbl *pcs_tbl_sec; > >> - int pcs_tbl_num_sec; > >> const struct qmp_phy_init_tbl *pcs_misc_tbl; > >> int pcs_misc_tbl_num; > >> - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; > >> - int pcs_misc_tbl_num_sec; > >> +}; > >> + > >> +/* struct qmp_phy_cfg - per-PHY initialization config */ > >> +struct qmp_phy_cfg { > >> + int lanes; > >> + > >> + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ > >> + struct qmp_phy_cfg_tables common; > > > > And this could be "tbls_common". > > I'd go for common_tables, if you don't mind. Sure, if you prefer. struct qmp_phy_cfg_tables tables; struct qmp_phy_cfg_tables tables_ep; struct qmp_phy_cfg_tables tables_rc; Could also be an alternative. Not sure the "common" prefix/suffix really adds anything. Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c index 7aff3f9940a5..30806816c8b0 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c @@ -1300,31 +1300,30 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = { QMP_PHY_INIT_CFG(QPHY_V5_20_PCS_PCIE_G4_PRE_GAIN, 0x2e), }; -/* struct qmp_phy_cfg - per-PHY initialization config */ -struct qmp_phy_cfg { - int lanes; - - /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ +struct qmp_phy_cfg_tables { const struct qmp_phy_init_tbl *serdes_tbl; int serdes_tbl_num; - const struct qmp_phy_init_tbl *serdes_tbl_sec; - int serdes_tbl_num_sec; const struct qmp_phy_init_tbl *tx_tbl; int tx_tbl_num; - const struct qmp_phy_init_tbl *tx_tbl_sec; - int tx_tbl_num_sec; const struct qmp_phy_init_tbl *rx_tbl; int rx_tbl_num; - const struct qmp_phy_init_tbl *rx_tbl_sec; - int rx_tbl_num_sec; const struct qmp_phy_init_tbl *pcs_tbl; int pcs_tbl_num; - const struct qmp_phy_init_tbl *pcs_tbl_sec; - int pcs_tbl_num_sec; const struct qmp_phy_init_tbl *pcs_misc_tbl; int pcs_misc_tbl_num; - const struct qmp_phy_init_tbl *pcs_misc_tbl_sec; - int pcs_misc_tbl_num_sec; +}; + +/* struct qmp_phy_cfg - per-PHY initialization config */ +struct qmp_phy_cfg { + int lanes; + + /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */ + struct qmp_phy_cfg_tables common; + /* + * Additional init sequence for PHY blocks, providing additional + * register programming. Unless required it can be left omitted. + */ + struct qmp_phy_cfg_tables *extra; /* clock ids to be requested */ const char * const *clk_list; @@ -1459,6 +1458,7 @@ static const char * const sdm845_pciephy_reset_l[] = { static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = ipq8074_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(ipq8074_pcie_serdes_tbl), .tx_tbl = ipq8074_pcie_tx_tbl, @@ -1467,6 +1467,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { .rx_tbl_num = ARRAY_SIZE(ipq8074_pcie_rx_tbl), .pcs_tbl = ipq8074_pcie_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(ipq8074_pcie_pcs_tbl), + }, .clk_list = ipq8074_pciephy_clk_l, .num_clks = ARRAY_SIZE(ipq8074_pciephy_clk_l), .reset_list = ipq8074_pciephy_reset_l, @@ -1487,6 +1488,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = { .lanes = 1, + .common = { .serdes_tbl = ipq8074_pcie_gen3_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(ipq8074_pcie_gen3_serdes_tbl), .tx_tbl = ipq8074_pcie_gen3_tx_tbl, @@ -1495,6 +1497,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = { .rx_tbl_num = ARRAY_SIZE(ipq8074_pcie_gen3_rx_tbl), .pcs_tbl = ipq8074_pcie_gen3_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(ipq8074_pcie_gen3_pcs_tbl), + }, .clk_list = ipq8074_pciephy_clk_l, .num_clks = ARRAY_SIZE(ipq8074_pciephy_clk_l), .reset_list = ipq8074_pciephy_reset_l, @@ -1516,6 +1519,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = { static const struct qmp_phy_cfg ipq6018_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = ipq6018_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(ipq6018_pcie_serdes_tbl), .tx_tbl = ipq6018_pcie_tx_tbl, @@ -1526,6 +1530,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl), .pcs_misc_tbl = ipq6018_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(ipq6018_pcie_pcs_misc_tbl), + }, .clk_list = ipq8074_pciephy_clk_l, .num_clks = ARRAY_SIZE(ipq8074_pciephy_clk_l), .reset_list = ipq8074_pciephy_reset_l, @@ -1545,6 +1550,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = { static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = sdm845_qmp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sdm845_qmp_pcie_serdes_tbl), .tx_tbl = sdm845_qmp_pcie_tx_tbl, @@ -1555,6 +1561,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sdm845_qmp_pcie_pcs_tbl), .pcs_misc_tbl = sdm845_qmp_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sdm845_qmp_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1575,6 +1582,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = { static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = sdm845_qhp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sdm845_qhp_pcie_serdes_tbl), .tx_tbl = sdm845_qhp_pcie_tx_tbl, @@ -1583,6 +1591,7 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { .rx_tbl_num = ARRAY_SIZE(sdm845_qhp_pcie_rx_tbl), .pcs_tbl = sdm845_qhp_pcie_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(sdm845_qhp_pcie_pcs_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1603,24 +1612,28 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = sm8250_qmp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl), - .serdes_tbl_sec = sm8250_qmp_gen3x1_pcie_serdes_tbl, - .serdes_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), .tx_tbl = sm8250_qmp_pcie_tx_tbl, .tx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl), .rx_tbl = sm8250_qmp_pcie_rx_tbl, .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl), - .rx_tbl_sec = sm8250_qmp_gen3x1_pcie_rx_tbl, - .rx_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), .pcs_tbl = sm8250_qmp_pcie_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl), - .pcs_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_tbl, - .pcs_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), .pcs_misc_tbl = sm8250_qmp_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl), - .pcs_misc_tbl_sec = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, - .pcs_misc_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), + }, + .extra = &(struct qmp_phy_cfg_tables) { + .serdes_tbl = sm8250_qmp_gen3x1_pcie_serdes_tbl, + .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl), + .rx_tbl = sm8250_qmp_gen3x1_pcie_rx_tbl, + .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl), + .pcs_tbl = sm8250_qmp_gen3x1_pcie_pcs_tbl, + .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl), + .pcs_misc_tbl = sm8250_qmp_gen3x1_pcie_pcs_misc_tbl, + .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1641,24 +1654,28 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = { .lanes = 2, + .common = { .serdes_tbl = sm8250_qmp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl), .tx_tbl = sm8250_qmp_pcie_tx_tbl, .tx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl), - .tx_tbl_sec = sm8250_qmp_gen3x2_pcie_tx_tbl, - .tx_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl), .rx_tbl = sm8250_qmp_pcie_rx_tbl, .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl), - .rx_tbl_sec = sm8250_qmp_gen3x2_pcie_rx_tbl, - .rx_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl), .pcs_tbl = sm8250_qmp_pcie_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl), - .pcs_tbl_sec = sm8250_qmp_gen3x2_pcie_pcs_tbl, - .pcs_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl), .pcs_misc_tbl = sm8250_qmp_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl), - .pcs_misc_tbl_sec = sm8250_qmp_gen3x2_pcie_pcs_misc_tbl, - .pcs_misc_tbl_num_sec = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl), + }, + .extra = &(struct qmp_phy_cfg_tables) { + .tx_tbl = sm8250_qmp_gen3x2_pcie_tx_tbl, + .tx_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl), + .rx_tbl = sm8250_qmp_gen3x2_pcie_rx_tbl, + .rx_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl), + .pcs_tbl = sm8250_qmp_gen3x2_pcie_pcs_tbl, + .pcs_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl), + .pcs_misc_tbl = sm8250_qmp_gen3x2_pcie_pcs_misc_tbl, + .pcs_misc_tbl_num = ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1679,6 +1696,7 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = { static const struct qmp_phy_cfg msm8998_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = msm8998_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(msm8998_pcie_serdes_tbl), .tx_tbl = msm8998_pcie_tx_tbl, @@ -1687,6 +1705,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = { .rx_tbl_num = ARRAY_SIZE(msm8998_pcie_rx_tbl), .pcs_tbl = msm8998_pcie_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(msm8998_pcie_pcs_tbl), + }, .clk_list = msm8996_phy_clk_l, .num_clks = ARRAY_SIZE(msm8996_phy_clk_l), .reset_list = ipq8074_pciephy_reset_l, @@ -1703,6 +1722,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = { static const struct qmp_phy_cfg sc8180x_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = sc8180x_qmp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sc8180x_qmp_pcie_serdes_tbl), .tx_tbl = sc8180x_qmp_pcie_tx_tbl, @@ -1713,6 +1733,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sc8180x_qmp_pcie_pcs_tbl), .pcs_misc_tbl = sc8180x_qmp_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sc8180x_qmp_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1732,6 +1753,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = { static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = { .lanes = 2, + .common = { .serdes_tbl = sdx55_qmp_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sdx55_qmp_pcie_serdes_tbl), .tx_tbl = sdx55_qmp_pcie_tx_tbl, @@ -1742,6 +1764,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sdx55_qmp_pcie_pcs_tbl), .pcs_misc_tbl = sdx55_qmp_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sdx55_qmp_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1762,6 +1785,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = { static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = { .lanes = 1, + .common = { .serdes_tbl = sm8450_qmp_gen3x1_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_serdes_tbl), .tx_tbl = sm8450_qmp_gen3x1_pcie_tx_tbl, @@ -1772,6 +1796,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_tbl), .pcs_misc_tbl = sm8450_qmp_gen3x1_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1792,6 +1817,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = { static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = { .lanes = 2, + .common = { .serdes_tbl = sm8450_qmp_gen4x2_pcie_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_serdes_tbl), .tx_tbl = sm8450_qmp_gen4x2_pcie_tx_tbl, @@ -1802,6 +1828,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_tbl), .pcs_misc_tbl = sm8450_qmp_gen4x2_pcie_pcs_misc_tbl, .pcs_misc_tbl_num = ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_misc_tbl), + }, .clk_list = sdm845_pciephy_clk_l, .num_clks = ARRAY_SIZE(sdm845_pciephy_clk_l), .reset_list = sdm845_pciephy_reset_l, @@ -1854,11 +1881,9 @@ static int qmp_pcie_serdes_init(struct qmp_phy *qphy) { const struct qmp_phy_cfg *cfg = qphy->cfg; void __iomem *serdes = qphy->serdes; - const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl; - int serdes_tbl_num = cfg->serdes_tbl_num; - qmp_pcie_configure(serdes, cfg->regs, serdes_tbl, serdes_tbl_num); - qmp_pcie_configure(serdes, cfg->regs, cfg->serdes_tbl_sec, cfg->serdes_tbl_num_sec); + qmp_pcie_configure(serdes, cfg->regs, cfg->common.serdes_tbl, cfg->common.serdes_tbl_num); + qmp_pcie_configure(serdes, cfg->regs, cfg->extra->serdes_tbl, cfg->extra->serdes_tbl_num); return 0; } @@ -1949,31 +1974,31 @@ static int qmp_pcie_power_on(struct phy *phy) } /* Tx, Rx, and PCS configurations */ - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num, 1); - qmp_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl_sec, cfg->tx_tbl_num_sec, 1); + qmp_pcie_configure_lane(tx, cfg->regs, cfg->common.tx_tbl, cfg->common.tx_tbl_num, 1); + qmp_pcie_configure_lane(tx, cfg->regs, cfg->extra->tx_tbl, cfg->extra->tx_tbl_num, 1); if (cfg->lanes >= 2) { - qmp_pcie_configure_lane(qphy->tx2, cfg->regs, cfg->tx_tbl, - cfg->tx_tbl_num, 2); - qmp_pcie_configure_lane(qphy->tx2, cfg->regs, cfg->tx_tbl_sec, - cfg->tx_tbl_num_sec, 2); + qmp_pcie_configure_lane(qphy->tx2, cfg->regs, cfg->common.tx_tbl, + cfg->common.tx_tbl_num, 2); + qmp_pcie_configure_lane(qphy->tx2, cfg->regs, cfg->extra->tx_tbl, + cfg->extra->tx_tbl_num, 2); } - qmp_pcie_configure_lane(rx, cfg->regs, cfg->rx_tbl, cfg->rx_tbl_num, 1); - qmp_pcie_configure_lane(rx, cfg->regs, cfg->rx_tbl_sec, cfg->rx_tbl_num_sec, 1); + qmp_pcie_configure_lane(rx, cfg->regs, cfg->common.rx_tbl, cfg->common.rx_tbl_num, 1); + qmp_pcie_configure_lane(rx, cfg->regs, cfg->extra->rx_tbl, cfg->extra->rx_tbl_num, 1); if (cfg->lanes >= 2) { - qmp_pcie_configure_lane(qphy->rx2, cfg->regs, cfg->rx_tbl, - cfg->rx_tbl_num, 2); - qmp_pcie_configure_lane(qphy->rx2, cfg->regs, cfg->rx_tbl_sec, - cfg->rx_tbl_num_sec, 2); + qmp_pcie_configure_lane(qphy->rx2, cfg->regs, cfg->common.rx_tbl, + cfg->common.rx_tbl_num, 2); + qmp_pcie_configure_lane(qphy->rx2, cfg->regs, cfg->extra->rx_tbl, + cfg->extra->rx_tbl_num, 2); } - qmp_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num); - qmp_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl_sec, cfg->pcs_tbl_num_sec); + qmp_pcie_configure(pcs, cfg->regs, cfg->common.pcs_tbl, cfg->common.pcs_tbl_num); + qmp_pcie_configure(pcs, cfg->regs, cfg->extra->pcs_tbl, cfg->extra->pcs_tbl_num); - qmp_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl, cfg->pcs_misc_tbl_num); - qmp_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl_sec, cfg->pcs_misc_tbl_num_sec); + qmp_pcie_configure(pcs_misc, cfg->regs, cfg->common.pcs_misc_tbl, cfg->common.pcs_misc_tbl_num); + qmp_pcie_configure(pcs_misc, cfg->regs, cfg->extra->pcs_misc_tbl, cfg->extra->pcs_misc_tbl_num); /* * Pull out PHY from POWER DOWN state. @@ -2237,7 +2262,7 @@ static int qmp_pcie_create(struct device *dev, struct device_node *np, int id, qphy->pcs_misc = qphy->pcs + 0x400; if (IS_ERR(qphy->pcs_misc)) { - if (cfg->pcs_misc_tbl || cfg->pcs_misc_tbl_sec) + if (cfg->common.pcs_misc_tbl || cfg->extra->pcs_misc_tbl) return PTR_ERR(qphy->pcs_misc); }
SM8250 configuration tables are split into two parts: the common one and the PHY-specific tables. Make this split more formal. Rather than having a blind renamed copy of all QMP table fields, add separate struct qmp_phy_cfg_tables and add two instances of this structure to the struct qmp_phy_cfg. Later on this will be used to support different PHY modes (RC vs EP). Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 129 ++++++++++++++--------- 1 file changed, 77 insertions(+), 52 deletions(-)