From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f196.google.com (mail-qt1-f196.google.com [209.85.160.196]) by mx.groups.io with SMTP id smtpd.web12.6489.1589378159983679328 for ; Wed, 13 May 2020 06:56:00 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=tSje4LRf; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.160.196, mailfrom: mw@semihalf.com) Received: by mail-qt1-f196.google.com with SMTP id v4so13257338qte.3 for ; Wed, 13 May 2020 06:55:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=dQSwZ1KpQjnjrrZuc1mOd7AEMVB6u6WuYygkv+ai+Pw=; b=tSje4LRf9ZoKpu/OJ/y0QKoMeZcEaXuPq5Gh1CcRK0o2J7gRr+hiRc1dxjST9vSUmH Vxaf9D7aJJOwEHVNS3NegrEfdok67PVrrXGU6KHVvuKl1rWl7AocV83ELaLheu8AuYDx kZZfyJUvfNdCFZGu7RJG09ddyQWCqATTvzABuzF35liV1xCtcHki9MS6PlvA+e4LLEHC 4zyiADUnayCFRPDDcKgPhx12umQoWff1z0eexFpkVO2VHiIe/XjdzpXfusbxPXWelL04 njnpGaGCO5FEfJn29SomXVyd7brkChgKdNwUaT4ao0wAhGpyLgoaAeMmeDrFeJj/ke+x hX+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=dQSwZ1KpQjnjrrZuc1mOd7AEMVB6u6WuYygkv+ai+Pw=; b=KT2LV/GmK3jYk5h/6a/OpTTcvOj1OURBG7QF66Thk2nlYSPFVM6irhkfWVNmEbOK4G YxzEUjTvEWRnjaJ2o25VivPzD8kG1s1SKChI7rVOlqBsapX1JwhhHAbuaD72v5O0KJss sgKip8ACBmsVxKUgyCPdfxlSrzvBVKvVHM0t2Sozm8FaalDon4Ged8QfCQUlcY8tttBc UqM8w1WKNT7oHR9USUDoTP64OzKeLS0JJzeK31qmSp5szqT/XTC2ZAakz8OIxHKTUnI4 oUcbO6juJxNEbIdyfPUGX1lUSXLlJvTwDPA5HBvh0FeAkTPMK57nVU4YZUfcgdy6MZAh 9ZjQ== X-Gm-Message-State: AOAM532YArBzCfh7C+1zZcygjLwOH7FVeSwPFGtW5WV9ck8Gzexx34Wo 4HdqoCSqyAnxvAe5tcU1+bU3R0o9lksw0B6YSk1guA== X-Google-Smtp-Source: ABdhPJzklWht4WBp9iSVRx7q0mrZIdC4dnHSymH0fwkRzOFWB4oy+Py3LmEHN/47PuPRXMDqUS90ypvPWMpFAylsVDE= X-Received: by 2002:ac8:7774:: with SMTP id h20mr7606695qtu.247.1589378158927; Wed, 13 May 2020 06:55:58 -0700 (PDT) MIME-Version: 1.0 References: <1589309971-12939-1-git-send-email-mw@semihalf.com> <1589309971-12939-2-git-send-email-mw@semihalf.com> <20200513134126.GQ21486@vanye> In-Reply-To: <20200513134126.GQ21486@vanye> From: "Marcin Wojtas" Date: Wed, 13 May 2020 15:55:47 +0200 Message-ID: Subject: Re: [edk2-platforms: PATCH 1/3] Marvell/Library: UtmiLib: update USB2.0 analog settings To: Leif Lindholm Cc: edk2-devel-groups-io , ard.biesheuvel@arm.com, "jsd@semihalf.com" , Grzegorz Jaszczyk , Kostya Porotchkin Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Leif, =C5=9Br., 13 maj 2020 o 15:41 Leif Lindholm napisa=C5= =82(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 detecto= r > > circuit and update host disconnect threshold value. > > * Update LS TX driver strength coarse and fine adjustment values. > > > > Signed-off-by: Grzegorz Jaszczyk > > Signed-off-by: Marcin Wojtas > > --- > > 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_C= TRL_IMPCAL_VTH_OFFSET) > > +#define UTMI_CALIB_CTRL_IMPCAL_START_OFFSET 13 > > +#define UTMI_CALIB_CTRL_IMPCAL_START_MASK (0x1 << UTMI_CALIB_C= TRL_IMPCAL_START_OFFSET) > > +#define UTMI_CALIB_CTRL_PLLCAL_START_OFFSET 22 > > +#define UTMI_CALIB_CTRL_PLLCAL_START_MASK (0x1 << UTMI_CALIB_C= TRL_PLLCAL_START_OFFSET) > > #define UTMI_CALIB_CTRL_IMPCAL_DONE_OFFSET 23 > > #define UTMI_CALIB_CTRL_IMPCAL_DONE_MASK (0x1 << UTMI_CALIB_C= TRL_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_C= TRL_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_C= TRL_IMP_SEL_LS_OFFSET) > > +#define UTMI_TX_CH_CTRL_AMP_OFFSET 20 > > +#define UTMI_TX_CH_CTRL_AMP_MASK (0x7 << UTMI_TX_CH_C= TRL_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_C= TRL0_DISCON_THRESH_OFFSET) > > #define UTMI_RX_CH_CTRL0_SQ_DET_OFFSET 15 > > #define UTMI_RX_CH_CTRL0_SQ_DET_MASK (0x1 << UTMI_RX_CH_C= TRL0_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_C= TRL1_SQ_AMP_CAL_OFFSET) > > +#define UTMI_RX_CH_CTRL1_SQ_AMP_CAL_MASK (0x7 << UTMI_RX_CH_C= TRL1_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_C= TRL1_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 =3D UTMI_CALIB_CTRL_IMPCAL_VTH_MASK; > > + Data =3D 0x7 << UTMI_CALIB_CTRL_IMPCAL_VTH_OFFSET; > > + RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask); > > + > > + /* Start Impedance and PLL Calibration */ > > + Mask =3D UTMI_CALIB_CTRL_PLLCAL_START_MASK; > > + Data =3D (0x1 << UTMI_CALIB_CTRL_PLLCAL_START_OFFSET); > > + Mask |=3D UTMI_CALIB_CTRL_IMPCAL_START_MASK; > > + Data |=3D (0x1 << UTMI_CALIB_CTRL_IMPCAL_START_OFFSET); > > + RegSet (UtmiBaseAddr + UTMI_CALIB_CTRL_REG, Data, Mask); > > > > /* Set LS TX driver strength coarse control */ > > - Mask =3D UTMI_TX_CH_CTRL_DRV_EN_LS_MASK; > > - Data =3D 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET; > > - /* Set LS TX driver fine adjustment */ > > + Mask =3D UTMI_TX_CH_CTRL_AMP_MASK; > > + Data =3D 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET; > > Mask |=3D UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK; > > Data |=3D 0x3 << UTMI_TX_CH_CTRL_IMP_SEL_LS_OFFSET; > > + Mask |=3D UTMI_TX_CH_CTRL_DRV_EN_LS_MASK; > > + Data |=3D 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 =3D UTMI_TX_CH_CTRL_DRV_EN_LS_MASK; > Data =3D 0x3 << UTMI_TX_CH_CTRL_DRV_EN_LS_OFFSET; > + Mask |=3D UTMI_TX_CH_CTRL_AMP_MASK; > + Data |=3D 0x4 << UTMI_TX_CH_CTRL_AMP_OFFSET; > /* Set LS TX driver fine adjustment */ > Mask |=3D UTMI_TX_CH_CTRL_IMP_SEL_LS_MASK; > Data |=3D 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 aspec= t. Best regards, Marcin > / > Leif > > > > > /* Enable SQ */ > > Mask =3D UTMI_RX_CH_CTRL0_SQ_DET_MASK; > > - Data =3D 0x0 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET; > > + Data =3D 0x1 << UTMI_RX_CH_CTRL0_SQ_DET_OFFSET; > > /* Enable analog squelch detect */ > > Mask |=3D UTMI_RX_CH_CTRL0_SQ_ANA_DTC_MASK; > > - Data |=3D 0x1 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET; > > + Data |=3D 0x0 << UTMI_RX_CH_CTRL0_SQ_ANA_DTC_OFFSET; > > + Mask |=3D UTMI_RX_CH_CTRL0_DISCON_THRESH_MASK; > > + Data |=3D 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 > >