On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > ---
> > 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
