On Mon, Oct 31, 2022 at 09:50:59PM +0300, Dmitry Baryshkov wrote:
> On 31/10/2022 18:46, Manivannan Sadhasivam wrote:
> > On Sun, Oct 30, 2022 at 12:50:50AM +0300, Dmitry Baryshkov wrote:
> > > On 29/10/2022 17:16, Manivannan Sadhasivam wrote:
> > > > As done for Qcom PCIe PHY driver, let's move the register settings to the
> > > > common qmp_phy_cfg_tables struct. This helps in adding any additional PHY
> > > > settings needed for functionalities like HS-G4 in the future by adding one
> > > > more instance of the qmp_phy_cfg_tables.
> > > >
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > > > ---
> > > > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 223 +++++++++++++-----------
> > > > 1 file changed, 126 insertions(+), 97 deletions(-)
> > > >
> > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
[...]
> > > > static int qmp_ufs_com_init(struct qmp_phy *qphy)
> > > > @@ -933,31 +977,16 @@ static int qmp_ufs_power_on(struct phy *phy)
> > > > struct qmp_phy *qphy = phy_get_drvdata(phy);
> > > > struct qcom_qmp *qmp = qphy->qmp;
> > > > const struct qmp_phy_cfg *cfg = qphy->cfg;
> > > > - void __iomem *tx = qphy->tx;
> > > > - void __iomem *rx = qphy->rx;
> > > > void __iomem *pcs = qphy->pcs;
> > > > void __iomem *status;
> > > > unsigned int mask, val, ready;
> > > > int ret;
> > > > - qmp_ufs_serdes_init(qphy);
> > > > -
> > > > - /* Tx, Rx, and PCS configurations */
> > > > - qmp_ufs_configure_lane(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num, 1);
> > > > + qmp_ufs_serdes_init(qphy, &cfg->tables);
> > > > - if (cfg->lanes >= 2) {
> > > > - qmp_ufs_configure_lane(qphy->tx2, cfg->regs,
> > > > - cfg->tx_tbl, cfg->tx_tbl_num, 2);
> > > > - }
> > > > -
> > > > - qmp_ufs_configure_lane(rx, cfg->regs, cfg->rx_tbl, cfg->rx_tbl_num, 1);
> > > > -
> > > > - if (cfg->lanes >= 2) {
> > > > - qmp_ufs_configure_lane(qphy->rx2, cfg->regs,
> > > > - cfg->rx_tbl, cfg->rx_tbl_num, 2);
> > > > - }
> > > > + qmp_ufs_lanes_init(qphy, &cfg->tables);
> > > > - qmp_ufs_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
> > > > + qmp_ufs_pcs_init(qphy, &cfg->tables);
> > >
> > > I'd suggest going straight to qmp_ufs_init_registers, which would contain
> > > both serdes, lanes and pcs inits.
> > >
> >
> > That adds one more level of indirection which may not be needed here. Moreover,
> > I'm trying to be in sync with other qmp drivers, specifically the pcie one.
> > This helps in working with these drivers.
>
> Yes, I understand. However I hope that the respective patchset (including
> [1]) will be merged soon. Thus I suggest skipping the step and using the
> same function already.
>
> [1] https://lore.kernel.org/linux-phy/20221028133603.18470-10-johan+linaro@xxxxxxxxxx/
>
Ah, I missed this series. Will use the common function then.
Thanks,
Mani
> >
> > Thanks,
> > Mani
> >
> > > > ret = reset_control_deassert(qmp->ufs_reset);
> > > > if (ret)
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > >
> >
>
> --
> With best wishes
> Dmitry
>
--
மணிவண்ணன் சதாசிவம்
