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

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

[Dmitry Baryshkov]

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.

Cc: Johan Hovold <johan+linaro@kernel.org>
Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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,
+	.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");
-- 
2.39.0

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

[Dmitry Baryshkov]

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.

Cc: Johan Hovold <johan+linaro@kernel.org>
Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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,
+	.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");
-- 
2.39.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Johan Hovold]

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).

> Cc: Johan Hovold <johan+linaro@kernel.org>
> Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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.

> +	.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

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

[Johan Hovold]

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).

> Cc: Johan Hovold <johan+linaro@kernel.org>
> Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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.

> +	.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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

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

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

[Dmitry Baryshkov]

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Johan Hovold]

On Mon, Jan 23, 2023 at 08:59:48PM +0200, Dmitry Baryshkov wrote:
> On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan@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.

Yeah, I agree, it seems odd, but then again the sc8280xp PHY is a
different beast which also supports USB4, etc. The DP_PHY offset comes
from the header file in the vendor tree and there are no separate DP TX
registers there.

By the way, I forgot that Bjorn actually copy-pasted the devicetree
nodes from an earlier platform when first adding the PHY and only later
realised that the DP part did not work. The fix for that (e.g. to use
the same TX registers) was never merged to mainline (instead we
temporarily disabled the DP part) so the Fixes tag you used would have
been correct if this turns out to be wrong.

> > > Cc: Johan Hovold <johan+linaro@kernel.org>
> > > Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Johan

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

[Johan Hovold]

On Mon, Jan 23, 2023 at 08:59:48PM +0200, Dmitry Baryshkov wrote:
> On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan@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.

Yeah, I agree, it seems odd, but then again the sc8280xp PHY is a
different beast which also supports USB4, etc. The DP_PHY offset comes
from the header file in the vendor tree and there are no separate DP TX
registers there.

By the way, I forgot that Bjorn actually copy-pasted the devicetree
nodes from an earlier platform when first adding the PHY and only later
realised that the DP part did not work. The fix for that (e.g. to use
the same TX registers) was never merged to mainline (instead we
temporarily disabled the DP part) so the Fixes tag you used would have
been correct if this turns out to be wrong.

> > > Cc: Johan Hovold <johan+linaro@kernel.org>
> > > Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Dmitry Baryshkov]

On 24/01/2023 09:35, Johan Hovold wrote:
> On Mon, Jan 23, 2023 at 08:59:48PM +0200, Dmitry Baryshkov wrote:
>> On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan@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.
> 
> Yeah, I agree, it seems odd, but then again the sc8280xp PHY is a
> different beast which also supports USB4, etc. The DP_PHY offset comes
> from the header file in the vendor tree and there are no separate DP TX
> registers there.
> 
> By the way, I forgot that Bjorn actually copy-pasted the devicetree
> nodes from an earlier platform when first adding the PHY and only later
> realised that the DP part did not work. The fix for that (e.g. to use
> the same TX registers) was never merged to mainline (instead we
> temporarily disabled the DP part) so the Fixes tag you used would have
> been correct if this turns out to be wrong.

Let's check with Bjorn. However even if the original offsets are 
correct, I think we should add dp_txa/dp_txb offsets (and make them 
equal to txa/txb in sc8280xp case). WDYT?

> 
>>>> Cc: Johan Hovold <johan+linaro@kernel.org>
>>>> Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Johan

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

On 24/01/2023 09:35, Johan Hovold wrote:
> On Mon, Jan 23, 2023 at 08:59:48PM +0200, Dmitry Baryshkov wrote:
>> On Mon, 23 Jan 2023 at 16:00, Johan Hovold <johan@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.
> 
> Yeah, I agree, it seems odd, but then again the sc8280xp PHY is a
> different beast which also supports USB4, etc. The DP_PHY offset comes
> from the header file in the vendor tree and there are no separate DP TX
> registers there.
> 
> By the way, I forgot that Bjorn actually copy-pasted the devicetree
> nodes from an earlier platform when first adding the PHY and only later
> realised that the DP part did not work. The fix for that (e.g. to use
> the same TX registers) was never merged to mainline (instead we
> temporarily disabled the DP part) so the Fixes tag you used would have
> been correct if this turns out to be wrong.

Let's check with Bjorn. However even if the original offsets are 
correct, I think we should add dp_txa/dp_txb offsets (and make them 
equal to txa/txb in sc8280xp case). WDYT?

> 
>>>> Cc: Johan Hovold <johan+linaro@kernel.org>
>>>> Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Johan

-- 
With best wishes
Dmitry


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Johan Hovold]

On Tue, Jan 24, 2023 at 11:19:32AM +0200, Dmitry Baryshkov wrote:

> Let's check with Bjorn. However even if the original offsets are 
> correct, I think we should add dp_txa/dp_txb offsets (and make them 
> equal to txa/txb in sc8280xp case). WDYT?

That is already being done as part of Luca's series:

	https://lore.kernel.org/lkml/20230120-sm6350-usbphy-v4-2-4d700a90ba16@fairphone.com/

Johan

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

[Johan Hovold]

On Tue, Jan 24, 2023 at 11:19:32AM +0200, Dmitry Baryshkov wrote:

> Let's check with Bjorn. However even if the original offsets are 
> correct, I think we should add dp_txa/dp_txb offsets (and make them 
> equal to txa/txb in sc8280xp case). WDYT?

That is already being done as part of Luca's series:

	https://lore.kernel.org/lkml/20230120-sm6350-usbphy-v4-2-4d700a90ba16@fairphone.com/

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy