public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Marcin Wojtas <mw@semihalf.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	ard.biesheuvel@arm.com, "jsd@semihalf.com" <jsd@semihalf.com>,
	Grzegorz Jaszczyk <jaz@semihalf.com>,
	Kostya Porotchkin <kostap@marvell.com>
Subject: Re: [edk2-platforms: PATCH v2 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings
Date: Mon, 18 May 2020 19:16:13 +0100	[thread overview]
Message-ID: <20200518181613.GJ10467@vanye> (raw)
In-Reply-To: <CAPv3WKeyD2J0-qNYWt6uLsXtZzYnyGLUffk1JojA7OybxkN4Cw@mail.gmail.com>

On Mon, May 18, 2020 at 20:11:49 +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> pon., 18 maj 2020 o 19:12 Leif Lindholm <leif@nuviainc.com> napisał(a):
> >
> > On Fri, May 15, 2020 at 23:05:56 +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>
> >
> > I'm OK with the current version of the code, but just noticed this.
> > No one can give Signed-off-by for another.
> > If Grzegorz is the author, that should be noted in a From: tag. Git
> > format-patch does this automatically if the commit's Author metadata
> > is set.
> >
> > This applies to all 3 patches.
> >
> 
> The 3/3 has only only my signed-off tag (and my authorship).

OK, I admit it, I was lazy and spotted this and only checked 2/3 and
assumed it was all of them :)

> Regarding the first 2, I did the actual change, but it was based on
> the original U-Boot patch from Grzegorz. How about, instead of the tag,
> I give him a credit in a following way:
> 
> Based on the original U-Boot patch from Grzegorz Jaszczyk <jaz@semihalf.com>
> 
> Would that work for you?

I have no objections to that as such, but perhaps a link to the patch
posting in an email archive, or the commit in a git repo, would be
more generically useful.

Best Regards,

Leif

> Best regards,
> Marcin
> 
> >
> > > ---
> > >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h | 10 +++++++++-
> > >  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c | 18 ++++++++++++++----
> > >  2 files changed, 23 insertions(+), 5 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..42f38db 100644
> > > --- a/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > > +++ b/Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> > > @@ -118,23 +118,33 @@ UtmiPhyConfig (
> > >
> > >    /* Impedance Calibration Threshold Setting */
> > >    RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG,
> > > -    0x6 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
> > > +    0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET,
> > >      UTMI_CALIB_CTRL_IMPCAL_VTH_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;
> > >    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
> > >

  reply	other threads:[~2020-05-18 18:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 21:05 [edk2-platforms: PATCH v2 0/3] Marvell UTMI fixes Marcin Wojtas
2020-05-15 21:05 ` [edk2-platforms: PATCH v2 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings Marcin Wojtas
2020-05-18 17:12   ` Leif Lindholm
2020-05-18 18:11     ` Marcin Wojtas
2020-05-18 18:16       ` Leif Lindholm [this message]
2020-05-18 23:09         ` Marcin Wojtas
2020-05-19 15:26           ` Leif Lindholm
2020-05-15 21:05 ` [edk2-platforms: PATCH v2 2/3] Marvell/Library: UtmiLib: Fix pll initialization for the second port Marcin Wojtas
2020-05-15 21:05 ` [edk2-platforms: PATCH v2 3/3] Marvell/Library: UtmiLib: Fix USB mux configuration Marcin Wojtas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200518181613.GJ10467@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox