From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::244; helo=mail-wm0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (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 CA04521A1099A for ; Tue, 12 Dec 2017 09:42:43 -0800 (PST) Received: by mail-wm0-x244.google.com with SMTP id b199so315476wme.1 for ; Tue, 12 Dec 2017 09:47:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mik8ZWUJdJV+Mz7mLUzfUK2nxR0IDJnL+Dl8O1E9LmQ=; b=BUDcYSIbVeAVEo9icSO+dv7eB05igyLULQBORNv96gGq7NEjFeOE/snCGWFz5GiTeW hcj90rDoeQqAw7FQfjnJlaPlDH4EjiH9bx3l1N4spaAytSCDDngnBTLRYW9Wj+suAHC/ elj7b1r7Flzkpd+3M0Sh7EotnPcXF5IktTC+w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mik8ZWUJdJV+Mz7mLUzfUK2nxR0IDJnL+Dl8O1E9LmQ=; b=p+BWMpjR9VqXo8DJmQAfD0D5ohrj0rUX4XoO4w/EaYQz0pCBAE6mZOv5ZHaDg1I6xm 4JO4lf7faR1jAHGDHMUiAScX161lWCT0h9gywqyq3z12W9rRESufjgJ4zqVhbKn9IOYF KIjDYzMrNm9RpXst5grt9C31vIgl/naxYSWj7rws+aIJgBKcoSOj2wjyXjU7J95ms1Wv fw/l7N6g8fKl4dG6aPsoqGVaED00VU83O0yvWA9jfMPYGOpmk38bBzr3Hye/mI7vUk+F vKe8CXpYenbvWocok3mxQ5MPoHwzpAAffetx7W0tC/HSgPKzGbh4ZtCMfZnKUJo7mQnX Zv3w== X-Gm-Message-State: AKGB3mIYkBXh+F484BDJ8GWOsi0Y8WeKB3HpDfDnaNmrvDLTowg+JEWM TDkJYHBQrilWCnPBQTzV8vfFWwV7cAo= X-Google-Smtp-Source: ACJfBottR49rDejCtJA4DvW9wpKOkkmUeW4uatSjdBgd0a2QBbl41YGgXm2FBifvC5Nl6ybyTGameA== X-Received: by 10.28.66.138 with SMTP id k10mr2605445wmi.88.1513100841304; Tue, 12 Dec 2017 09:47:21 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id e197sm114233wmf.42.2017.12.12.09.47.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 12 Dec 2017 09:47:20 -0800 (PST) Date: Tue, 12 Dec 2017 17:47:18 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, daniel.thompson@linaro.org, masami.hiramatsu@linaro.org Message-ID: <20171212174718.mbbrhh7opjbqmxs6@bivouac.eciton.net> References: <20171212103807.18836-1-ard.biesheuvel@linaro.org> <20171212103807.18836-9-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20171212103807.18836-9-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Dec 2017 17:42:44 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Dec 12, 2017 at 10:38:07AM +0000, Ard Biesheuvel wrote: > For some reason, the Asmedia 118x PCIe switch needs a little help to > make sure that the downstream links train at Gen2 speed. So add a > PCI I/O protocol notifier that implements this for each PCIe downstream > port that is present on the system. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c | 140 ++++++++++++++++++++ > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 13 +- > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 37 ++++++ > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 3 + > 4 files changed, 184 insertions(+), 9 deletions(-) > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c > new file mode 100644 > index 000000000000..b069b42d0a42 > --- /dev/null > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c Bikeshedding time: This driver would likely be needed for any other platform including this switch as well, right? While it may be premature to create a standalone driver under Silicon/Asmedia ... how about calling this file something to make it clear that it is specifically intended to handle Asmedia 118x devices, to make it easier* to do so in the future? I.e. Asmedia118x.c? * by avoiding accruing other random bits of platform-specific PCI hackery in the same file. / Leif > @@ -0,0 +1,140 @@ > + /** @file > + SynQuacer DXE platform driver - PCIe support > + > + Copyright (c) 2017, Linaro, Ltd. 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 "PlatformDxe.h" > + > +#define ASMEDIA_VID 0x1b21 > +#define ASM1182E_PID 0x1182 > +#define ASM1184E_PID 0x1184 > + > +#define ASM118x_PCIE_CAPABILITY_OFFSET 0x80 > +#define ASM118x_PCIE_LINK_CONTROL_OFFSET (ASM118x_PCIE_CAPABILITY_OFFSET + \ > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, \ > + LinkControl)) > + > +STATIC VOID *mPciProtocolNotifyRegistration; > +STATIC EFI_EVENT mPciProtocolNotifyEvent; > + > +#pragma pack(1) > +typedef struct { > + EFI_PCI_CAPABILITY_HDR CapHdr; > + PCI_REG_PCIE_CAPABILITY Pcie; > +} PCIE_CAP; > +#pragma pack() > + > +STATIC > +VOID > +RetrainAsm1184eDownstreamPort ( > + IN EFI_PCI_IO_PROTOCOL *PciIo > + ) > +{ > + UINT16 PciVidPid[2]; > + EFI_STATUS Status; > + PCIE_CAP Cap; > + PCI_REG_PCIE_LINK_CONTROL LinkControl; > + > + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_VENDOR_ID_OFFSET, > + ARRAY_SIZE (PciVidPid), &PciVidPid); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_WARN, "%a: failed to read PCI vendor/product ID - %r\n", > + __FUNCTION__, Status)); > + return; > + } > + > + if (PciVidPid[0] != ASMEDIA_VID || > + (PciVidPid[1] != ASM1182E_PID && PciVidPid[1] != ASM1184E_PID)) { > + return; > + } > + > + // > + // The upstream and downstream ports share the same PID/VID, so check > + // the port type. This assumes the PCIe Express capability block lives > + // at offset 0x80 in the port's config space, which is known to be the > + // case for these particular chips. > + // > + ASSERT (sizeof (Cap) == sizeof (UINT32)); > + ASSERT (sizeof (LinkControl) == sizeof (UINT16)); > + > + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, > + ASM118x_PCIE_CAPABILITY_OFFSET, 1, &Cap); > + ASSERT_EFI_ERROR (Status); > + ASSERT (Cap.CapHdr.CapabilityID == EFI_PCI_CAPABILITY_ID_PCIEXP); > + > + if (Cap.Pcie.Bits.DevicePortType != PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT) { > + return; > + } > + > + DEBUG ((DEBUG_INFO, "%a: retraining ASM1184x downstream PCIe port\n", > + __FUNCTION__)); > + > + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, > + ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl); > + ASSERT_EFI_ERROR (Status); > + > + LinkControl.Bits.RetrainLink = 1; > + > + Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, > + ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl); > + ASSERT_EFI_ERROR (Status); > +} > + > +STATIC > +VOID > +EFIAPI > +OnPciIoProtocolNotify ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_PCI_IO_PROTOCOL *PciIo; > + EFI_STATUS Status; > + EFI_HANDLE HandleBuffer; > + UINTN BufferSize; > + > + while (TRUE) { > + BufferSize = sizeof (EFI_HANDLE); > + Status = gBS->LocateHandle (ByRegisterNotify, NULL, > + mPciProtocolNotifyRegistration, &BufferSize, &HandleBuffer); > + if (EFI_ERROR (Status)) { > + break; > + } > + > + Status = gBS->HandleProtocol (HandleBuffer, &gEfiPciIoProtocolGuid, > + (VOID **)&PciIo); > + ASSERT_EFI_ERROR (Status); > + > + // > + // The ASM1184E 4-port PCIe switch on the DeveloperBox board (and its > + // 2-port sibling of which samples were used in development) needs a > + // little nudge to get it to train the downstream links at Gen2 speed. > + // > + RetrainAsm1184eDownstreamPort (PciIo); > + } > +} > + > +EFI_STATUS > +EFIAPI > +RegisterPcieNotifier ( > + VOID > + ) > +{ > + mPciProtocolNotifyEvent = EfiCreateProtocolNotifyEvent ( > + &gEfiPciIoProtocolGuid, > + TPL_CALLBACK, > + OnPciIoProtocolNotify, > + NULL, > + &mPciProtocolNotifyRegistration); > + > + return EFI_SUCCESS; > +} > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > index e58a2093eb49..098a4dbd324e 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > @@ -12,15 +12,7 @@ > WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > **/ > > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > +#include "PlatformDxe.h" > > STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mNetsecDesc[] = { > { > @@ -202,5 +194,8 @@ PlatformDxeEntryPoint ( > > SmmuEnableCoherentDma (); > > + Status = RegisterPcieNotifier (); > + ASSERT_EFI_ERROR (Status); > + > return EFI_SUCCESS; > } > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h > new file mode 100644 > index 000000000000..d1dad2a3eace > --- /dev/null > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h > @@ -0,0 +1,37 @@ > +/** @file > + SynQuacer DXE platform driver. > + > + Copyright (c) 2017, Linaro, Ltd. 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 __PLATFORM_DXE_H__ > +#define __PLATFORM_DXE_H__ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +EFI_STATUS > +EFIAPI > +RegisterPcieNotifier ( > + VOID > + ); > + > +#endif > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > index 00c1130906c4..84498eaddcef 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > @@ -23,6 +23,7 @@ [Defines] > ENTRY_POINT = PlatformDxeEntryPoint > > [Sources] > + Pcie.c > PlatformDxe.c > > [Packages] > @@ -41,6 +42,7 @@ [LibraryClasses] > MemoryAllocationLib > UefiBootServicesTableLib > UefiDriverEntryPoint > + UefiLib > > [Guids] > gFdtTableGuid > @@ -50,6 +52,7 @@ [Guids] > > [Protocols] > gEdkiiNonDiscoverableDeviceProtocolGuid ## PRODUCES > + gEfiPciIoProtocolGuid ## CONSUMES > gPcf8563RealTimeClockLibI2cMasterProtolGuid ## PRODUCES > > [FixedPcd] > -- > 2.11.0 >