Messages in this thread | | | Date | Tue, 17 Jan 2023 16:08:10 +0200 | Subject | Re: [PATCH 07/10] phy: qualcomm: qmp-pcie: Add support for SM8550 g3x2 and g4x2 PCIEs | From | Dmitry Baryshkov <> |
| |
On 17/01/2023 15:57, Abel Vesa wrote: > On 23-01-01 22:15:55, Dmitry Baryshkov wrote: >> On 16/11/2022 14:01, 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. >>> >>> Co-developed-by: Neil Armstrong <neil.armstrong@linaro.org> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 354 +++++++++++++++++++++++ >>> 1 file changed, 354 insertions(+) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>> index 47cccc4b35b2..87c7c20dfc8d 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >> >> [skipped tables] >> >>> @@ -1473,6 +1701,8 @@ struct qmp_pcie_offsets { >>> struct qmp_phy_cfg_tbls { >>> const struct qmp_phy_init_tbl *serdes; >>> int serdes_num; >>> + const struct qmp_phy_init_tbl *ln_shrd_serdes; >>> + int ln_shrd_serdes_num; >>> const struct qmp_phy_init_tbl *tx; >>> int tx_num; >>> const struct qmp_phy_init_tbl *rx; >>> @@ -1510,6 +1740,9 @@ struct qmp_phy_cfg { >>> /* resets to be requested */ >>> const char * const *reset_list; >>> int num_resets; >>> + /* no CSR resets to be requested */ >>> + const char * const *nocsr_reset_list; >>> + int num_nocsr_resets; >> >> Is there any difference between 'no CSR' resets and the plain ones? Can we >> handle them in a single array instead? > > Yes, on power on, only the 'No CSR' are necessary to be deasserted. > > So we need to differentiate between 'No CSR' and the rest.
Ack.
> >> >>> /* regulators to be requested */ >>> const char * const *vreg_list; >>> int num_vregs;
[skipped]
>>> @@ -2502,6 +2847,9 @@ static int qmp_pcie_parse_dt_legacy(struct qmp_pcie *qmp, struct device_node *np >>> return PTR_ERR(qmp->rx2); >>> qmp->pcs_misc = devm_of_iomap(dev, np, 5, NULL); >>> + >>> + if (cfg->has_ln_shrd_serdes_tbl) >>> + qmp->ln_shrd_serdes = devm_of_iomap(dev, np, 6, NULL); >> >> I think we also need to check the returned value. Also, I think we can drop >> the conditional check here. we don't have to validate the DT, so if the reg >> is present in DT, then it's present. If not, it's not required. > > Yeah, but I was trying to make sure that if there is a new type of reg > for some upcomming SoC, we don't take that as ln_shrd. That's why the check > was added. > > I'll add the returned value check though.
This is the parse_dt_legacy() function. I think we can leave it as is and support sm8550 only using the new style of parsing.
> >> >>> } else { >>> qmp->pcs_misc = devm_of_iomap(dev, np, 3, NULL); >>> } >>> @@ -2729,6 +3077,12 @@ static const struct of_device_id qmp_pcie_of_match_table[] = { >>> }, { >>> .compatible = "qcom,sm8450-qmp-gen4x2-pcie-phy", >>> .data = &sm8450_qmp_gen4x2_pciephy_cfg, >>> + }, { >>> + .compatible = "qcom,sm8550-qmp-gen3x2-pcie-phy", >>> + .data = &sm8550_qmp_gen3x2_pciephy_cfg, >>> + }, { >>> + .compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy", >>> + .data = &sm8550_qmp_gen4x2_pciephy_cfg, >>> }, >>> { }, >>> }; >> >> -- >> With best wishes >> Dmitry >>
-- With best wishes Dmitry
| |