[PATCH] phy: qcom-qmp-combo: correct DP register offsets

[Dmitry Baryshkov]

On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan at kernel.org> wrote:
>
> On Mon, Jan 23, 2023 at 02:08:07PM +0200, Dmitry Baryshkov wrote:
> > Correct DP register offsets used with new DT bindings scheme. First, DP
> > TX registers reside in separate regions, not in the same regions as USB
> > TX registers do. Second, correct DP_PHY region offset to follow the
> > offset used for earlier generations/bindings.
>
> No, this patch is broken. SC8280XP is different, doesn't seem to have
> separate DP TX regions and the DP_PHY registers lies at a different
> offset than on previous generations.
>
> You can't just pull these numbers out of your ... ;)
>
> This is the only platform that I can test the DP part on and it is
> working. If that happens to be by chance, then you need to blame the
> commit adding support for sc8280xp (i.e. not the one that fixed the
> binding). And note that this was added by Bjorn who do have access to
> the documentation for this SoC (as well as actual hardware).

Ack, let's wait for Bjorn to check the offsets. I find it extremely
suspicious that dp_txa/txb use the same region as usb txa/txb do.

>
> > Cc: Johan Hovold <johan+linaro at kernel.org>
> > Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > index 1f022e580407..c6634f92de19 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > @@ -807,6 +807,8 @@ struct qmp_combo_offsets {
> >       u16 usb3_pcs;
> >       u16 usb3_pcs_usb;
> >       u16 dp_serdes;
> > +     u16 dp_txa;
> > +     u16 dp_txb;
> >       u16 dp_dp_phy;
> >  };
> >
> > @@ -984,7 +986,9 @@ static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
> >       .usb3_pcs       = 0x1400,
> >       .usb3_pcs_usb   = 0x1700,
> >       .dp_serdes      = 0x2000,
> > -     .dp_dp_phy      = 0x2200,
> > +     .dp_txa         = 0x2200,
> > +     .dp_txa         = 0x2600,
>
> You added dp_txa twice.

Ack

>
> > +     .dp_dp_phy      = 0x2a00,
> >  };
> >
> >  static const struct qmp_phy_cfg sc7180_usb3dpphy_cfg = {
> > @@ -2639,8 +2643,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
> >       qmp->pcs_usb = base + offs->usb3_pcs_usb;
> >
> >       qmp->dp_serdes = base + offs->dp_serdes;
> > -     qmp->dp_tx = base + offs->txa;
> > -     qmp->dp_tx2 = base + offs->txb;
> > +     qmp->dp_tx = base + offs->dp_txa;
> > +     qmp->dp_tx2 = base + offs->dp_txb;
> >       qmp->dp_dp_phy = base + offs->dp_dp_phy;
> >
> >       qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");
>
> Johan



-- 
With best wishes
Dmitry