From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::343; helo=mail-wm1-x343.google.com; envelope-from=haojian.zhuang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 091622117B57F for ; Sun, 21 Oct 2018 19:51:35 -0700 (PDT) Received: by mail-wm1-x343.google.com with SMTP id 193-v6so8662306wme.3 for ; Sun, 21 Oct 2018 19:51:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PS5lWSXHcqd6kjXbxVQ4I9wt53Qyrv0eXAhmmw/Fm9c=; b=icBcDtg5YeXgMWogubMSJtzU0kWrWSlyKbmghrPSQrekzVK4iPVYYgHgMqS0yYqTYF lP+rB/tQ3rb0QDzAoE7UlSbVjVk1egwVd8Th2jRrlGICWKp8ruerWQad7C5fWCiz/V+T uoRPpldVqfcWOw/SJ9Yz8RyPOKbDzNBS023UE= 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; bh=PS5lWSXHcqd6kjXbxVQ4I9wt53Qyrv0eXAhmmw/Fm9c=; b=l8TmwUt/+L8apO4bLGKrTcoar2IDFrU617C48gl1wMV1em8/oR3nA+5ggX7aY/rFwf 5dLFvvS747psBZrhTDFyJ4fJ/2qWFjoMvj01Rzw2F8iqCU9Ewrm3uN2FuE2Ex8llcUd6 qGRvyKwhsM9gQ7nagGExn2FrCEQNUsr9T/j0rUtqQwEerD1wDWdp+VkUM0Xfm/DxyQrW +jYDRn4cpzq5ce9vDa9jMVZsgW4IJZiq4vPXUe2Tnq3h7apOinpl2r3cm8byah6Ei6i0 Jei14LLc//ywEoUs1Kf6T2c8lu9yMn4e5+oISVzFkRH79FN2cuCAbCO18isB/Vbp4QF3 Qs+A== X-Gm-Message-State: ABuFfogYGopZ91h4Bt4K5/X216ZrYFdS20vy3EBJYbILMXS7rUQ35fCJ lgM/TWNBkUxP1nNWcLnxIB1H9SzGAqojh7MEk1Cn3Q== X-Google-Smtp-Source: ACcGV62hU2zkUtPvL/nWNv0SI92zUb3C+fWjUeat321HQu53s8GQ4p/GulzFPB0OsGYCBpuOdFZnAdMVPj7jXoEAUCA= X-Received: by 2002:a1c:f417:: with SMTP id z23-v6mr1917465wma.80.1540176693988; Sun, 21 Oct 2018 19:51:33 -0700 (PDT) MIME-Version: 1.0 References: <1534761085-26972-1-git-send-email-haojian.zhuang@linaro.org> <1534761085-26972-3-git-send-email-haojian.zhuang@linaro.org> <20181004163207.ydfxtrsdxaf6ifgf@bivouac.eciton.net> In-Reply-To: <20181004163207.ydfxtrsdxaf6ifgf@bivouac.eciton.net> From: Haojian Zhuang Date: Mon, 22 Oct 2018 10:51:22 +0800 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Ard Biesheuvel Subject: Re: [PATCH v1 2/2] EmbeddedPkg/Drivers: add DwUsb3Dxe driver X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Oct 2018 02:51:36 -0000 Content-Type: text/plain; charset="UTF-8" On Fri, 5 Oct 2018 at 00:32, Leif Lindholm wrote: > > On Mon, Aug 20, 2018 at 06:31:25PM +0800, Haojian Zhuang wrote: > > Add Designware USB 3.0 device driver. It's focus on USB device > > functionality, not USB Host functionality. The USB driver is > > mainly used for Android Fastboot application. > > > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Haojian Zhuang > > --- > > EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.dec | 44 + > > EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.inf | 52 + > > EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.h | 632 +++++ > > EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.c | 2434 ++++++++++++++++++++ > > 4 files changed, 3162 insertions(+) > > > > diff --git a/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.dec b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.dec > > new file mode 100644 > > index 000000000000..038e4881a948 > > --- /dev/null > > +++ b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.dec > > @@ -0,0 +1,44 @@ > > +#/** @file > > +# Framework Module Development Environment Industry Standards > > Please change this to reflect this package rather than EmbeddedPkg one > (which appears to have stolen it from an old version of MdePkg.dec :) > OK > > +# > > +# This Package provides headers and libraries that conform to EFI/PI Industry standards. > > +# Copyright (c) 2007, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2012-2014, ARM Ltd. All rights reserved.
> > I see no need to keep these copyright statements. > OK > > +# Copyright (c) 2018, Linaro. All rights reserved.
> > +# > > +# This program and the accompanying materials are licensed and made available under > > +# the terms and conditions of the BSD License which accompanies this distribution. > > +# The full text of the license may be found at > > +# http://opensource.org/licenses/bsd-license.php > > +# > > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > +# > > +#**/ > > + > > +[Defines] > > + DEC_SPECIFICATION = 0x00010019 > > + PACKAGE_NAME = DwUsb3DxePkg > > + PACKAGE_GUID = 9b7aa6fe-405b-4955-af1f-5faf183aedec > > + PACKAGE_VERSION = 0.1 > > + > > + > > +################################################################################ > > +# > > +# Include Section - list of Include Paths that are provided by this package. > > +# Comments are used for Keywords and Module Types. > > +# > > +# Supported Module Types: > > +# BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION > > +# > > +################################################################################ > > + > > +[Guids.common] > > + gDwUsb3DxeTokenSpaceGuid = { 0x098a50d3, 0x92e2, 0x461a, { 0xb6, 0xba, 0xf8, 0x5d, 0x9d, 0x89, 0x49, 0xf5 }} > > + > > +[Protocols.common] > > + gDwUsbProtocolGuid = { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }} > > Please make this file a common .dec for DwUsb and DwUsb3 instead of > duplicating guid information. > (If we weren't moving it to edk2-platforms, it would still have been > better to add it to EmbeddedPkg.dec.) > OK > > + > > +[PcdsFixedAtBuild.common] > > + # DwUsb Driver PCDs > > + gDwUsb3DxeTokenSpaceGuid.PcdDwUsb3DxeBaseAddress|0x0|UINT32|0x00000001 > > diff --git a/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.inf b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.inf > > new file mode 100644 > > index 000000000000..510b51a34de7 > > --- /dev/null > > +++ b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.inf > > @@ -0,0 +1,52 @@ > > +#/** @file > > +# > > +# Copyright (c) 2018, Linaro Limited. All rights reserved. > > +# > > +# This program and the accompanying materials > > +# are licensed and made available under the terms and conditions of the BSD License > > +# which accompanies this distribution. The full text of the license may be found at > > +# http://opensource.org/licenses/bsd-license.php > > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > +# > > +# > > +#**/ > > + > > +[Defines] > > + INF_VERSION = 0x00010019 > > + BASE_NAME = DwUsb3Dxe > > + FILE_GUID = 0879cd34-c399-4315-9891-56024072e711 > > + MODULE_TYPE = UEFI_DRIVER > > + VERSION_STRING = 1.0 > > + ENTRY_POINT = DwUsb3EntryPoint > > + > > +[Sources.common] > > + DwUsb3Dxe.c > > + > > +[LibraryClasses] > > + CacheMaintenanceLib > > + DebugLib > > + DmaLib > > + IoLib > > + MemoryAllocationLib > > + TimerLib > > + UefiBootServicesTableLib > > + UefiDriverEntryPoint > > + UsbSerialNumberLib > > I don't see this library anywhere in the tree? > I'll add it into the patch series for the next revision. > > + > > +[Protocols] > > + gEfiDriverBindingProtocolGuid > > + gUsbDeviceProtocolGuid > > + > > +[Packages] > > + ArmPkg/ArmPkg.dec > > + EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.dec > > + EmbeddedPkg/EmbeddedPkg.dec > > + MdePkg/MdePkg.dec > > + MdeModulePkg/MdeModulePkg.dec > > + > > +[Pcd] > > + gDwUsb3DxeTokenSpaceGuid.PcdDwUsb3DxeBaseAddress > > + > > +[Depex] > > + TRUE > > diff --git a/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.h b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.h > > new file mode 100644 > > index 000000000000..92b610553169 > > --- /dev/null > > +++ b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.h > > @@ -0,0 +1,632 @@ > > +/** @file > > + > > + Copyright (c) 2017, Linaro Limited. All rights reserved. > > + > > + This program and the accompanying materials > > + are licensed and made available under the terms and conditions of the BSD License > > + which accompanies this distribution. The full text of the license may be found at > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > + > > +**/ > > + > > +#ifndef __DW_USB3_DXE_H__ > > +#define __DW_USB3_DXE_H__ > > DW -> DESIGNWARE (throughout). > OK > > + > > +#define DW_USB3_BASE FixedPcdGet32 (PcdDwUsb3DxeBaseAddress) > > + > > +#define GSBUCFG0 (DW_USB3_BASE + 0xC100) > > +#define GCTL (DW_USB3_BASE + 0xC110) > > + > > +#define GCTL_PWRDNSCALE_MASK (0x1FFF << 19) > > +#define GCTL_PWRDNSCALE(x) (((x) & 0x1FFF) << 19) > > +#define GCTL_U2RSTECN BIT16 > > +#define GCTL_PRTCAPDIR_MASK (BIT13 | BIT12) > > +#define GCTL_PRTCAPDIR_HOST BIT12 > > +#define GCTL_PRTCAPDIR_DEVICE BIT13 > > +#define GCTL_PRTCAPDIR_OTG (BIT13 | BIT12) > > +#define GCTL_U2EXIT_LFPS BIT2 > > + > > +#define GUSB2PHYCFG(x) (DW_USB3_BASE + 0xC200 + (((x) & 0xF) << 2)) > > + > > +#define GUSB2PHYCFG_USBTRDTIM_MASK (0xF << 10) > > +#define GUSB2PHYCFG_USBTRDTIM(x) (((x) & 0xF) << 10) > > +#define GUSB2PHYCFG_SUSPHY BIT6 > > + > > +#define GUSB3PIPECTL(x) (DW_USB3_BASE + 0xC2C0 + (((x) & 0x3) << 2)) > > + > > +#define PIPECTL_DELAYP1TRANS BIT18 > > +#define PIPECTL_SUSPEND_EN BIT17 > > +#define PIPECTL_LFPS_FILTER BIT9 > > +#define PIPECTL_TX_DEMPH_MASK (0x3 << 1) > > +#define PIPECTL_TX_DEMPH(x) (((x) & 0x3) << 1) > > + > > +#define GTXFIFOSIZ(x) (DW_USB3_BASE + 0xC300 + (((x) & 0x1F) << 2)) > > +#define GRXFIFOSIZ(x) (DW_USB3_BASE + 0xC380 + (((x) & 0x1F) << 2)) > > + > > +#define FIFOSIZ_ADDR(x) (((x) & 0xFFFF) << 16) > > +#define FIFOSIZ_DEP(x) ((x) & 0xFFFF) > > + > > +#define GEVNTADRL(x) (DW_USB3_BASE + 0xC400 + (((x) & 0x1F) << 2)) > > +#define GEVNTADRH(x) (DW_USB3_BASE + 0xC404 + (((x) & 0x1F) << 2)) > > +#define GEVNTSIZ(x) (DW_USB3_BASE + 0xC408 + (((x) & 0x1F) << 2)) > > + > > +#define GEVNTSIZ_EVNTINTMASK BIT31 > > +#define GEVNTSIZ_EVNTSIZ_MASK (0xFFFF) > > +#define GEVNTSIZ_EVNTSIZ(x) ((x) & 0xFFFF) > > + > > +#define GEVNTCOUNT(x) (DW_USB3_BASE + 0xC40C + (((x) & 0x1F) << 2)) > > +#define GEVNTCOUNT_EVNTCOUNT_MASK (0xFFFF) > > +#define GEVNTCOUNT_EVNTCOUNT(x) ((x) & 0xFFFF) > > + > > +// Non-Endpoint specific event flag > > +#define GEVNT_INTTYPE_MASK (0x7F << 1) > > +#define GEVNT_INTTYPE(x) (((x) & 0x7F) << 1) > > +#define EVENT_I2C_INT 4 > > +#define EVENT_CARKIT_INT 3 > > +#define EVENT_OTG_INT 1 > > +#define EVENT_DEV_INT 0 > > + > > +#define GEVNT_NON_EP BIT0 > > +// Endpoint specific event flag > > +#define GEVNT_DEPEVT_INTTYPE_MASK (0xF << 6) > > +#define GEVNT_DEPEVT_INTTYPE(x) (((x) & 0xF) << 6) > > +#define GEVNT_DEPEVT_INTTYPE_EPCMD_CMPL (7 << 6) > > +#define GEVNT_DEPEVT_INTTYPE_STRM_EVT (6 << 6) > > +#define GEVNT_DEPEVT_INTTYPE_FIFOXRUN (4 << 6) > > +#define GEVNT_DEPEVT_INTTYPE_XFER_NRDY (3 << 6) > > +#define GEVNT_DEPEVT_INTTYPE_XFER_IN_PROG (2 << 6) > > +#define GEVNT_DEPEVT_INTTYPE_XFER_CMPL (1 << 6) > > +#define GEVNT_DEPEVT_EPNUM_MASK (0x1F << 1) > > +#define GEVNT_DEPEVT_EPNUM_SHIFT 1 > > +#define GEVNT_DEPEVT_EPNUM(x) (((x) & 0x1F) << 1) > > +// Devices specific event flag > > +#define GEVNT_DEVT_MASK (0xF << 8) > > +#define GEVNT_DEVT_SHIFT 8 > > +#define GEVNT_DEVT(x) (((x) & 0xF) << 8) > > +#define GEVNT_DEVT_INACT_TIMEOUT_RCVD (0x15 << 8) > > +#define GEVNT_DEVT_VNDR_DEV_TST_RCVD (0x14 << 8) > > +#define GEVNT_DEVT_OVERFLOW (0x13 << 8) > > +#define GEVNT_DEVT_CMD_CMPL (0x12 << 8) > > +#define GEVNT_DEVT_ERRATICERR (0x11 << 8) > > +#define GEVNT_DEVT_SOF (0x7 << 8) > > +#define GEVNT_DEVT_EOPF (0x6 << 8) > > +#define GEVNT_DEVT_HIBER_REQ (0x5 << 8) > > +#define GEVNT_DEVT_WKUP (0x4 << 8) > > +#define GEVNT_DEVT_ULST_CHNG (0x3 << 8) > > +#define GEVNT_DEVT_CONNDONE (0x2 << 8) > > +#define GEVNT_DEVT_USBRESET (0x1 << 8) > > +#define GEVNT_DEVT_DISCONN (0x0 << 8) > > + > > +#define DCFG (DW_USB3_BASE + 0xC700) > > + > > +#define DCFG_NUMP_MASK (0x1F << 17) > > +#define DCFG_NUMP(x) (((x) & 0x1F) << 17) > > +#define DCFG_DEVADDR_MASK (0x7F << 3) > > +#define DCFG_DEVADDR(x) (((x) & 0x7F) << 3) > > +#define DCFG_DEVSPD_MASK (0x7) > > +#define DCFG_DEVSPD(x) ((x) & 0x7) > > +#define DEVSPD_HS_PHY_30MHZ_OR_60MHZ 0 > > +#define DEVSPD_FS_PHY_30MHZ_OR_60MHZ 1 > > +#define DEVSPD_LS_PHY_6MHZ 2 > > +#define DEVSPD_FS_PHY_48MHZ 3 > > +#define DEVSPD_SS_PHY_125MHZ_OR_250MHZ 4 > > + > > +#define DCTL (DW_USB3_BASE + 0xC704) > > + > > +#define DCTL_RUN_STOP BIT31 > > +#define DCTL_CSFTRST BIT30 > > +#define DCTL_INIT_U2_EN BIT12 > > +#define DCTL_ACCEPT_U2_EN BIT11 > > +#define DCTL_INIT_U1_EN BIT10 > > +#define DCTL_ACCEPT_U1_EN BIT9 > > + > > +#define DEVTEN (DW_USB3_BASE + 0xC708) > > +#define DEVTEN_CONNECTDONEEN BIT2 > > +#define DEVTEN_USBRSTEN BIT1 > > +#define DEVTEN_DISCONNEN BIT0 > > + > > +#define DSTS (DW_USB3_BASE + 0xC70C) > > +#define DSTS_GET_DEVSPD(x) ((x) & 0x7) > > + > > +#define DALEPENA (DW_USB3_BASE + 0xC720) > > + > > +#define DEPCMDPAR2(x) (DW_USB3_BASE + 0xC800 + ((x) & 0x1F) * 0x10) > > +#define DEPCMDPAR1(x) (DW_USB3_BASE + 0xC804 + ((x) & 0x1F) * 0x10) > > +#define DEPCMDPAR0(x) (DW_USB3_BASE + 0xC808 + ((x) & 0x1F) * 0x10) > > +#define DEPCMD(x) (DW_USB3_BASE + 0xc80C + ((x) & 0x1F) * 0x10) > > + > > +#define DEPCMD_COMMANDPARAM_MASK (0xFFFF << 16) > > +#define DEPCMD_COMMANDPARAM(x) (((x) & 0xFFFF) << 16) > > +/* Stream Number or uFrame (input) */ > > +#define DEPCMD_STR_NUM_OR_UF_MASK (0xFFFF << 16) > > +#define DEPCMD_STR_NUM_OR_UF(x) (((x) & 0xFFFF) << 16) > > +/* Transfer Resource Index (output) */ > > +#define DEPCMD_XFER_RSRC_IDX_SHIFT 16 > > +#define DEPCMD_XFER_RSRC_IDX_MASK (0x7F << 16) > > +#define DEPCMD_XFER_RSRC_IDX(x) (((x) & 0x7F) << 16) > > +#define GET_DEPCMD_XFER_RSRC_IDX(x) (((x) >> 16) & 0x7F) > > +#define DEPCMD_CMDACT BIT10 > > +#define DEPCMD_CMDTYPE_MASK 0xFF > > +#define DEPCMD_CMDTYPE(x) ((x) & 0xFF) > > + > > +/* EP registers range as: OUT0, IN0, OUT1, IN1, ... */ > > +#define EP_OUT_IDX(x) ((x) * 2) > > +#define EP_IN_IDX(x) (((x) * 2) + 1) > > + > > +#define CMDTYPE_SET_EP_CFG 1 > > +#define CMDTYPE_SET_XFER_CFG 2 > > +#define CMDTYPE_GET_EP_STATE 3 > > +#define CMDTYPE_SET_STALL 4 > > +#define CMDTYPE_CLR_STALL 5 > > +#define CMDTYPE_START_XFER 6 > > +#define CMDTYPE_UPDATE_XFER 7 > > +#define CMDTYPE_END_XFER 8 > > +#define CMDTYPE_START_NEW_CFG 9 > > + > > +#define EPTYPE_CONTROL 0 > > +#define EPTYPE_ISOC 1 > > +#define EPTYPE_BULK 2 > > +#define EPTYPE_INTR 3 > > + > > +#define CFG_ACTION_INIT 0 > > +#define CFG_ACTION_RESTORE 1 > > +#define CFG_ACTION_MODIFY 2 > > + > > +#define EPCFG0_CFG_ACTION_MASK (0x3 << 30) > > +#define EPCFG0_CFG_ACTION(x) (((x) & 0x3) << 30) > > +#define EPCFG0_BRSTSIZ_MASK (0xF << 22) > > +#define EPCFG0_BRSTSIZ(x) (((x) & 0xF) << 22) > > +#define EPCFG0_TXFNUM_MASK (0x1F << 17) > > +#define EPCFG0_TXFNUM(x) (((x) & 0x1F) << 17) > > +#define EPCFG0_MPS_MASK (0x7FF << 3) > > +#define EPCFG0_MPS(x) (((x) & 0x7FF) << 3) > > +#define EPCFG0_EPTYPE_MASK (0x3 << 1) > > +#define EPCFG0_EPTYPE_SHIFT 1 > > +#define EPCFG0_EPTYPE(x) (((x) & 0x3) << 1) > > + > > +/* Endpoint Number */ > > +#define EPCFG1_EP_NUM_MASK (0xF << 26) > > +#define EPCFG1_EP_NUM(x) (((x) & 0xF) << 26) > > +/* Endpoint Direction */ > > +#define EPCFG1_EP_DIR_IN BIT25 > > +/* Stream Not Ready */ > > +#define EPCFG1_XFER_NRDY BIT10 > > +/* XferInProgress Enable */ > > +#define EPCFG1_XFER_IN_PROG BIT9 > > +/* Stream Completed */ > > +#define EPCFG1_XFER_CMPL BIT8 > > + > > +#define USB_SPEED_UNKNOWN 0 > > +#define USB_SPEED_LOW 1 > > +#define USB_SPEED_FULL 2 > > +#define USB_SPEED_HIGH 3 > > +#define USB_SPEED_VARIABLE 4 > > +#define USB_SPEED_SUPER 5 > > + > > +// DMA registers > > +#define DSCSTS_TRBRSP_MASK (0xF << 28) > > +#define DSCSTS_TRBRSP(x) (((x) & 0xF) << 28) > > +#define GET_DSCSTS_TRBRSP(x) (((x) >> 28) & 0xF) > > +#define TRBRSP_MISSED_ISOC_IN 1 > > +#define TRBRSP_SETUP_PEND 2 > > +#define TRBRSP_XFER_IN_PROG 4 > > +#define DSCSTS_PCM1_MASK (0x3 << 24) > > +#define DSCSTS_PCM1(x) (((x) & 0x3) << 24) > > +#define DSCSTS_XFERCNT_MASK 0xFFFFFF > > +#define DSCSTS_XFERCNT(x) ((x) & 0xFFFFFF) > > +#define GET_DSCSTS_XFERCNT(x) ((x) & 0xFFFFFF) > > + > > +#define DSCCTL_STRMID_SOFN(x) (((x) & 0xFFFF) << 14) > > +#define DSCCTL_IOC BIT11 > > +#define DSCCTL_ISP BIT10 > > +#define DSCCTL_TRBCTL_MASK (0x3F << 4) > > +#define DSCCTL_TRBCTL(x) (((x) & 0x3F) << 4) > > +#define DSCCTL_LST BIT1 > > +#define DSCCTL_HWO BIT0 > > +#define TRBCTL_NORMAL 1 > > +#define TRBCTL_SETUP 2 > > +#define TRBCTL_STATUS_2 3 > > +#define TRBCTL_STATUS_3 4 > > +#define TRBCTL_CTLDATA_1ST 5 > > +#define TRBCTL_ISOC_1ST 6 > > +#define TRBCTL_ISOC 7 > > +#define TRBCTL_LINK 8 > > +#define TRBCTL_NORMAL_ZLP 9 > > + > > + > > +#define UE_DIR_IN 0x80 > > +#define UE_DIR_OUT 0 > > +#define UE_SET_DIR(a, d) ((a) | (((d) & 1) << 7)) > > +#define UE_GET_DIR(a) ((a) & 0x80) > > +#define UE_ADDR 0x0F > > +#define UE_GET_ADDR(a) ((a) & UE_ADDR) > > + > > +#define UT_GET_DIR(a) ((a) & 0x80) > > +#define UT_WRITE 0x00 > > +#define UT_READ 0x80 > > + > > +#define UT_GET_TYPE(a) ((a) & 0x60) > > +#define UT_STANDARD 0x00 > > +#define UT_CLASS 0x20 > > +#define UT_VENDOR 0x40 > > + > > +#define UT_GET_RECIPIENT(a) ((a) & 0x1f) > > +#define UT_DEVICE 0x00 > > +#define UT_INTERFACE 0x01 > > +#define UT_ENDPOINT 0x02 > > +#define UT_OTHER 0x03 > > + > > +#define UR_GET_STATUS 0x00 > > +#define UR_CLEAR_FEATURE 0x01 > > +#define UR_SET_FEATURE 0x03 > > +#define UR_SET_ADDRESS 0x05 > > +#define UR_GET_DESCRIPTOR 0x06 > > +#define UR_SET_DESCRIPTOR 0x07 > > +#define UR_GET_CONFIG 0x08 > > +#define UR_SET_CONFIG 0x09 > > +#define UR_GET_INTERFACE 0x0A > > +#define UR_SET_INTERFACE 0x0B > > +#define UR_SYNCH_FRAME 0x0C > > +#define UR_SET_SEL 0x30 > > +#define UR_SET_ISOC_DELAY 0x31 > > + > > Somewhere around here starts a bunch of redefinitions of things > already described in MdePkg/Include/IndustryStandard/Usb.h. > Can you please reuse that include file as much as possible. > OK > > +/* Feature numbers */ > > +#define UF_ENDPOINT_HALT 0 > > +#define UF_DEVICE_REMOTE_WAKEUP 1 > > +#define UF_TEST_MODE 2 > > +#define UF_DEVICE_B_HNP_ENABLE 3 > > +#define UF_DEVICE_A_HNP_SUPPORT 4 > > +#define UF_DEVICE_A_ALT_HNP_SUPPORT 5 > > +#define UF_FUNCTION_SUSPEND 0 > > +#define UF_U1_ENABLE 48 > > +#define UF_U2_ENABLE 49 > > +#define UF_LTM_ENABLE 50 > > + > > +#define UDESC_DEVICE 0x01 > > +#define UDESC_CONFIG 0x02 > > +#define UDESC_STRING 0x03 > > +#define UDESC_INTERFACE 0x04 > > +#define UDESC_ENDPOINT 0x05 > > +#define UDESC_SS_USB_COMPANION 0x30 > > +#define UDESC_DEVICE_QUALIFIER 0x06 > > +#define UDESC_BOS 0x0f > > +#define UDESC_DEVICE_CAPABILITY 0x10 > > + > > +#define STRING_LANGUAGE 0 > > +#define STRING_MANUFACTURER 1 > > +#define STRING_PRODUCT 2 > > +#define STRING_SERIAL 3 > > + > > +#define CONFIG_VALUE 1 > > + > > +#define USB3_BULK_IN_EP 1 > > +#define USB3_BULK_OUT_EP 1 > > + > > +#define USB_ENUM_ADB_PORT_VID 0x18D1 > > +#define USB_ENUM_ADB_PORT_PID 0xD00D > > +#define USB_ENUM_INTERFACE_ADB_SUBCLASS 0x42 > > +#define USB_ENUM_INTERFACE_ADB_PROTOCOL 0x03 > > + > > +struct usb3_pcd; > > + > > +typedef enum pcd_state { > > + USB3_STATE_UNCONNECTED, /* no host */ > > + USB3_STATE_DEFAULT, > > + USB3_STATE_ADDRESSED, > > + USB3_STATE_CONFIGURED, > > +} pcdstate_e; > > These structs also break the coding style completely. > Will using IndustryStandard/Usb.h get rid of all of that? > OK > > + > > +typedef enum ep0_state { > > + EP0_IDLE, > > + EP0_IN_DATA_PHASE, > > + EP0_OUT_DATA_PHASE, > > + EP0_IN_WAIT_NRDY, > > + EP0_OUT_WAIT_NRDY, > > + EP0_IN_STATUS_PHASE, > > + EP0_OUT_STATUS_PHASE, > > + EP0_STALL, > > +} ep0state_e; > > + > > +typedef struct usb3_dma_desc { > > + /** Buffer Pointer - Low address quadlet */ > > + UINT32 bptl; > > + > > + /** Buffer Pointer - High address quadlet */ > > + UINT32 bpth; > > + > > + /** Status quadlet. Fields defined in enum @ref desc_sts_data. */ > > + UINT32 status; > > + > > + /** Control quadlet. Fields defined in enum @ref desc_ctl_data. */ > > + UINT32 control; > > +} usb3_dma_desc_t; > > + > > +typedef struct usb3_pcd_req { > > + usb3_dma_desc_t *trb; > > + UINT64 trbdma; > > + > > + UINT32 length; > > + UINT32 actual; > > + > > + UINT64 *bufdma; > > + int (*complete)(unsigned actual, int status); > > +} usb3_pcd_req_t; > > + > > +typedef struct usb_device_request { > > + UINT8 bmRequestType; > > + UINT8 bRequest; > > + UINT16 wValue; > > + UINT16 wIndex; > > + UINT16 wLength; > > +} usb_device_request_t; > > + > > +#pragma pack(1) > > +/** USB_DT_DEVICE: Device descriptor */ > > +typedef struct usb_device_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT16 bcdUSB; > > +#define USB_CLASS_COMM 0x02 > > +#define USB_CLASS_VENDOR_SPEC 0xFF > > +#define USB_SC_VENDOR_SPEC 0xFF > > +#define USB_PR_VENDOR_SPEC 0xFF > > + UINT8 bDeviceClass; > > + UINT8 bDeviceSubClass; > > + UINT8 bDeviceProtocol; > > + UINT8 bMaxPacketSize0; > > + UINT16 idVendor; > > + UINT16 idProduct; > > + UINT16 bcdDevice; > > + UINT8 iManufacturer; > > + UINT8 iProduct; > > + UINT8 iSerialNumber; > > + UINT8 bNumConfigurations; > > +} usb_device_descriptor_t; > > + > > +/* USB_DT_CONFIG: Config descriptor */ > > +typedef struct usb_config_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT16 wTotalLength; > > + UINT8 bNumInterfaces; > > +#define CONFIG_VALUE 1 > > + UINT8 bConfigurationValue; > > + UINT8 iConfiguration; > > +#define USB_CONFIG_ATT_ONE (1 << 7) > > + UINT8 bmAttributes; > > +#define USB_CONFIG_VBUS_DRAW (0xFA) > > + UINT8 bMaxPower; > > +} usb_config_descriptor_t; > > + > > +/* USB_DT_DEVICE_QUALIFIER: Device Qualifier descriptor */ > > +typedef struct usb_qualifier_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT16 bcdUSB; > > + UINT8 bDeviceClass; > > + UINT8 bDeviceSubClass; > > + UINT8 bDeviceProtocol; > > + UINT8 bMaxPacketSize0; > > + UINT8 bNumConfigurations; > > + UINT8 bRESERVED; > > +} usb_qualifier_descriptor_t; > > + > > +/* USB_DT_INTERFACE: Interface descriptor */ > > +typedef struct usb_interface_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT8 bInterfaceNumber; > > + UINT8 bAlternateSetting; > > + UINT8 bNumEndpoints; > > + UINT8 bInterfaceClass; > > + UINT8 bInterfaceSubClass; > > + UINT8 bInterfaceProtocol; > > + UINT8 iInterface; > > +} usb_interface_descriptor_t; > > + > > +/* USB_DT_ENDPOINT: Endpoint descriptor */ > > +typedef struct usb_endpoint_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT8 bEndpointAddress; > > + UINT8 bmAttributes; > > +#define USB_ENDPOINT_XFER_CONTROL 0x00 > > +#define USB_ENDPOINT_XFER_ISOC 0x01 > > +#define USB_ENDPOINT_XFER_BULK 0x02 > > +#define USB_ENDPOINT_XFER_INT 0x03 > > + UINT16 wMaxPacketSize; > > + UINT8 bInterval; > > +} usb_endpoint_descriptor_t; > > + > > +/* USB_DT_SS_ENDPOINT_COMP: SuperSpeed Endpoint Companion descriptor */ > > +typedef struct usb_ss_ep_comp_descriptor { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + > > + UINT8 bMaxBurst; > > + UINT8 bmAttributes; > > + UINT16 wBytesPerInterval; > > +} usb_ss_ep_comp_descriptor_t; > > + > > +/* WUSB BOS Descriptor (Binary device Object Store) */ > > +typedef struct wusb_bos_desc { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + UINT16 wTotalLength; > > + UINT8 bNumDeviceCaps; > > +} wusb_bos_desc_t; > > + > > +#define USB_DEVICE_CAPABILITY_20_EXTENSION 0x02 > > +typedef struct usb_dev_cap_20_ext_desc { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + UINT8 bDevCapabilityType; > > +#define USB_20_EXT_LPM 0x02 > > + UINT32 bmAttributes; > > +} usb_dev_cap_20_ext_desc_t; > > + > > +#define USB_DEVICE_CAPABILITY_SS_USB 0x03 > > +typedef struct usb_dev_cap_ss_usb { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + UINT8 bDevCapabilityType; > > +#define USB_DC_SS_USB_LTM_CAPABLE 0x02 > > + UINT8 bmAttributes; > > +#define USB_DC_SS_USB_SPEED_SUPPORT_LOW 0x01 > > +#define USB_DC_SS_USB_SPEED_SUPPORT_FULL 0x02 > > +#define USB_DC_SS_USB_SPEED_SUPPORT_HIGH 0x04 > > +#define USB_DC_SS_USB_SPEED_SUPPORT_SS 0x08 > > + UINT32 wSpeedsSupported; > > + UINT8 bFunctionalitySupport; > > + UINT8 bU1DevExitLat; > > + UINT32 wU2DevExitLat; > > +} usb_dev_cap_ss_usb_t; > > + > > +#define USB_DEVICE_CAPABILITY_CONTAINER_ID 0x04 > > +typedef struct usb_dev_cap_container_id { > > + UINT8 bLength; > > + UINT8 bDescriptorType; > > + UINT8 bDevCapabilityType; > > + UINT8 bReserved; > > + UINT8 containerID[16]; > > +} usb_dev_cap_container_id_t; > > +#pragma pack() > > + > > +typedef union usb_setup_pkt { > > + usb_device_request_t req; > > + UINT32 d32[2]; > > + UINT8 d8[8]; > > +} usb_setup_pkt_t; > > + > > +typedef struct usb3_pcd_ep { > > + struct usb3_pcd *pcd; > > + > > + UINT8 EpInIdx; > > + UINT8 EpOutIdx; > > + UINT8 phys; > > + > > + //UINT8 phys; > > + UINT8 num; > > + UINT8 type; > > + UINT8 maxburst; > > + UINT16 maxpacket; > > + /* Tx FIFO # for IN EPs */ > > + UINT8 tx_fifo_num; > > + > > + /* The Transfer Resource Index from the Start Transfer command */ > > + UINT8 tri_out; > > + UINT8 tri_in; > > + > > + UINT8 stopped; > > + /* Send ZLP */ > > + UINT8 send_zlp; > > + /* True if 3-stage control transfer */ > > + UINT8 three_stage; > > + /* True if transfer has been started on EP */ > > + UINT8 xfer_started; > > + /* EP direction 0 = OUT */ > > + UINT8 is_in; > > + /* True if endpoint is active */ > > + UINT8 active; > > + /* Initial data pid of bulk endpoint */ > > + UINT8 data_pid_start; > > + > > + /* ep_desc (excluding ep0) */ > > + usb3_dma_desc_t *ep_desc; > > + > > +#if 0 > > + /* TRB descriptor must be aligned to 16 bytes */ > > + UINT8 epx_desc[32]; > > +#endif > > + > > + /* request (excluding ep0) */ > > + usb3_pcd_req_t req; > > +} usb3_pcd_ep_t; > > + > > +typedef struct usb3_pcd { > > + //struct usb3_device *usb3_dev; > > + > > + INT32 link_state; > > + pcdstate_e state; > > + UINT8 new_config; > > + ep0state_e ep0state; > > + > > + UINT32 eps_enabled; > > + UINT32 ltm_enable; > > + > > + usb3_pcd_ep_t ep0; > > + usb3_pcd_ep_t out_ep; > > + usb3_pcd_ep_t in_ep; > > + > > + /* > > + usb3_dev_global_regs_t *dev_global_regs; > > + usb3_dev_ep_regs_t *out_ep_regs; > > + usb3_dev_ep_regs_t *in_ep_regs; > > + */ > > + > > + usb3_pcd_req_t ep0_req; > > + > > + UINT8 speed; > > + > > + usb3_dma_desc_t *ep0_setup_desc; > > + usb3_dma_desc_t *ep0_in_desc; > > + usb3_dma_desc_t *ep0_out_desc; > > + > > + /* TRB descriptor must be aligned to 16 bytes */ > > +#if 0 > > + UINT8 ep0_setup[32]; > > + UINT8 ep0_in[32]; > > + UINT8 ep0_out[32]; > > + > > + usb_setup_pkt_t ep0_setup_pkt[5]; > > + > > +#define USB3_STATUS_BUF_SIZE 512 > > + UINT8 ep0_status_buf[USB3_STATUS_BUF_SIZE]; > > + > > +#define USB3_BULK_BUF_SIZE 2048 > > + UINT8 ss_bulk_buf[USB3_BULK_BUF_SIZE]; > > +#endif > > + > > + UINT32 file_type; > > + UINT32 file_address; > > + UINT32 file_capacity; > > + UINT32 file_total_frame; > > + UINT32 file_curr_frame; > > + UINT32 file_next_frame; > > + UINT32 file_received; > > + UINT32 file_complete; > > + > > + UINT16 test_mode_nr; > > + UINT16 test_mode; > > +} usb3_pcd_t; > > + > > +struct usb_enum_port_param { > > + UINT16 idVendor; > > + UINT16 idProduct; > > + UINT8 bInterfaceSubClass; > > + UINT8 bInterfaceProtocol; > > +}; > > + > > +#if 0 > > If not used, leave it out. > OK > > +typedef struct usb3_pcd_req { > > + usb3_dma_desc_t *trb; > > + UINT64 trbdma; > > + > > + UINT32 length; > > + UINT32 actual; > > + > > + UINT64 *bufdma; > > + int (*complete)(unsigned actual, int status); > > +} usb3_pcd_req_t; > > + > > +#endif > > + > > +#endif /* __DW_USB3_DXE_H__ */ > > diff --git a/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.c b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.c > > new file mode 100644 > > index 000000000000..83d5e4736de0 > > --- /dev/null > > +++ b/EmbeddedPkg/Drivers/DwUsb3Dxe/DwUsb3Dxe.c > > @@ -0,0 +1,2434 @@ > > +/** @file > > + > > + Copyright (c) 2018, Linaro Limited. All rights reserved. > > + > > + This program and the accompanying materials are licensed and made available > > + under the terms and conditions of the BSD License which accompanies this > > + distribution. The full text of the license may be found at > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > + > > +**/ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "DwUsb3Dxe.h" > > + > > +#define FIFO_DIR_TX 0 > > +#define FIFO_DIR_RX 1 > > + > > +#define TX_FIFO_ADDR 0 > > +#define RX_FIFO_ADDR 0 > > + > > +#define RAM_WIDTH 8 > > +#define RAM_TX0_DEPTH 2048 > > +#define RAM_TX1_DEPTH 4096 > > +#define RAM_RX_DEPTH 8192 > > Should these not also go into DwUsb3Dxe.h? > OK > > + > > +#define USB_TYPE_LENGTH 16 > > +#define USB_BLOCK_HIGH_SPEED_SIZE 512 > > +#define DATA_SIZE 131072 > > +#define CMD_SIZE 512 > > What do the above definitions mean? > I'll add comment. > > +#define MATCH_CMD_LITERAL(Cmd, Buf) !AsciiStrnCmp (Cmd, Buf, sizeof (Cmd) - 1) > > This one does not appear to be used? > OK > > + > > +#define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1)) > > Use available macros in Base.h. > OK > > + > > +// > > +// The time between interrupt polls, in units of 100 nanoseconds > > +// 10 Microseconds > > +// > > +#define DW_INTERRUPT_POLL_PERIOD 100 > > + > > +#define DWUSB3_EVENT_BUF_SIZE 256 > > + > > +// > > +// Maxpacket size for EP0, defined by USB3 spec > > +// > > +#define USB3_MAX_EP0_SIZE 512 > > + > > +// > > +// Maxpacket size for any EP, defined by USB3 spec > > +// > > +#define USB3_MAX_PACKET_SIZE 1024 > > +#define USB2_HS_MAX_PACKET_SIZE 512 > > +#define USB2_FS_MAX_PACKET_SIZE 64 > > + > > +#define USB3_STATE_UNCONNECTED 0 > > +#define USB3_STATE_DEFAULT 1 > > +#define USB3_STATE_ADDRESSED 2 > > +#define USB3_STATE_CONFIGURED 3 > > + > > +#define USB3_STATUS_BUF_SIZE 512 > > If these are defined by USB3 spec, please add them to > MdePkg/Include/IndustryStandard/Usb.h instead of defining them > locally. > OK > > + > > +#define GET_EVENTBUF_COUNT() \ > > + (GEVNTCOUNT_EVNTCOUNT (MmioRead32 (GEVNTCOUNT (0)))) > > +#define UPDATE_EVENTBUF_COUNT(x) \ > > + (MmioWrite32 (GEVNTCOUNT (0), GEVNTCOUNT_EVNTCOUNT (x))) > > + > > +#define SET_DEVADDR(x) \ > > + (MmioAndThenOr32 (DCFG, ~DCFG_DEVADDR_MASK, DCFG_DEVADDR (x))) > > Should these not also move to DwUsb3Dxe.h? > OK > > + > > +EFI_GUID gDwUsbProtocolGuid = DW_USB_PROTOCOL_GUID; > > This looks less than ideal - this makes it impossible to include > DwUsb2 and DwUsb3 drivers in the same image. > On the view of hardware, DwUsb2 and DwUsb3 doesn't exist at the same time. > > + > > +STATIC DW_USB_PROTOCOL *DwUsb; > > + > > +STATIC usb3_pcd_t gPcd; > > This seems a very unfortunate choice for variable name. > OK. I'll rename it. > > +STATIC UINT32 *gEventBuf, *gEventPtr; > > +STATIC struct usb_device_descriptor gDwUsb3DevDesc; > > +STATIC VOID *gRxBuf; > > + > > +STATIC usb_setup_pkt_t *gEndPoint0SetupPacket; > > +STATIC UINT8 *gEndPoint0StatusBuf; > > Although most if not all of these will disappear when converting to > UEFI driver model, why do they have 'g' prefix in the first place? > 'g' is for globally visible symbols. Variables that are "global" in > this file only are better off using 'm'. OK. I'll fix it. > > > +STATIC USB_DEVICE_RX_CALLBACK mDataReceivedCallback; > > +STATIC UINTN mDataBufferSize; > > + > > +struct usb_interface_descriptor intf = { > > + sizeof (struct usb_interface_descriptor), > > + UDESC_INTERFACE, > > + 0, > > + 0, > > + 2, > > + USB_CLASS_VENDOR_SPEC, > > + 0x42, > > + 0x03, > > + 0 > > +}; > > + > > +const struct usb_ss_ep_comp_descriptor ep_comp = { > > + sizeof (struct usb_ss_ep_comp_descriptor), > > + UDESC_SS_USB_COMPANION, > > + 0, > > + 0, > > + 0 > > +}; > > + > > +const struct usb_endpoint_descriptor hs_bulk_in = { > > + sizeof (struct usb_endpoint_descriptor), > > + UDESC_ENDPOINT, > > + UE_DIR_IN | USB3_BULK_IN_EP, > > + USB_ENDPOINT_XFER_BULK, > > + 0x200, > > + 0 > > +}; > > + > > +const struct usb_endpoint_descriptor > > +hs_bulk_out = { > > + sizeof(struct usb_endpoint_descriptor), /* bLength */ > > + UDESC_ENDPOINT, /* bDescriptorType */ > > + > > + UE_DIR_OUT | USB3_BULK_OUT_EP, /* bEndpointAddress */ > > + USB_ENDPOINT_XFER_BULK, /* bmAttributes */ > > + 0x200, /* wMaxPacketSize: 512 of high-speed */ > > + 1, /* bInterval */ > > +}; > > + > > +const struct usb_endpoint_descriptor ss_bulk_in = { > > + sizeof(struct usb_endpoint_descriptor), /* bLength */ > > + UDESC_ENDPOINT, /* bDescriptorType */ > > + > > + UE_DIR_IN | USB3_BULK_IN_EP, /* bEndpointAddress */ > > + USB_ENDPOINT_XFER_BULK, /* bmAttributes */ > > + 0x400, /* wMaxPacketSize: 1024 of super-speed */ > > + 0, /* bInterval */ > > +}; > > + > > +const struct usb_endpoint_descriptor ss_bulk_out = { > > + sizeof(struct usb_endpoint_descriptor), /* bLength */ > > + UDESC_ENDPOINT, /* bDescriptorType */ > > + > > + UE_DIR_OUT | USB3_BULK_OUT_EP, /* bEndpointAddress */ > > + USB_ENDPOINT_XFER_BULK, /* bmAttributes */ > > + 0x400, /* wMaxPacketSize: 1024 of super-speed */ > > + 0, /* bInterval */ > > +}; > > + > > +/** The BOS Descriptor */ > > What's a BOS? > Let me add comment on it. > > + > > +const struct usb_dev_cap_20_ext_desc cap1 = { > > + sizeof(struct usb_dev_cap_20_ext_desc), /* bLength */ > > + UDESC_DEVICE_CAPABILITY, /* bDescriptorType */ > > + USB_DEVICE_CAPABILITY_20_EXTENSION, /* bDevCapabilityType */ > > + 0x2, /* bmAttributes */ > > +}; > > + > > +const struct usb_dev_cap_ss_usb > > +cap2 = { > > + sizeof(struct usb_dev_cap_ss_usb), /* bLength */ > > + UDESC_DEVICE_CAPABILITY, /* bDescriptorType */ > > + USB_DEVICE_CAPABILITY_SS_USB, /* bDevCapabilityType */ > > + 0x0, /* bmAttributes */ > > + (USB_DC_SS_USB_SPEED_SUPPORT_SS | > > + USB_DC_SS_USB_SPEED_SUPPORT_HIGH), /* wSpeedsSupported */ > > + 0x2, /* bFunctionalitySupport */ > > + 0xa, /* bU1DevExitLat */ > > + 0x100, /* wU2DevExitLat */ > > +}; > > + > > +const struct usb_dev_cap_container_id > > +cap3 = { > > + sizeof(struct usb_dev_cap_container_id),/* bLength */ > > + UDESC_DEVICE_CAPABILITY, /* bDescriptorType */ > > + USB_DEVICE_CAPABILITY_CONTAINER_ID, /* bDevCapabilityType */ > > + 0, /* bReserved */ > > + {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* containerID */ > > +}; > > + > > +const struct wusb_bos_desc > > +bos = { > > + sizeof(struct wusb_bos_desc), /* bLength */ > > + UDESC_BOS, /* bDescriptorType */ > > + (sizeof(struct wusb_bos_desc) + \ > > + sizeof(cap1) + sizeof(cap2) + \ > > + sizeof(cap3)), /* wTotalLength */ > > + 3, /* bNumDeviceCaps */ > > +}; > > + > > +STATIC struct usb_enum_port_param usb_port_activity_config = { > > + .idVendor = USB_ENUM_ADB_PORT_VID, > > + .idProduct = USB_ENUM_ADB_PORT_PID, > > + .bInterfaceSubClass = USB_ENUM_INTERFACE_ADB_SUBCLASS, > > + .bInterfaceProtocol = USB_ENUM_INTERFACE_ADB_PROTOCOL > > +}; > > + > > +STATIC > > +UINT32 > > +DwUsb3GetEventBufEvent ( > > + IN UINTN Size > > + ) > > +{ > > + UINT32 Event; > > + > > + Event = *gEventPtr++; > > + if ((UINT32)(UINTN)gEventPtr >= (UINT32)(UINTN)gEventBuf + Size) { > > + gEventPtr = gEventBuf; > > + } > > + return Event; > > +} > > + > > +STATIC > > +VOID > > +DwUsb3SetFifoSize ( > > + IN UINT32 Addr, > > + IN UINT32 Depth, > > + IN UINT32 Dir, > > + IN UINT32 FifoNum > > + ) > > +{ > > + UINT32 Reg = 0; > > + > > + if (Dir == FIFO_DIR_TX) { > > + Reg = GTXFIFOSIZ (FifoNum); > > + } else if (Dir == FIFO_DIR_RX) { > > + Reg = GRXFIFOSIZ (FifoNum); > > + } else { > > + ASSERT (0); > > + } > > + MmioWrite32 (Reg, FIFOSIZ_DEP (Depth) | FIFOSIZ_ADDR (Addr)); > > +} > > + > > +STATIC > > +UINT32 > > +Handshake ( > > What does this function do? "Handshake" does not tell me enough to > figure out. > > > + IN UINT32 Reg, > > + IN UINT32 Mask, > > + IN UINT32 Done > > + ) > > +{ > > + UINT32 Timeout = 100000; > > + > > + do { > > + if ((MmioRead32 (Reg) & Mask) == Done) { > > + return 1; > > + } > > + MicroSecondDelay (1); > > + } while (Timeout-- > 0); > > + return 0; > > +} > > + > > +STATIC > > +VOID > > +DwUsb3FillDesc ( > > + IN usb3_dma_desc_t *desc, > > + IN UINT64 dma_addr, > > + IN UINT32 dma_len, > > + IN UINT32 stream, > > + IN UINT32 type, > > + IN UINT32 ctrlbits, > > + IN UINT32 own > > + ) > > +{ > > + desc->bptl = (UINT32)(dma_addr & 0xFFFFFFFF); > > + desc->bpth = (UINT32)(dma_addr >> 32); > > Could you possibly create LOW_32BIT and HIGH_32BIT macros for > MdePkg/Base.h instead? We are already duplicating these operations all > over the place. Including all over MdeModulePkg. > OK > > + desc->status = DSCSTS_XFERCNT (dma_len); > > + if (type) { > > + desc->control = DSCCTL_TRBCTL (type); > > + } > > + desc->control |= DSCCTL_STRMID_SOFN (stream) | ctrlbits; > > + ArmDataSynchronizationBarrier (); > > Please use MemoryFence() instead. > This controller is not ARM-specific. > > (DSB? That sounds excessive. DMB should suffice?) > OK > > + /* must execute this operation at last */ > > + if (own) { > > + desc->control |= DSCCTL_HWO; > > + } > > + ArmDataSynchronizationBarrier (); > > As above. > OK > > +} > > + > > +STATIC > > +VOID > > +DwUsb3DepStartNewCfg ( > > + IN UINT32 EpIdx, > > + IN UINT32 RsrcIdx > > + ) > > +{ > > + /* start the command */ > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_XFER_RSRC_IDX (RsrcIdx) | DEPCMD_CMDTYPE (CMDTYPE_START_NEW_CFG) | \ > > + DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3DepCfg ( > > Please no abbreviations in function names. This name tells me nothing > about what the function does (and there is no comment describing its > function). > OK > > + IN UINT32 EpIdx, > > + IN UINT32 DepCfg0, > > + IN UINT32 DepCfg1, > > + IN UINT32 DepCfg2 > > + ) > > +{ > > + MmioWrite32 (DEPCMDPAR2 (EpIdx), DepCfg2); > > + MmioWrite32 (DEPCMDPAR1 (EpIdx), DepCfg1); > > + MmioWrite32 (DEPCMDPAR0 (EpIdx), DepCfg0); > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_CMDTYPE (CMDTYPE_SET_EP_CFG) | DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3DepXferCfg ( > > DepTransferConfig. > Please address throughout. > > > + IN UINT32 EpIdx, > > + IN UINT32 DepStrmCfg > > + ) > > +{ > > + MmioWrite32 (DEPCMDPAR0 (EpIdx), DepStrmCfg); > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_CMDTYPE (CMDTYPE_SET_XFER_CFG) | DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +STATIC > > +UINT8 > > +DwUsb3DepStartXfer ( > > + IN UINT32 EpIdx, > > + IN UINT64 DmaAddr, > > + IN UINT32 StreamOrUf > > + ) > > +{ > > + UINT32 Data; > > + > > + MmioWrite32 (DEPCMDPAR1 (EpIdx), (UINT32)DmaAddr); > > + MmioWrite32 (DEPCMDPAR0 (EpIdx), (UINT32)(DmaAddr >> 32)); > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_STR_NUM_OR_UF (StreamOrUf) | DEPCMD_CMDTYPE (CMDTYPE_START_XFER) | \ > > + DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > + Data = MmioRead32 (DEPCMD (EpIdx)); > > + return GET_DEPCMD_XFER_RSRC_IDX(Data); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3DepStopXfer ( > > + IN UINT32 EpIdx, > > + IN UINT32 Tri > > + ) > > +{ > > + MmioWrite32 (DEPCMDPAR2 (EpIdx), 0); > > + MmioWrite32 (DEPCMDPAR1 (EpIdx), 0); > > + MmioWrite32 (DEPCMDPAR0 (EpIdx), 0); > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_XFER_RSRC_IDX (Tri) | DEPCMD_CMDTYPE (CMDTYPE_END_XFER) | \ > > + DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +VOID > > +DwUsb3DepUpdateXfer ( > > + IN UINT32 EpIdx, > > + IN UINT32 Tri > > + ) > > +{ > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_XFER_RSRC_IDX (Tri) | DEPCMD_CMDTYPE (CMDTYPE_UPDATE_XFER) | \ > > + DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3DepClearStall ( > > + IN UINTN EpIdx > > + ) > > +{ > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_CMDTYPE (CMDTYPE_CLR_STALL) | DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > + > > +STATIC > > +VOID > > +DwUsb3DepSetStall ( > > + IN UINTN EpIdx > > + ) > > +{ > > + MmioWrite32 ( > > + DEPCMD (EpIdx), > > + DEPCMD_CMDTYPE (CMDTYPE_SET_STALL) | DEPCMD_CMDACT > > + ); > > + Handshake (DEPCMD (EpIdx), DEPCMD_CMDACT, 0); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3EnableEp ( > > EnableEndpoint > OK > > + IN UINT32 EpIdx, > > EndpointIndex. > OK > > + IN usb3_pcd_ep_t *ep > > Endpoint. > OK > > + ) > > +{ > > + UINT32 Dalepena; > > What? > > > + > > + Dalepena = MmioRead32 (DALEPENA); > > + /* If the EP is already enabled, skip to set it again. */ > > endpoint > > > + if (Dalepena & (1 << EpIdx)) { > > + return; > > + } > > + Dalepena |= 1 << EpIdx; > > + MmioWrite32 (DALEPENA, Dalepena); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3Ep0Activate ( > > (EP0 is special, so may not need expanding.) > > > + IN OUT usb3_pcd_t *pcd > > + ) > > +{ > > + /* issue DEPCFG command to EP0 OUT */ > > ... except for in comments. > > > + DwUsb3DepStartNewCfg (EP_OUT_IDX (0), 0); > > + DwUsb3DepCfg ( > > + EP_OUT_IDX (0), > > + EPCFG0_EPTYPE (EPTYPE_CONTROL) | EPCFG0_MPS (512), > > + EPCFG1_XFER_CMPL | EPCFG1_XFER_NRDY, > > + 0 > > + ); > > + /* issue DEPSTRMCFG command to EP0 OUT */ > > + DwUsb3DepXferCfg (EP_OUT_IDX (0), 1); // one stream > > + /* issue DEPCFG command to EP0 IN */ > > + DwUsb3DepCfg ( > > + EP_IN_IDX (0), > > + EPCFG0_EPTYPE (EPTYPE_CONTROL) | EPCFG0_MPS (512) | \ > > + EPCFG0_TXFNUM (pcd->ep0.tx_fifo_num), > > + EPCFG1_XFER_NRDY | EPCFG1_XFER_CMPL | EPCFG1_EP_DIR_IN, > > + 0 > > + ); > > + /* issue DEPSTRMCFG command to EP0 IN */ > > + DwUsb3DepXferCfg (EP_IN_IDX (0), 1); // one stream > > + pcd->ep0.active = 1; > > +} > > + > > +STATIC > > +VOID > > +DwUsb3EpActivate ( > > + IN OUT usb3_pcd_t *pcd, > > + IN OUT usb3_pcd_ep_t *ep > > + ) > > +{ > > + UINT32 EpIdx, DepCfg0, DepCfg1; > > + if (ep->is_in) { > > + EpIdx = EP_IN_IDX (ep->num); > > + } else { > > + EpIdx = EP_OUT_IDX (ep->num); > > + } > > + > > + /* Start a new configurate when enable the first EP. */ > > + if (!pcd->eps_enabled) { > > + pcd->eps_enabled = 1; > > + /* Issue DEPCFG command to physical EP1 (logical EP0 IN) first. > > + * It resets the core's Tx FIFO mapping table. > > + */ > > + DepCfg0 = EPCFG0_EPTYPE (EPTYPE_CONTROL); > > + DepCfg0 |= EPCFG0_CFG_ACTION (CFG_ACTION_MODIFY); > > + DepCfg1 = EPCFG1_XFER_CMPL | EPCFG1_XFER_NRDY | EPCFG1_EP_DIR_IN; > > + > > + switch (pcd->speed) { > > + case USB_SPEED_SUPER: > > + DepCfg0 |= EPCFG0_MPS (512); > > + break; > > + case USB_SPEED_HIGH: > > + case USB_SPEED_FULL: > > + DepCfg0 |= EPCFG0_MPS (64); > > + break; > > + case USB_SPEED_LOW: > > + DepCfg0 |= EPCFG0_MPS (8); > > No live coding of values. > Add #defines for those speed setting values. > > > + break; > > + default: > > + ASSERT (0); > > + break; > > + } > > + DwUsb3DepCfg (EP_IN_IDX (0), DepCfg0, DepCfg1, 0); > > What does that final 0 do? > > > + DwUsb3DepStartNewCfg (EP_OUT_IDX (0), 2); > > And that 2? > > Please address throughout. > > > + } > > + /* issue DEPCFG command to EP */ > > + DepCfg0 = EPCFG0_EPTYPE (ep->type); > > + DepCfg0 |= EPCFG0_MPS (ep->maxpacket); > > + if (ep->is_in) { > > + DepCfg0 |= EPCFG0_TXFNUM (ep->tx_fifo_num); > > + } > > + DepCfg0 |= EPCFG0_BRSTSIZ (ep->maxburst); > > + DepCfg1 = EPCFG1_EP_NUM (ep->num); > > + if (ep->is_in) { > > + DepCfg1 |= EPCFG1_EP_DIR_IN; > > + } else { > > + DepCfg1 |= EPCFG1_XFER_CMPL; > > + } > > + DwUsb3DepCfg (EpIdx, DepCfg0, DepCfg1, 0); > > + /* issue DEPSTRMCFG command to EP */ > > + DwUsb3DepXferCfg (EpIdx, 1); > > + DwUsb3EnableEp (EpIdx, ep); > > + ep->active = 1; > > +} > > + > > +STATIC > > +VOID > > +DwUsb3Ep0OutStart ( > > + IN usb3_pcd_t *pcd > > + ) > > +{ > > + usb3_dma_desc_t *desc; > > + > > + /* Get the SETUP packet DMA Descriptor (TRB) */ > > + desc = pcd->ep0_setup_desc; > > + > > + /* DMA Descriptor setup */ > > + DwUsb3FillDesc ( > > + desc, > > + (UINT64)gEndPoint0SetupPacket, > > + pcd->ep0.maxpacket, > > + 0, > > + TRBCTL_SETUP, > > + DSCCTL_IOC | DSCCTL_ISP | DSCCTL_LST, > > + 1 > > + ); > > + > > + /* issue DEPSTRTXFER command to EP0 OUT */ > > + pcd->ep0.tri_out = DwUsb3DepStartXfer (EP_OUT_IDX (0), (UINT64)desc, 0); > > +} > > + > > +STATIC > > +VOID > > +DwUsb3Init ( > > + VOID > > + ) > > +{ > > + UINT32 Data, Addr; > > Address. > > > + usb3_pcd_t *pcd = &gPcd; > > + > > + /* soft reset the usb core */ > > + do { > > + MmioAndThenOr32 (DCTL, ~DCTL_RUN_STOP, DCTL_CSFTRST); > > + > > + do { > > + MicroSecondDelay (1000); > > + Data = MmioRead32 (DCTL); > > + } while (Data & DCTL_CSFTRST); > > + // > > + // wait for at least 3 PHY clocks > > + // > > + MicroSecondDelay (1000); > > + } while (0); > > + > > + pcd->link_state = 0; > > + > > + /* TI PHY: Set Turnaround Time = 9 (8-bit UTMI+ / ULPI) */ > > + MmioAndThenOr32 ( > > + GUSB2PHYCFG (0), > > + ~GUSB2PHYCFG_USBTRDTIM_MASK, > > + GUSB2PHYCFG_USBTRDTIM (9) > > How long is 9? > > I will stop my review here and give a full review on v2 when the > overarching comments have been addressed. > OK Best Regards Haojian