[edk2-devel] [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings

Marcin Wojtas posted 3 patches 5 years, 9 months ago
There is a newer version of this series
[edk2-devel] [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings
Posted by Marcin Wojtas 5 years, 9 months ago
This patch introduce following modifications, allowing to
overcome the instabilities observed with certain USB2.0 endpoints:
* Add additional step which enables the Impedance and PLL calibration.
* Enable old squelch detector instead of the new analog squelch detector
  circuit and update host disconnect threshold value.
* Update LS TX driver strength coarse and fine adjustment values.

Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h | 10 +++++++-
 Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c | 26 ++++++++++++++------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
index 20e3133..8659110 100644
--- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
+++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
@@ -44,6 +44,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define UTMI_CALIB_CTRL_REG                       0x8
 #define UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET         8
 #define UTMI_CALIB_CTRL_IMPCAL_VTH_MASK           (0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET)
+#define UTMI_CALIB_CTRL_IMPCAL_START_OFFSET       13
+#define UTMI_CALIB_CTRL_IMPCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET)
+#define UTMI_CALIB_CTRL_PLLCAL_START_OFFSET       22
+#define UTMI_CALIB_CTRL_PLLCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET)
 #define UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET        23
 #define UTMI_CALIB_CTRL_IMPCAL_DONE_MASK          (0x1 << UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET)
 #define UTMI_CALIB_CTRL_PLLCAL_DONE_OFFSET        31
@@ -54,8 +58,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define UTMI_TX_CH_CTRL_DRV_EN_LS_MASK            (0xf << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET)
 #define UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET         16
 #define UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK           (0xf << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET)
+#define UTMI_TX_CH_CTRL_AMP_OFFSET                20
+#define UTMI_TX_CH_CTRL_AMP_MASK                  (0x7 << UTMI_TX_CH_CTRL_AMP_OFFSET)
 
 #define UTMI_RX_CH_CTRL0_REG                      0x14
+#define UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET     8
+#define UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK       (0x3 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET)
 #define UTMI_RX_CH_CTRL0_SQ_DET_OFFSET            15
 #define UTMI_RX_CH_CTRL0_SQ_DET_MASK              (0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET)
 #define UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET        28
@@ -63,7 +71,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #define UTMI_RX_CH_CTRL1_REG                      0x18
 #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET        0
-#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x3 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
+#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x7 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
 #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET     3
 #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_MASK       (0x1 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET)
 
diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
index 3881ebd..60ea06e 100644
--- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
+++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
@@ -117,24 +117,34 @@ UtmiPhyConfig (
   RegSet (UtmiBaseAddr + UTMI_PLL_CTRL_REG, Data, Mask);
 
   /* Impedance Calibration Threshold Setting */
-  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG,
-    0x6 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
-    UTMI_CALIB_CTRL_IMPCAL_VTH_MASK);
+  Mask = UTMI_CALIB_CTRL_IMPCAL_VTH_MASK;
+  Data = 0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET;
+  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
+
+  /* Start Impedance and PLL Calibration */
+  Mask = UTMI_CALIB_CTRL_PLLCAL_START_MASK;
+  Data = (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET);
+  Mask |= UTMI_CALIB_CTRL_IMPCAL_START_MASK;
+  Data |= (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET);
+  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
 
   /* Set LS TX driver strength coarse control */
-  Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
-  Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
-  /* Set LS TX driver fine adjustment */
+  Mask = UTMI_TX_CH_CTRL_AMP_MASK;
+  Data = 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
   Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
   Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
+  Mask |= UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
+  Data |= 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
   RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);
 
   /* Enable SQ */
   Mask = UTMI_RX_CH_CTRL0_SQ_DET_MASK;
-  Data = 0x0 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
+  Data = 0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
   /* Enable analog squelch detect */
   Mask |= UTMI_RX_CH_CTRL0_SQ_ANA_DTC_MASK;
-  Data |= 0x1 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
+  Data |= 0x0 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
+  Mask |= UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK;
+  Data |= 0x0 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET;
   RegSet (UtmiBaseAddr + UTMI_RX_CH_CTRL0_REG, Data, Mask);
 
   /* Set External squelch calibration number */
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59358): https://edk2.groups.io/g/devel/message/59358
Mute This Topic: https://groups.io/mt/74165929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings
Posted by Leif Lindholm 5 years, 9 months ago
On Tue, May 12, 2020 at 20:59:29 +0200, Marcin Wojtas wrote:
> This patch introduce following modifications, allowing to
> overcome the instabilities observed with certain USB2.0 endpoints:
> * Add additional step which enables the Impedance and PLL calibration.
> * Enable old squelch detector instead of the new analog squelch detector
>   circuit and update host disconnect threshold value.
> * Update LS TX driver strength coarse and fine adjustment values.
> 
> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h | 10 +++++++-
>  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c | 26 ++++++++++++++------
>  2 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> index 20e3133..8659110 100644
> --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> @@ -44,6 +44,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define UTMI_CALIB_CTRL_REG                       0x8
>  #define UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET         8
>  #define UTMI_CALIB_CTRL_IMPCAL_VTH_MASK           (0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET)
> +#define UTMI_CALIB_CTRL_IMPCAL_START_OFFSET       13
> +#define UTMI_CALIB_CTRL_IMPCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET)
> +#define UTMI_CALIB_CTRL_PLLCAL_START_OFFSET       22
> +#define UTMI_CALIB_CTRL_PLLCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET)
>  #define UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET        23
>  #define UTMI_CALIB_CTRL_IMPCAL_DONE_MASK          (0x1 << UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET)
>  #define UTMI_CALIB_CTRL_PLLCAL_DONE_OFFSET        31
> @@ -54,8 +58,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define UTMI_TX_CH_CTRL_DRV_EN_LS_MASK            (0xf << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET)
>  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET         16
>  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK           (0xf << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET)
> +#define UTMI_TX_CH_CTRL_AMP_OFFSET                20
> +#define UTMI_TX_CH_CTRL_AMP_MASK                  (0x7 << UTMI_TX_CH_CTRL_AMP_OFFSET)
>  
>  #define UTMI_RX_CH_CTRL0_REG                      0x14
> +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET     8
> +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK       (0x3 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET)
>  #define UTMI_RX_CH_CTRL0_SQ_DET_OFFSET            15
>  #define UTMI_RX_CH_CTRL0_SQ_DET_MASK              (0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET)
>  #define UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET        28
> @@ -63,7 +71,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #define UTMI_RX_CH_CTRL1_REG                      0x18
>  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET        0
> -#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x3 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
> +#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x7 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
>  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET     3
>  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_MASK       (0x1 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET)
>  
> diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> index 3881ebd..60ea06e 100644
> --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> @@ -117,24 +117,34 @@ UtmiPhyConfig (
>    RegSet (UtmiBaseAddr + UTMI_PLL_CTRL_REG, Data, Mask);
>  
>    /* Impedance Calibration Threshold Setting */
> -  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG,
> -    0x6 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
> -    UTMI_CALIB_CTRL_IMPCAL_VTH_MASK);
> +  Mask = UTMI_CALIB_CTRL_IMPCAL_VTH_MASK;
> +  Data = 0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET;
> +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
> +
> +  /* Start Impedance and PLL Calibration */
> +  Mask = UTMI_CALIB_CTRL_PLLCAL_START_MASK;
> +  Data = (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET);
> +  Mask |= UTMI_CALIB_CTRL_IMPCAL_START_MASK;
> +  Data |= (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET);
> +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
>  
>    /* Set LS TX driver strength coarse control */
> -  Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> -  Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> -  /* Set LS TX driver fine adjustment */
> +  Mask = UTMI_TX_CH_CTRL_AMP_MASK;
> +  Data = 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
>    Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
>    Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
> +  Mask |= UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> +  Data |= 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
>    RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);

The above looks a bit more chaotic than necessary, partly because what
looks like spuriously moving the existing assignment around and
deleting a comment Is there a rationale behind that?

I.e., the following would appear equivalent:

   /* Set LS TX driver strength coarse control */
   Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
   Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
+  Mask |= UTMI_TX_CH_CTRL_AMP_MASK;
+  Data |= 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
   /* Set LS TX driver fine adjustment */
   Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
   Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
   RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);

/
    Leif

>  
>    /* Enable SQ */
>    Mask = UTMI_RX_CH_CTRL0_SQ_DET_MASK;
> -  Data = 0x0 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
> +  Data = 0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
>    /* Enable analog squelch detect */
>    Mask |= UTMI_RX_CH_CTRL0_SQ_ANA_DTC_MASK;
> -  Data |= 0x1 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> +  Data |= 0x0 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> +  Mask |= UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK;
> +  Data |= 0x0 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET;
>    RegSet (UtmiBaseAddr + UTMI_RX_CH_CTRL0_REG, Data, Mask);
>  
>    /* Set External squelch calibration number */
> -- 
> 2.7.4
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59431): https://edk2.groups.io/g/devel/message/59431
Mute This Topic: https://groups.io/mt/74165929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings
Posted by Marcin Wojtas 5 years, 9 months ago
Hi Leif,

śr., 13 maj 2020 o 15:41 Leif Lindholm <leif@nuviainc.com> napisał(a):
>
> On Tue, May 12, 2020 at 20:59:29 +0200, Marcin Wojtas wrote:
> > This patch introduce following modifications, allowing to
> > overcome the instabilities observed with certain USB2.0 endpoints:
> > * Add additional step which enables the Impedance and PLL calibration.
> > * Enable old squelch detector instead of the new analog squelch detector
> >   circuit and update host disconnect threshold value.
> > * Update LS TX driver strength coarse and fine adjustment values.
> >
> > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h | 10 +++++++-
> >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c | 26 ++++++++++++++------
> >  2 files changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > index 20e3133..8659110 100644
> > --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > @@ -44,6 +44,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #define UTMI_CALIB_CTRL_REG                       0x8
> >  #define UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET         8
> >  #define UTMI_CALIB_CTRL_IMPCAL_VTH_MASK           (0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET)
> > +#define UTMI_CALIB_CTRL_IMPCAL_START_OFFSET       13
> > +#define UTMI_CALIB_CTRL_IMPCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET)
> > +#define UTMI_CALIB_CTRL_PLLCAL_START_OFFSET       22
> > +#define UTMI_CALIB_CTRL_PLLCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET)
> >  #define UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET        23
> >  #define UTMI_CALIB_CTRL_IMPCAL_DONE_MASK          (0x1 << UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET)
> >  #define UTMI_CALIB_CTRL_PLLCAL_DONE_OFFSET        31
> > @@ -54,8 +58,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #define UTMI_TX_CH_CTRL_DRV_EN_LS_MASK            (0xf << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET)
> >  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET         16
> >  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK           (0xf << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET)
> > +#define UTMI_TX_CH_CTRL_AMP_OFFSET                20
> > +#define UTMI_TX_CH_CTRL_AMP_MASK                  (0x7 << UTMI_TX_CH_CTRL_AMP_OFFSET)
> >
> >  #define UTMI_RX_CH_CTRL0_REG                      0x14
> > +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET     8
> > +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK       (0x3 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET)
> >  #define UTMI_RX_CH_CTRL0_SQ_DET_OFFSET            15
> >  #define UTMI_RX_CH_CTRL0_SQ_DET_MASK              (0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET)
> >  #define UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET        28
> > @@ -63,7 +71,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  #define UTMI_RX_CH_CTRL1_REG                      0x18
> >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET        0
> > -#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x3 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
> > +#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x7 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
> >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET     3
> >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_MASK       (0x1 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET)
> >
> > diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > index 3881ebd..60ea06e 100644
> > --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > @@ -117,24 +117,34 @@ UtmiPhyConfig (
> >    RegSet (UtmiBaseAddr + UTMI_PLL_CTRL_REG, Data, Mask);
> >
> >    /* Impedance Calibration Threshold Setting */
> > -  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG,
> > -    0x6 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
> > -    UTMI_CALIB_CTRL_IMPCAL_VTH_MASK);
> > +  Mask = UTMI_CALIB_CTRL_IMPCAL_VTH_MASK;
> > +  Data = 0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET;
> > +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
> > +
> > +  /* Start Impedance and PLL Calibration */
> > +  Mask = UTMI_CALIB_CTRL_PLLCAL_START_MASK;
> > +  Data = (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET);
> > +  Mask |= UTMI_CALIB_CTRL_IMPCAL_START_MASK;
> > +  Data |= (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET);
> > +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
> >
> >    /* Set LS TX driver strength coarse control */
> > -  Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> > -  Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> > -  /* Set LS TX driver fine adjustment */
> > +  Mask = UTMI_TX_CH_CTRL_AMP_MASK;
> > +  Data = 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
> >    Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
> >    Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
> > +  Mask |= UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> > +  Data |= 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> >    RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);
>
> The above looks a bit more chaotic than necessary, partly because what
> looks like spuriously moving the existing assignment around and
> deleting a comment Is there a rationale behind that?
>
> I.e., the following would appear equivalent:
>
>    /* Set LS TX driver strength coarse control */
>    Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
>    Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> +  Mask |= UTMI_TX_CH_CTRL_AMP_MASK;
> +  Data |= 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
>    /* Set LS TX driver fine adjustment */
>    Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
>    Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
>    RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);
>

Right, I'll recheck and simplify the diff as much as possible in this aspect.

Best regards,
Marcin

> /
>     Leif
>
> >
> >    /* Enable SQ */
> >    Mask = UTMI_RX_CH_CTRL0_SQ_DET_MASK;
> > -  Data = 0x0 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
> > +  Data = 0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
> >    /* Enable analog squelch detect */
> >    Mask |= UTMI_RX_CH_CTRL0_SQ_ANA_DTC_MASK;
> > -  Data |= 0x1 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> > +  Data |= 0x0 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> > +  Mask |= UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK;
> > +  Data |= 0x0 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET;
> >    RegSet (UtmiBaseAddr + UTMI_RX_CH_CTRL0_REG, Data, Mask);
> >
> >    /* Set External squelch calibration number */
> > --
> > 2.7.4
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59433): https://edk2.groups.io/g/devel/message/59433
Mute This Topic: https://groups.io/mt/74165929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings
Posted by Leif Lindholm 5 years, 9 months ago
On Wed, May 13, 2020 at 15:55:47 +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> śr., 13 maj 2020 o 15:41 Leif Lindholm <leif@nuviainc.com> napisał(a):
> >
> > On Tue, May 12, 2020 at 20:59:29 +0200, Marcin Wojtas wrote:
> > > This patch introduce following modifications, allowing to
> > > overcome the instabilities observed with certain USB2.0 endpoints:
> > > * Add additional step which enables the Impedance and PLL calibration.
> > > * Enable old squelch detector instead of the new analog squelch detector
> > >   circuit and update host disconnect threshold value.
> > > * Update LS TX driver strength coarse and fine adjustment values.
> > >
> > > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > ---
> > >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h | 10 +++++++-
> > >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c | 26 ++++++++++++++------
> > >  2 files changed, 27 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > > index 20e3133..8659110 100644
> > > --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > > +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> > > @@ -44,6 +44,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >  #define UTMI_CALIB_CTRL_REG                       0x8
> > >  #define UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET         8
> > >  #define UTMI_CALIB_CTRL_IMPCAL_VTH_MASK           (0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET)
> > > +#define UTMI_CALIB_CTRL_IMPCAL_START_OFFSET       13
> > > +#define UTMI_CALIB_CTRL_IMPCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET)
> > > +#define UTMI_CALIB_CTRL_PLLCAL_START_OFFSET       22
> > > +#define UTMI_CALIB_CTRL_PLLCAL_START_MASK         (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET)
> > >  #define UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET        23
> > >  #define UTMI_CALIB_CTRL_IMPCAL_DONE_MASK          (0x1 << UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET)
> > >  #define UTMI_CALIB_CTRL_PLLCAL_DONE_OFFSET        31
> > > @@ -54,8 +58,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >  #define UTMI_TX_CH_CTRL_DRV_EN_LS_MASK            (0xf << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET)
> > >  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET         16
> > >  #define UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK           (0xf << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET)
> > > +#define UTMI_TX_CH_CTRL_AMP_OFFSET                20
> > > +#define UTMI_TX_CH_CTRL_AMP_MASK                  (0x7 << UTMI_TX_CH_CTRL_AMP_OFFSET)
> > >
> > >  #define UTMI_RX_CH_CTRL0_REG                      0x14
> > > +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET     8
> > > +#define UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK       (0x3 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET)
> > >  #define UTMI_RX_CH_CTRL0_SQ_DET_OFFSET            15
> > >  #define UTMI_RX_CH_CTRL0_SQ_DET_MASK              (0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET)
> > >  #define UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET        28
> > > @@ -63,7 +71,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  #define UTMI_RX_CH_CTRL1_REG                      0x18
> > >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET        0
> > > -#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x3 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
> > > +#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK          (0x7 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_OFFSET)
> > >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET     3
> > >  #define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_MASK       (0x1 << UTMI_RX_CH_CTRL1_SQ_AMP_CAL_EN_OFFSET)
> > >
> > > diff --git a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > > index 3881ebd..60ea06e 100644
> > > --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > > +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > > @@ -117,24 +117,34 @@ UtmiPhyConfig (
> > >    RegSet (UtmiBaseAddr + UTMI_PLL_CTRL_REG, Data, Mask);
> > >
> > >    /* Impedance Calibration Threshold Setting */
> > > -  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG,
> > > -    0x6 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
> > > -    UTMI_CALIB_CTRL_IMPCAL_VTH_MASK);
> > > +  Mask = UTMI_CALIB_CTRL_IMPCAL_VTH_MASK;
> > > +  Data = 0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET;
> > > +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
> > > +
> > > +  /* Start Impedance and PLL Calibration */
> > > +  Mask = UTMI_CALIB_CTRL_PLLCAL_START_MASK;
> > > +  Data = (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET);
> > > +  Mask |= UTMI_CALIB_CTRL_IMPCAL_START_MASK;
> > > +  Data |= (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET);
> > > +  RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask);
> > >
> > >    /* Set LS TX driver strength coarse control */
> > > -  Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> > > -  Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> > > -  /* Set LS TX driver fine adjustment */
> > > +  Mask = UTMI_TX_CH_CTRL_AMP_MASK;
> > > +  Data = 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
> > >    Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
> > >    Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
> > > +  Mask |= UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> > > +  Data |= 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> > >    RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);
> >
> > The above looks a bit more chaotic than necessary, partly because what
> > looks like spuriously moving the existing assignment around and
> > deleting a comment Is there a rationale behind that?
> >
> > I.e., the following would appear equivalent:
> >
> >    /* Set LS TX driver strength coarse control */
> >    Mask = UTMI_TX_CH_CTRL_DRV_EN_LS_MASK;
> >    Data = 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET;
> > +  Mask |= UTMI_TX_CH_CTRL_AMP_MASK;
> > +  Data |= 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET;
> >    /* Set LS TX driver fine adjustment */
> >    Mask |= UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK;
> >    Data |= 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET;
> >    RegSet (UtmiBaseAddr + UTMI_TX_CH_CTRL_REG, Data, Mask);
> >
> 
> Right, I'll recheck and simplify the diff as much as possible in this aspect.

Thanks.
For clarity, I have no issue with 2-3/3, so you can just resubmit this
one after rework.

Regards,

Leif

> 
> Best regards,
> Marcin
> 
> > /
> >     Leif
> >
> > >
> > >    /* Enable SQ */
> > >    Mask = UTMI_RX_CH_CTRL0_SQ_DET_MASK;
> > > -  Data = 0x0 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
> > > +  Data = 0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET;
> > >    /* Enable analog squelch detect */
> > >    Mask |= UTMI_RX_CH_CTRL0_SQ_ANA_DTC_MASK;
> > > -  Data |= 0x1 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> > > +  Data |= 0x0 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET;
> > > +  Mask |= UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK;
> > > +  Data |= 0x0 << UTMI_RX_CH_CTRL0_DISCON_THRESH_OFFSET;
> > >    RegSet (UtmiBaseAddr + UTMI_RX_CH_CTRL0_REG, Data, Mask);
> > >
> > >    /* Set External squelch calibration number */
> > > --
> > > 2.7.4
> > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59435): https://edk2.groups.io/g/devel/message/59435
Mute This Topic: https://groups.io/mt/74165929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-