| Message ID | 20221111085643.9478-17-johan+linaro@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phy: qcom-qmp-combo: preparatory cleanups (set 2/3) | expand |
On 11/11/2022 11:56, Johan Hovold wrote: > In preparation for merging the USB and DP configurations, add dedicated > pointers for the DP serdes and tx tables to the configurations. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 45 ++++++++++++----------- > 1 file changed, 24 insertions(+), 21 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 499368e19e00..b27d1821116c 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -816,6 +816,11 @@ struct qmp_phy_cfg { > const struct qmp_phy_init_tbl *pcs_usb_tbl; > int pcs_usb_tbl_num; > > + const struct qmp_phy_init_tbl *dp_serdes_tbl; > + int dp_serdes_tbl_num; > + const struct qmp_phy_init_tbl *dp_tx_tbl; > + int dp_tx_tbl_num; > + I'd prefer to have DP-specific instance of struct qmp_phy_cfg_tbls here instead of having separate dp-specific fields. WDYT?
On Sat, Nov 12, 2022 at 10:39:12AM +0300, Dmitry Baryshkov wrote: > On 11/11/2022 11:56, Johan Hovold wrote: > > In preparation for merging the USB and DP configurations, add dedicated > > pointers for the DP serdes and tx tables to the configurations. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 45 ++++++++++++----------- > > 1 file changed, 24 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > index 499368e19e00..b27d1821116c 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > @@ -816,6 +816,11 @@ struct qmp_phy_cfg { > > const struct qmp_phy_init_tbl *pcs_usb_tbl; > > int pcs_usb_tbl_num; > > > > + const struct qmp_phy_init_tbl *dp_serdes_tbl; > > + int dp_serdes_tbl_num; > > + const struct qmp_phy_init_tbl *dp_tx_tbl; > > + int dp_tx_tbl_num; > > + > > I'd prefer to have DP-specific instance of struct qmp_phy_cfg_tbls here > instead of having separate dp-specific fields. WDYT? No, I don't see any good reason for doing so currently. You may be able to share a few pointers between some of the SoCs but only until it turns out they need to override certain sequences anyway (e.g. sc8280xp which mixes v4 and v5 tables currently). You'd also need dedicated aggregate table structs for USB and DP and it seems all of this would just make things more opaque for little gain. Johan
On 14/11/2022 11:38, Johan Hovold wrote: > On Sat, Nov 12, 2022 at 10:39:12AM +0300, Dmitry Baryshkov wrote: >> On 11/11/2022 11:56, Johan Hovold wrote: >>> In preparation for merging the USB and DP configurations, add dedicated >>> pointers for the DP serdes and tx tables to the configurations. >>> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 45 ++++++++++++----------- >>> 1 file changed, 24 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> index 499368e19e00..b27d1821116c 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> @@ -816,6 +816,11 @@ struct qmp_phy_cfg { >>> const struct qmp_phy_init_tbl *pcs_usb_tbl; >>> int pcs_usb_tbl_num; >>> >>> + const struct qmp_phy_init_tbl *dp_serdes_tbl; >>> + int dp_serdes_tbl_num; >>> + const struct qmp_phy_init_tbl *dp_tx_tbl; >>> + int dp_tx_tbl_num; >>> + >> >> I'd prefer to have DP-specific instance of struct qmp_phy_cfg_tbls here >> instead of having separate dp-specific fields. WDYT? > > No, I don't see any good reason for doing so currently. > > You may be able to share a few pointers between some of the SoCs but > only until it turns out they need to override certain sequences anyway > (e.g. sc8280xp which mixes v4 and v5 tables currently). > > You'd also need dedicated aggregate table structs for USB and DP and it > seems all of this would just make things more opaque for little gain. I was thinking about moving the qmp.*init_registers into the common library. However let's probably finish the cleanup before working on code consolidation.
On 14/11/2022 11:38, Johan Hovold wrote: > On Sat, Nov 12, 2022 at 10:39:12AM +0300, Dmitry Baryshkov wrote: >> On 11/11/2022 11:56, Johan Hovold wrote: >>> In preparation for merging the USB and DP configurations, add dedicated >>> pointers for the DP serdes and tx tables to the configurations. >>> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 45 ++++++++++++----------- >>> 1 file changed, 24 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> index 499368e19e00..b27d1821116c 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> @@ -816,6 +816,11 @@ struct qmp_phy_cfg { >>> const struct qmp_phy_init_tbl *pcs_usb_tbl; >>> int pcs_usb_tbl_num; >>> >>> + const struct qmp_phy_init_tbl *dp_serdes_tbl; >>> + int dp_serdes_tbl_num; >>> + const struct qmp_phy_init_tbl *dp_tx_tbl; >>> + int dp_tx_tbl_num; >>> + >> >> I'd prefer to have DP-specific instance of struct qmp_phy_cfg_tbls here >> instead of having separate dp-specific fields. WDYT? > > No, I don't see any good reason for doing so currently. > > You may be able to share a few pointers between some of the SoCs but > only until it turns out they need to override certain sequences anyway > (e.g. sc8280xp which mixes v4 and v5 tables currently). > > You'd also need dedicated aggregate table structs for USB and DP and it > seems all of this would just make things more opaque for little gain. Forgot: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index 499368e19e00..b27d1821116c 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -816,6 +816,11 @@ struct qmp_phy_cfg { const struct qmp_phy_init_tbl *pcs_usb_tbl; int pcs_usb_tbl_num; + const struct qmp_phy_init_tbl *dp_serdes_tbl; + int dp_serdes_tbl_num; + const struct qmp_phy_init_tbl *dp_tx_tbl; + int dp_tx_tbl_num; + /* Init sequence for DP PHY block link rates */ const struct qmp_phy_init_tbl *serdes_tbl_rbr; int serdes_tbl_rbr_num; @@ -1030,10 +1035,10 @@ static const struct qmp_phy_cfg sc7180_dpphy_cfg = { .type = PHY_TYPE_DP, .lanes = 2, - .serdes_tbl = qmp_v3_dp_serdes_tbl, - .serdes_tbl_num = ARRAY_SIZE(qmp_v3_dp_serdes_tbl), - .tx_tbl = qmp_v3_dp_tx_tbl, - .tx_tbl_num = ARRAY_SIZE(qmp_v3_dp_tx_tbl), + .dp_serdes_tbl = qmp_v3_dp_serdes_tbl, + .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v3_dp_serdes_tbl), + .dp_tx_tbl = qmp_v3_dp_tx_tbl, + .dp_tx_tbl_num = ARRAY_SIZE(qmp_v3_dp_tx_tbl), .serdes_tbl_rbr = qmp_v3_dp_serdes_tbl_rbr, .serdes_tbl_rbr_num = ARRAY_SIZE(qmp_v3_dp_serdes_tbl_rbr), @@ -1147,10 +1152,10 @@ static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { .type = PHY_TYPE_DP, .lanes = 2, - .serdes_tbl = qmp_v4_dp_serdes_tbl, - .serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), - .tx_tbl = qmp_v4_dp_tx_tbl, - .tx_tbl_num = ARRAY_SIZE(qmp_v4_dp_tx_tbl), + .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, + .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), + .dp_tx_tbl = qmp_v4_dp_tx_tbl, + .dp_tx_tbl_num = ARRAY_SIZE(qmp_v4_dp_tx_tbl), .serdes_tbl_rbr = qmp_v4_dp_serdes_tbl_rbr, .serdes_tbl_rbr_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl_rbr), @@ -1203,10 +1208,10 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = { .type = PHY_TYPE_DP, .lanes = 2, - .serdes_tbl = qmp_v5_dp_serdes_tbl, - .serdes_tbl_num = ARRAY_SIZE(qmp_v5_dp_serdes_tbl), - .tx_tbl = qmp_v5_5nm_dp_tx_tbl, - .tx_tbl_num = ARRAY_SIZE(qmp_v5_5nm_dp_tx_tbl), + .dp_serdes_tbl = qmp_v5_dp_serdes_tbl, + .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v5_dp_serdes_tbl), + .dp_tx_tbl = qmp_v5_5nm_dp_tx_tbl, + .dp_tx_tbl_num = ARRAY_SIZE(qmp_v5_5nm_dp_tx_tbl), .serdes_tbl_rbr = qmp_v4_dp_serdes_tbl_rbr, .serdes_tbl_rbr_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl_rbr), @@ -1263,10 +1268,10 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = { .type = PHY_TYPE_DP, .lanes = 2, - .serdes_tbl = qmp_v4_dp_serdes_tbl, - .serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), - .tx_tbl = qmp_v4_dp_tx_tbl, - .tx_tbl_num = ARRAY_SIZE(qmp_v4_dp_tx_tbl), + .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, + .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), + .dp_tx_tbl = qmp_v4_dp_tx_tbl, + .dp_tx_tbl_num = ARRAY_SIZE(qmp_v4_dp_tx_tbl), .serdes_tbl_rbr = qmp_v4_dp_serdes_tbl_rbr, .serdes_tbl_rbr_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl_rbr), @@ -1324,10 +1329,8 @@ static int qmp_combo_dp_serdes_init(struct qmp_phy *qphy) const struct qmp_phy_cfg *cfg = qphy->cfg; void __iomem *serdes = qphy->dp_serdes; const struct phy_configure_opts_dp *dp_opts = &qphy->dp_opts; - const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl; - int serdes_tbl_num = cfg->serdes_tbl_num; - qmp_combo_configure(serdes, serdes_tbl, serdes_tbl_num); + qmp_combo_configure(serdes, cfg->dp_serdes_tbl, cfg->dp_serdes_tbl_num); switch (dp_opts->link_rate) { case 1620: @@ -2000,10 +2003,10 @@ static int qmp_combo_dp_power_on(struct phy *phy) qmp_combo_dp_serdes_init(qphy); - qmp_combo_configure_lane(tx, cfg->tx_tbl, cfg->tx_tbl_num, 1); + qmp_combo_configure_lane(tx, cfg->dp_tx_tbl, cfg->dp_tx_tbl_num, 1); if (cfg->lanes >= 2) - qmp_combo_configure_lane(qphy->dp_tx2, cfg->tx_tbl, cfg->tx_tbl_num, 2); + qmp_combo_configure_lane(qphy->dp_tx2, cfg->dp_tx_tbl, cfg->dp_tx_tbl_num, 2); /* Configure special DP tx tunings */ cfg->configure_dp_tx(qphy);
In preparation for merging the USB and DP configurations, add dedicated pointers for the DP serdes and tx tables to the configurations. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 45 ++++++++++++----------- 1 file changed, 24 insertions(+), 21 deletions(-)