| Message ID | 20221111085643.9478-18-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: > It does not really make any sense to keep separate configuration > structures for the USB and DP parts of the same PHY so merge them. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 182 +++++++--------------- > 1 file changed, 57 insertions(+), 125 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index b27d1821116c..249912b75964 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -798,10 +798,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr_rbr[4][4] = { > > struct qmp_phy; > > -/* struct qmp_phy_cfg - per-PHY initialization config */ > struct qmp_phy_cfg { > - /* phy-type - PCIE/UFS/USB */ > - unsigned int type; > int lanes; int lanes doesn't really make sense here in my opinion. It should be usb_lanes and dp_lanes. > > /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ > @@ -864,11 +861,6 @@ struct qmp_phy_cfg { > > }; > > -struct qmp_phy_combo_cfg { > - const struct qmp_phy_cfg *usb_cfg; > - const struct qmp_phy_cfg *dp_cfg; > -}; > - > /** > * struct qmp_phy - per-lane phy descriptor > * > @@ -1008,8 +1000,7 @@ static const char * const sc7180_usb3phy_reset_l[] = { > "phy", > }; > > -static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { > - .type = PHY_TYPE_USB3, > +static const struct qmp_phy_cfg sc7180_usb3dpphy_cfg = { > .lanes = 2, > > .serdes_tbl = qmp_v3_usb3_serdes_tbl, > @@ -1020,20 +1011,6 @@ static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { > .rx_tbl_num = ARRAY_SIZE(qmp_v3_usb3_rx_tbl), > .pcs_tbl = qmp_v3_usb3_pcs_tbl, > .pcs_tbl_num = ARRAY_SIZE(qmp_v3_usb3_pcs_tbl), > - .clk_list = qmp_v3_phy_clk_l, > - .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), > - .reset_list = sc7180_usb3phy_reset_l, > - .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), > - .vreg_list = qmp_phy_vreg_l, > - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > - .regs = qmp_v3_usb3phy_regs_layout, > - > - .has_pwrdn_delay = true, > -}; > - > -static const struct qmp_phy_cfg sc7180_dpphy_cfg = { > - .type = PHY_TYPE_DP, > - .lanes = 2, > > .dp_serdes_tbl = qmp_v3_dp_serdes_tbl, > .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v3_dp_serdes_tbl), > @@ -1058,15 +1035,19 @@ static const struct qmp_phy_cfg sc7180_dpphy_cfg = { > .configure_dp_tx = qcom_qmp_v3_phy_configure_dp_tx, > .configure_dp_phy = qcom_qmp_v3_phy_configure_dp_phy, > .calibrate_dp_phy = qcom_qmp_v3_dp_phy_calibrate, > -}; > > -static const struct qmp_phy_combo_cfg sc7180_usb3dpphy_cfg = { > - .usb_cfg = &sc7180_usb3phy_cfg, > - .dp_cfg = &sc7180_dpphy_cfg, > + .clk_list = qmp_v3_phy_clk_l, > + .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), > + .reset_list = sc7180_usb3phy_reset_l, > + .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), > + .vreg_list = qmp_phy_vreg_l, > + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > + .regs = qmp_v3_usb3phy_regs_layout, > + > + .has_pwrdn_delay = true, > }; > > -static const struct qmp_phy_cfg sdm845_usb3phy_cfg = { > - .type = PHY_TYPE_USB3, > +static const struct qmp_phy_cfg sdm845_usb3dpphy_cfg = { > .lanes = 2, > > .serdes_tbl = qmp_v3_usb3_serdes_tbl, > @@ -1077,25 +1058,11 @@ static const struct qmp_phy_cfg sdm845_usb3phy_cfg = { > .rx_tbl_num = ARRAY_SIZE(qmp_v3_usb3_rx_tbl), > .pcs_tbl = qmp_v3_usb3_pcs_tbl, > .pcs_tbl_num = ARRAY_SIZE(qmp_v3_usb3_pcs_tbl), > - .clk_list = qmp_v3_phy_clk_l, > - .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), > - .reset_list = msm8996_usb3phy_reset_l, > - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > - .vreg_list = qmp_phy_vreg_l, > - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > - .regs = qmp_v3_usb3phy_regs_layout, > - > - .has_pwrdn_delay = true, > -}; > > -static const struct qmp_phy_cfg sdm845_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), > @@ -1115,15 +1082,18 @@ static const struct qmp_phy_cfg sdm845_dpphy_cfg = { > .configure_dp_tx = qcom_qmp_v3_phy_configure_dp_tx, > .configure_dp_phy = qcom_qmp_v3_phy_configure_dp_phy, > .calibrate_dp_phy = qcom_qmp_v3_dp_phy_calibrate, > -}; > > -static const struct qmp_phy_combo_cfg sdm845_usb3dpphy_cfg = { > - .usb_cfg = &sdm845_usb3phy_cfg, > - .dp_cfg = &sdm845_dpphy_cfg, > -}; > + .clk_list = qmp_v3_phy_clk_l, > + .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), > + .reset_list = msm8996_usb3phy_reset_l, > + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > + .vreg_list = qmp_phy_vreg_l, > + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > + .regs = qmp_v3_usb3phy_regs_layout, > > -static const struct qmp_phy_cfg sm8150_usb3phy_cfg = { > - .type = PHY_TYPE_USB3, > + .has_pwrdn_delay = true, > +}; > +static const struct qmp_phy_cfg sc8180x_usb3dpphy_cfg = { > .lanes = 2, > > .serdes_tbl = sm8150_usb3_serdes_tbl, > @@ -1136,21 +1106,6 @@ static const struct qmp_phy_cfg sm8150_usb3phy_cfg = { > .pcs_tbl_num = ARRAY_SIZE(sm8150_usb3_pcs_tbl), > .pcs_usb_tbl = sm8150_usb3_pcs_usb_tbl, > .pcs_usb_tbl_num = ARRAY_SIZE(sm8150_usb3_pcs_usb_tbl), > - .clk_list = qmp_v4_phy_clk_l, > - .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), > - .reset_list = msm8996_usb3phy_reset_l, > - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > - .vreg_list = qmp_phy_vreg_l, > - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > - .regs = qmp_v4_usb3phy_regs_layout, > - .pcs_usb_offset = 0x300, > - > - .has_pwrdn_delay = true, > -}; > - > -static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { > - .type = PHY_TYPE_DP, > - .lanes = 2, > > .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, > .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), > @@ -1175,15 +1130,20 @@ static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { > .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, > .configure_dp_phy = qcom_qmp_v4_phy_configure_dp_phy, > .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, > -}; > > -static const struct qmp_phy_combo_cfg sc8180x_usb3dpphy_cfg = { > - .usb_cfg = &sm8150_usb3phy_cfg, > - .dp_cfg = &sc8180x_dpphy_cfg, > + .clk_list = qmp_v4_phy_clk_l, > + .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), > + .reset_list = msm8996_usb3phy_reset_l, > + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > + .vreg_list = qmp_phy_vreg_l, > + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > + .regs = qmp_v4_usb3phy_regs_layout, > + .pcs_usb_offset = 0x300, > + > + .has_pwrdn_delay = true, > }; > > -static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = { > - .type = PHY_TYPE_USB3, > +static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = { > .lanes = 2, > > .serdes_tbl = sc8280xp_usb43dp_serdes_tbl, > @@ -1194,19 +1154,6 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = { > .rx_tbl_num = ARRAY_SIZE(sc8280xp_usb43dp_rx_tbl), > .pcs_tbl = sc8280xp_usb43dp_pcs_tbl, > .pcs_tbl_num = ARRAY_SIZE(sc8280xp_usb43dp_pcs_tbl), > - .clk_list = qmp_v4_phy_clk_l, > - .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), > - .reset_list = msm8996_usb3phy_reset_l, > - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > - .vreg_list = qmp_phy_vreg_l, > - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > - .regs = qmp_v4_usb3phy_regs_layout, > - .pcs_usb_offset = 0x300, > -}; > - > -static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = { > - .type = PHY_TYPE_DP, > - .lanes = 2, > > .dp_serdes_tbl = qmp_v5_dp_serdes_tbl, > .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v5_dp_serdes_tbl), > @@ -1231,15 +1178,18 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = { > .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, > .configure_dp_phy = qcom_qmp_v5_phy_configure_dp_phy, > .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, > -}; > > -static const struct qmp_phy_combo_cfg sc8280xp_usb43dpphy_cfg = { > - .usb_cfg = &sc8280xp_usb43dp_usb_cfg, > - .dp_cfg = &sc8280xp_usb43dp_dp_cfg, > + .clk_list = qmp_v4_phy_clk_l, > + .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), > + .reset_list = msm8996_usb3phy_reset_l, > + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > + .vreg_list = qmp_phy_vreg_l, > + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > + .regs = qmp_v4_usb3phy_regs_layout, > + .pcs_usb_offset = 0x300, > }; > > -static const struct qmp_phy_cfg sm8250_usb3phy_cfg = { > - .type = PHY_TYPE_USB3, > +static const struct qmp_phy_cfg sm8250_usb3dpphy_cfg = { > .lanes = 2, > > .serdes_tbl = sm8150_usb3_serdes_tbl, > @@ -1252,21 +1202,6 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = { > .pcs_tbl_num = ARRAY_SIZE(sm8250_usb3_pcs_tbl), > .pcs_usb_tbl = sm8250_usb3_pcs_usb_tbl, > .pcs_usb_tbl_num = ARRAY_SIZE(sm8250_usb3_pcs_usb_tbl), > - .clk_list = qmp_v4_sm8250_usbphy_clk_l, > - .num_clks = ARRAY_SIZE(qmp_v4_sm8250_usbphy_clk_l), > - .reset_list = msm8996_usb3phy_reset_l, > - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > - .vreg_list = qmp_phy_vreg_l, > - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > - .regs = qmp_v4_usb3phy_regs_layout, > - .pcs_usb_offset = 0x300, > - > - .has_pwrdn_delay = true, > -}; > - > -static const struct qmp_phy_cfg sm8250_dpphy_cfg = { > - .type = PHY_TYPE_DP, > - .lanes = 2, > > .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, > .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), > @@ -1291,11 +1226,17 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = { > .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, > .configure_dp_phy = qcom_qmp_v4_phy_configure_dp_phy, > .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, > -}; > > -static const struct qmp_phy_combo_cfg sm8250_usb3dpphy_cfg = { > - .usb_cfg = &sm8250_usb3phy_cfg, > - .dp_cfg = &sm8250_dpphy_cfg, > + .clk_list = qmp_v4_sm8250_usbphy_clk_l, > + .num_clks = ARRAY_SIZE(qmp_v4_sm8250_usbphy_clk_l), > + .reset_list = msm8996_usb3phy_reset_l, > + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), > + .vreg_list = qmp_phy_vreg_l, > + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), > + .regs = qmp_v4_usb3phy_regs_layout, > + .pcs_usb_offset = 0x300, > + > + .has_pwrdn_delay = true, > }; > > static void qmp_combo_configure_lane(void __iomem *base, > @@ -2720,10 +2661,7 @@ static int qmp_combo_probe(struct platform_device *pdev) > void __iomem *serdes; > void __iomem *usb_serdes; > void __iomem *dp_serdes = NULL; > - const struct qmp_phy_combo_cfg *combo_cfg = NULL; > const struct qmp_phy_cfg *cfg = NULL; > - const struct qmp_phy_cfg *usb_cfg = NULL; > - const struct qmp_phy_cfg *dp_cfg = NULL; > int num, id, expected_phys; > int ret; > > @@ -2734,13 +2672,10 @@ static int qmp_combo_probe(struct platform_device *pdev) > qmp->dev = dev; > dev_set_drvdata(dev, qmp); > > - combo_cfg = of_device_get_match_data(dev); > - if (!combo_cfg) > + cfg = of_device_get_match_data(dev); > + if (!cfg) > return -EINVAL; > > - usb_cfg = combo_cfg->usb_cfg; > - cfg = usb_cfg; /* Setup clks and regulators */ > - > usb_serdes = serdes = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(serdes)) > return PTR_ERR(serdes); > @@ -2753,7 +2688,6 @@ static int qmp_combo_probe(struct platform_device *pdev) > if (IS_ERR(dp_serdes)) > return PTR_ERR(dp_serdes); > > - dp_cfg = combo_cfg->dp_cfg; > expected_phys = 2; > > mutex_init(&qmp->phy_mutex); > @@ -2792,7 +2726,6 @@ static int qmp_combo_probe(struct platform_device *pdev) > id = 0; > for_each_available_child_of_node(dev->of_node, child) { > if (of_node_name_eq(child, "dp-phy")) { > - cfg = dp_cfg; > serdes = dp_serdes; > > /* Create per-lane phy */ > @@ -2810,7 +2743,6 @@ static int qmp_combo_probe(struct platform_device *pdev) > goto err_node_put; > } > } else if (of_node_name_eq(child, "usb3-phy")) { > - cfg = usb_cfg; > serdes = usb_serdes; > > /* Create per-lane phy */
On Sat, Nov 12, 2022 at 10:43:14AM +0300, Dmitry Baryshkov wrote: > On 11/11/2022 11:56, Johan Hovold wrote: > > It does not really make any sense to keep separate configuration > > structures for the USB and DP parts of the same PHY so merge them. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 182 +++++++--------------- > > 1 file changed, 57 insertions(+), 125 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > index b27d1821116c..249912b75964 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > > @@ -798,10 +798,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr_rbr[4][4] = { > > > > struct qmp_phy; > > > > -/* struct qmp_phy_cfg - per-PHY initialization config */ > > struct qmp_phy_cfg { > > - /* phy-type - PCIE/UFS/USB */ > > - unsigned int type; > > int lanes; > > int lanes doesn't really make sense here in my opinion. It should be > usb_lanes and dp_lanes. It doesn't make much less sense than having it here currently do. All of these USB-C PHYs are dual lane for bi-directional SS USB and quad lane for uni-directional DP (even if only CC1 orientation and lanes 2 and 3 are currently supported). I should probably just drop the lanes parameter completely, either as a preparatory clean up or as follow-on one (e.g. also a bit depending on if there are other reasons for respinning a v2). Johan
On 14/11/2022 11:54, Johan Hovold wrote: > On Sat, Nov 12, 2022 at 10:43:14AM +0300, Dmitry Baryshkov wrote: >> On 11/11/2022 11:56, Johan Hovold wrote: >>> It does not really make any sense to keep separate configuration >>> structures for the USB and DP parts of the same PHY so merge them. >>> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 182 +++++++--------------- >>> 1 file changed, 57 insertions(+), 125 deletions(-) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> index b27d1821116c..249912b75964 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> @@ -798,10 +798,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr_rbr[4][4] = { >>> >>> struct qmp_phy; >>> >>> -/* struct qmp_phy_cfg - per-PHY initialization config */ >>> struct qmp_phy_cfg { >>> - /* phy-type - PCIE/UFS/USB */ >>> - unsigned int type; >>> int lanes; >> >> int lanes doesn't really make sense here in my opinion. It should be >> usb_lanes and dp_lanes. > > It doesn't make much less sense than having it here currently do. > > All of these USB-C PHYs are dual lane for bi-directional SS USB and > quad lane for uni-directional DP (even if only CC1 orientation and lanes > 2 and 3 are currently supported). I was under impression that sdm845 has just a single lane for each of USB and DP. After rechecking the phy/next, I see that it was my mistake (quite logical, SS is two lanes, so the compliant PHY must have two lanes too). I wander how/if 4-lane DP works. The only thing that we do is programming of the QSERDES_DP_PHY_PD_CTL register, however judging e.g. your 4-lane PCIe changes, one should probably also program the other two lanes. Maybe it is handled automatically inside the hardware. > I should probably just drop the lanes parameter completely, either as a > preparatory clean up or as follow-on one (e.g. also a bit depending on > if there are other reasons for respinning a v2). I think a follow up is enough, but let's get it. Having a single lanes=2 field looks... strange.
On 11/11/2022 11:56, Johan Hovold wrote: > It does not really make any sense to keep separate configuration > structures for the USB and DP parts of the same PHY so merge them. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 182 +++++++--------------- > 1 file changed, 57 insertions(+), 125 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Mon, Nov 14, 2022 at 01:10:43PM +0300, Dmitry Baryshkov wrote: > On 14/11/2022 11:54, Johan Hovold wrote: > > On Sat, Nov 12, 2022 at 10:43:14AM +0300, Dmitry Baryshkov wrote: > >> On 11/11/2022 11:56, Johan Hovold wrote: > >>> It does not really make any sense to keep separate configuration > >>> structures for the USB and DP parts of the same PHY so merge them. > >>> -/* struct qmp_phy_cfg - per-PHY initialization config */ > >>> struct qmp_phy_cfg { > >>> - /* phy-type - PCIE/UFS/USB */ > >>> - unsigned int type; > >>> int lanes; > >> > >> int lanes doesn't really make sense here in my opinion. It should be > >> usb_lanes and dp_lanes. > > > > It doesn't make much less sense than having it here currently do. > > > > All of these USB-C PHYs are dual lane for bi-directional SS USB and > > quad lane for uni-directional DP (even if only CC1 orientation and lanes > > 2 and 3 are currently supported). > > I was under impression that sdm845 has just a single lane for each of > USB and DP. After rechecking the phy/next, I see that it was my mistake > (quite logical, SS is two lanes, so the compliant PHY must have two > lanes too). > > I wander how/if 4-lane DP works. The only thing that we do is > programming of the QSERDES_DP_PHY_PD_CTL register, however judging e.g. > your 4-lane PCIe changes, one should probably also program the other two > lanes. Maybe it is handled automatically inside the hardware. 4-lane PCIe on SC8280XP is a different thing entirely (and remember that that's actually 8 uni-directional lanes). I'm sure there are further problems with the current DP alt mode implementation, but hopefully that can be resolved when adding support for orientation detection. I'm just fixing the obvious bugs and try to stay "bug compatible" otherwise. :) > > I should probably just drop the lanes parameter completely, either as a > > preparatory clean up or as follow-on one (e.g. also a bit depending on > > if there are other reasons for respinning a v2). > > I think a follow up is enough, but let's get it. Having a single lanes=2 > field looks... strange. I dropped the lane parameter as a preparatory patch to this one in v2 that I'll post in a bit. Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index b27d1821116c..249912b75964 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -798,10 +798,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr_rbr[4][4] = { struct qmp_phy; -/* struct qmp_phy_cfg - per-PHY initialization config */ struct qmp_phy_cfg { - /* phy-type - PCIE/UFS/USB */ - unsigned int type; int lanes; /* Init sequence for PHY blocks - serdes, tx, rx, pcs */ @@ -864,11 +861,6 @@ struct qmp_phy_cfg { }; -struct qmp_phy_combo_cfg { - const struct qmp_phy_cfg *usb_cfg; - const struct qmp_phy_cfg *dp_cfg; -}; - /** * struct qmp_phy - per-lane phy descriptor * @@ -1008,8 +1000,7 @@ static const char * const sc7180_usb3phy_reset_l[] = { "phy", }; -static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { - .type = PHY_TYPE_USB3, +static const struct qmp_phy_cfg sc7180_usb3dpphy_cfg = { .lanes = 2, .serdes_tbl = qmp_v3_usb3_serdes_tbl, @@ -1020,20 +1011,6 @@ static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { .rx_tbl_num = ARRAY_SIZE(qmp_v3_usb3_rx_tbl), .pcs_tbl = qmp_v3_usb3_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(qmp_v3_usb3_pcs_tbl), - .clk_list = qmp_v3_phy_clk_l, - .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), - .reset_list = sc7180_usb3phy_reset_l, - .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), - .vreg_list = qmp_phy_vreg_l, - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), - .regs = qmp_v3_usb3phy_regs_layout, - - .has_pwrdn_delay = true, -}; - -static const struct qmp_phy_cfg sc7180_dpphy_cfg = { - .type = PHY_TYPE_DP, - .lanes = 2, .dp_serdes_tbl = qmp_v3_dp_serdes_tbl, .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v3_dp_serdes_tbl), @@ -1058,15 +1035,19 @@ static const struct qmp_phy_cfg sc7180_dpphy_cfg = { .configure_dp_tx = qcom_qmp_v3_phy_configure_dp_tx, .configure_dp_phy = qcom_qmp_v3_phy_configure_dp_phy, .calibrate_dp_phy = qcom_qmp_v3_dp_phy_calibrate, -}; -static const struct qmp_phy_combo_cfg sc7180_usb3dpphy_cfg = { - .usb_cfg = &sc7180_usb3phy_cfg, - .dp_cfg = &sc7180_dpphy_cfg, + .clk_list = qmp_v3_phy_clk_l, + .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), + .reset_list = sc7180_usb3phy_reset_l, + .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), + .vreg_list = qmp_phy_vreg_l, + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), + .regs = qmp_v3_usb3phy_regs_layout, + + .has_pwrdn_delay = true, }; -static const struct qmp_phy_cfg sdm845_usb3phy_cfg = { - .type = PHY_TYPE_USB3, +static const struct qmp_phy_cfg sdm845_usb3dpphy_cfg = { .lanes = 2, .serdes_tbl = qmp_v3_usb3_serdes_tbl, @@ -1077,25 +1058,11 @@ static const struct qmp_phy_cfg sdm845_usb3phy_cfg = { .rx_tbl_num = ARRAY_SIZE(qmp_v3_usb3_rx_tbl), .pcs_tbl = qmp_v3_usb3_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(qmp_v3_usb3_pcs_tbl), - .clk_list = qmp_v3_phy_clk_l, - .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), - .reset_list = msm8996_usb3phy_reset_l, - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), - .vreg_list = qmp_phy_vreg_l, - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), - .regs = qmp_v3_usb3phy_regs_layout, - - .has_pwrdn_delay = true, -}; -static const struct qmp_phy_cfg sdm845_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), @@ -1115,15 +1082,18 @@ static const struct qmp_phy_cfg sdm845_dpphy_cfg = { .configure_dp_tx = qcom_qmp_v3_phy_configure_dp_tx, .configure_dp_phy = qcom_qmp_v3_phy_configure_dp_phy, .calibrate_dp_phy = qcom_qmp_v3_dp_phy_calibrate, -}; -static const struct qmp_phy_combo_cfg sdm845_usb3dpphy_cfg = { - .usb_cfg = &sdm845_usb3phy_cfg, - .dp_cfg = &sdm845_dpphy_cfg, -}; + .clk_list = qmp_v3_phy_clk_l, + .num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l), + .reset_list = msm8996_usb3phy_reset_l, + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), + .vreg_list = qmp_phy_vreg_l, + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), + .regs = qmp_v3_usb3phy_regs_layout, -static const struct qmp_phy_cfg sm8150_usb3phy_cfg = { - .type = PHY_TYPE_USB3, + .has_pwrdn_delay = true, +}; +static const struct qmp_phy_cfg sc8180x_usb3dpphy_cfg = { .lanes = 2, .serdes_tbl = sm8150_usb3_serdes_tbl, @@ -1136,21 +1106,6 @@ static const struct qmp_phy_cfg sm8150_usb3phy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sm8150_usb3_pcs_tbl), .pcs_usb_tbl = sm8150_usb3_pcs_usb_tbl, .pcs_usb_tbl_num = ARRAY_SIZE(sm8150_usb3_pcs_usb_tbl), - .clk_list = qmp_v4_phy_clk_l, - .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), - .reset_list = msm8996_usb3phy_reset_l, - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), - .vreg_list = qmp_phy_vreg_l, - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), - .regs = qmp_v4_usb3phy_regs_layout, - .pcs_usb_offset = 0x300, - - .has_pwrdn_delay = true, -}; - -static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { - .type = PHY_TYPE_DP, - .lanes = 2, .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), @@ -1175,15 +1130,20 @@ static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, .configure_dp_phy = qcom_qmp_v4_phy_configure_dp_phy, .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, -}; -static const struct qmp_phy_combo_cfg sc8180x_usb3dpphy_cfg = { - .usb_cfg = &sm8150_usb3phy_cfg, - .dp_cfg = &sc8180x_dpphy_cfg, + .clk_list = qmp_v4_phy_clk_l, + .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), + .reset_list = msm8996_usb3phy_reset_l, + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), + .vreg_list = qmp_phy_vreg_l, + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), + .regs = qmp_v4_usb3phy_regs_layout, + .pcs_usb_offset = 0x300, + + .has_pwrdn_delay = true, }; -static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = { - .type = PHY_TYPE_USB3, +static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = { .lanes = 2, .serdes_tbl = sc8280xp_usb43dp_serdes_tbl, @@ -1194,19 +1154,6 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = { .rx_tbl_num = ARRAY_SIZE(sc8280xp_usb43dp_rx_tbl), .pcs_tbl = sc8280xp_usb43dp_pcs_tbl, .pcs_tbl_num = ARRAY_SIZE(sc8280xp_usb43dp_pcs_tbl), - .clk_list = qmp_v4_phy_clk_l, - .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), - .reset_list = msm8996_usb3phy_reset_l, - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), - .vreg_list = qmp_phy_vreg_l, - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), - .regs = qmp_v4_usb3phy_regs_layout, - .pcs_usb_offset = 0x300, -}; - -static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = { - .type = PHY_TYPE_DP, - .lanes = 2, .dp_serdes_tbl = qmp_v5_dp_serdes_tbl, .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v5_dp_serdes_tbl), @@ -1231,15 +1178,18 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = { .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, .configure_dp_phy = qcom_qmp_v5_phy_configure_dp_phy, .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, -}; -static const struct qmp_phy_combo_cfg sc8280xp_usb43dpphy_cfg = { - .usb_cfg = &sc8280xp_usb43dp_usb_cfg, - .dp_cfg = &sc8280xp_usb43dp_dp_cfg, + .clk_list = qmp_v4_phy_clk_l, + .num_clks = ARRAY_SIZE(qmp_v4_phy_clk_l), + .reset_list = msm8996_usb3phy_reset_l, + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), + .vreg_list = qmp_phy_vreg_l, + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), + .regs = qmp_v4_usb3phy_regs_layout, + .pcs_usb_offset = 0x300, }; -static const struct qmp_phy_cfg sm8250_usb3phy_cfg = { - .type = PHY_TYPE_USB3, +static const struct qmp_phy_cfg sm8250_usb3dpphy_cfg = { .lanes = 2, .serdes_tbl = sm8150_usb3_serdes_tbl, @@ -1252,21 +1202,6 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = { .pcs_tbl_num = ARRAY_SIZE(sm8250_usb3_pcs_tbl), .pcs_usb_tbl = sm8250_usb3_pcs_usb_tbl, .pcs_usb_tbl_num = ARRAY_SIZE(sm8250_usb3_pcs_usb_tbl), - .clk_list = qmp_v4_sm8250_usbphy_clk_l, - .num_clks = ARRAY_SIZE(qmp_v4_sm8250_usbphy_clk_l), - .reset_list = msm8996_usb3phy_reset_l, - .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), - .vreg_list = qmp_phy_vreg_l, - .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), - .regs = qmp_v4_usb3phy_regs_layout, - .pcs_usb_offset = 0x300, - - .has_pwrdn_delay = true, -}; - -static const struct qmp_phy_cfg sm8250_dpphy_cfg = { - .type = PHY_TYPE_DP, - .lanes = 2, .dp_serdes_tbl = qmp_v4_dp_serdes_tbl, .dp_serdes_tbl_num = ARRAY_SIZE(qmp_v4_dp_serdes_tbl), @@ -1291,11 +1226,17 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = { .configure_dp_tx = qcom_qmp_v4_phy_configure_dp_tx, .configure_dp_phy = qcom_qmp_v4_phy_configure_dp_phy, .calibrate_dp_phy = qcom_qmp_v4_dp_phy_calibrate, -}; -static const struct qmp_phy_combo_cfg sm8250_usb3dpphy_cfg = { - .usb_cfg = &sm8250_usb3phy_cfg, - .dp_cfg = &sm8250_dpphy_cfg, + .clk_list = qmp_v4_sm8250_usbphy_clk_l, + .num_clks = ARRAY_SIZE(qmp_v4_sm8250_usbphy_clk_l), + .reset_list = msm8996_usb3phy_reset_l, + .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), + .vreg_list = qmp_phy_vreg_l, + .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), + .regs = qmp_v4_usb3phy_regs_layout, + .pcs_usb_offset = 0x300, + + .has_pwrdn_delay = true, }; static void qmp_combo_configure_lane(void __iomem *base, @@ -2720,10 +2661,7 @@ static int qmp_combo_probe(struct platform_device *pdev) void __iomem *serdes; void __iomem *usb_serdes; void __iomem *dp_serdes = NULL; - const struct qmp_phy_combo_cfg *combo_cfg = NULL; const struct qmp_phy_cfg *cfg = NULL; - const struct qmp_phy_cfg *usb_cfg = NULL; - const struct qmp_phy_cfg *dp_cfg = NULL; int num, id, expected_phys; int ret; @@ -2734,13 +2672,10 @@ static int qmp_combo_probe(struct platform_device *pdev) qmp->dev = dev; dev_set_drvdata(dev, qmp); - combo_cfg = of_device_get_match_data(dev); - if (!combo_cfg) + cfg = of_device_get_match_data(dev); + if (!cfg) return -EINVAL; - usb_cfg = combo_cfg->usb_cfg; - cfg = usb_cfg; /* Setup clks and regulators */ - usb_serdes = serdes = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(serdes)) return PTR_ERR(serdes); @@ -2753,7 +2688,6 @@ static int qmp_combo_probe(struct platform_device *pdev) if (IS_ERR(dp_serdes)) return PTR_ERR(dp_serdes); - dp_cfg = combo_cfg->dp_cfg; expected_phys = 2; mutex_init(&qmp->phy_mutex); @@ -2792,7 +2726,6 @@ static int qmp_combo_probe(struct platform_device *pdev) id = 0; for_each_available_child_of_node(dev->of_node, child) { if (of_node_name_eq(child, "dp-phy")) { - cfg = dp_cfg; serdes = dp_serdes; /* Create per-lane phy */ @@ -2810,7 +2743,6 @@ static int qmp_combo_probe(struct platform_device *pdev) goto err_node_put; } } else if (of_node_name_eq(child, "usb3-phy")) { - cfg = usb_cfg; serdes = usb_serdes; /* Create per-lane phy */
It does not really make any sense to keep separate configuration structures for the USB and DP parts of the same PHY so merge them. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 182 +++++++--------------- 1 file changed, 57 insertions(+), 125 deletions(-)