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::243; helo=mail-wm0-x243.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) (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 6B507222A337C for ; Wed, 24 Jan 2018 03:56:58 -0800 (PST) Received: by mail-wm0-x243.google.com with SMTP id i186so7979234wmi.4 for ; Wed, 24 Jan 2018 04:02:26 -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=EPf1ziuEWO4NY4DsznmIimDWtEr1PoJuXvHJ61bhcTo=; b=DXc7k4iB8Ta+Ub3G4RB6PbYnMZo6g0XKA2eeppIaRUI+Ht9Zo9FJPMn6SwIeNyu8oG UWQlAd8khgSwNXYYRuGJTAUO1HFUOhEkLf53sjyCo2QHZL1p6/jn3gaBZygc8XpJm9Qv d2A3adjw37WXwzOJuLtocREo9m6z9K04A4FJQ= 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=EPf1ziuEWO4NY4DsznmIimDWtEr1PoJuXvHJ61bhcTo=; b=WLfHJCdNbm4BaTC+a+Tibaht0pJJA/pcmjnUpZ4hQejUx06dt5TVQnfKjJ0x6CjRS/ ForxOF1dFvRUyTSgRax1WqUHq5/TNyc4QN9fIe3Cjzxad/Hxyitj2KnF808Ht7cg9Hbg 6p1wEHDZ0LyYJsPUkH4H78/fQncm5/5YZP81EnA5cJkBgOOBsqJ8MYNlpXxfFba5NhDP Rbhxkua4/oJidxdrIMaChkSOe3vvodttPSzREm5fl8UsUqXfTHj1rfP+tEUlAmWyR5Vp 9veErtjwxL7qEG8DOqZeHqY1ZSWQjXskgqSdzzU+ma4Jreev2s6cRIgYyKCiJtnHnJqF NFcQ== X-Gm-Message-State: AKwxytfHNTII4mYZi0058ZtyhkJuCZ/InUbZ4btP4PMdkzKoG41RsGdg kIjHjnp0la9127o1awDnJ1iyfIZ7ztM= X-Google-Smtp-Source: AH8x224ngaHaeqVv/TeT1dtyefIbw2vj27DIdgaUWVrB/4IRAWo3CYsFToq+B3Cm5vpOHkP0LDkj9A== X-Received: by 10.28.74.66 with SMTP id x63mr4350057wma.4.1516795344818; Wed, 24 Jan 2018 04:02:24 -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 67sm208728wmg.13.2018.01.24.04.02.23 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Jan 2018 04:02:23 -0800 (PST) Date: Wed, 24 Jan 2018 12:02:22 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org Message-ID: <20180124120222.yyaih5vnylacreg6@bivouac.eciton.net> References: <20180124115711.18797-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20180124115711.18797-1-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH v2] Silicon/SynQuacer/PlatformDxe: enable spread spectrum mode for ASM1061 SATA X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 11:56:59 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jan 24, 2018 at 11:57:11AM +0000, Ard Biesheuvel wrote: > The ASM1061 SATA controller integrated into the DeveloperBox board > emits too much electromagnetic radiation, so it needs spread spectrum > mode enabled. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm > --- > v2: - rename AsmediaXXX.c to Pci.c > - rename RegisterAsm1184Notifier() to RegisterPciIoNotifier() and make > its invocation unconditional (rather than only if PCI domain #0 is > not limited to Gen1 speed) > - use switch() rather than if() in PID/VID check Thanks! > > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Pci.c} | 87 +++++++++++++++----- > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 18 ++-- > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 5 +- > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 +- > 4 files changed, 80 insertions(+), 32 deletions(-) > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c > similarity index 63% > rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c > rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c > index c4cbacd3dff9..7ac96ab22b7a 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c > @@ -15,9 +15,12 @@ > #include "PlatformDxe.h" > > #define ASMEDIA_VID 0x1b21 > +#define ASM1061_PID 0x0612 > #define ASM1182E_PID 0x1182 > #define ASM1184E_PID 0x1184 > > +#define ASM1061_SSC_OFFSET 0xA10 > + > #define ASM118x_PCIE_CAPABILITY_OFFSET 0x80 > #define ASM118x_PCIE_LINK_CONTROL_OFFSET (ASM118x_PCIE_CAPABILITY_OFFSET + \ > OFFSET_OF (PCI_CAPABILITY_PCIEXP, \ > @@ -39,24 +42,10 @@ 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 > @@ -91,6 +80,34 @@ RetrainAsm1184eDownstreamPort ( > > STATIC > VOID > +EnableAsm1061SpreadSpectrum ( > + IN EFI_PCI_IO_PROTOCOL *PciIo > + ) > +{ > + EFI_STATUS Status; > + UINT8 SscVal; > + > + DEBUG ((DEBUG_INFO, "%a: enabling spread spectrum mode 0 for ASM1061\n", > + __FUNCTION__)); > + > + // SSC mode 0~-4000 ppm, 1:1 modulation > + > + SscVal = 0; > + Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1, > + &SscVal); > + ASSERT_EFI_ERROR (Status); > + > + MemoryFence (); > + gBS->Stall (1); // delay at least 100 ns between writes of the same register > + > + SscVal = 1; > + Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1, > + &SscVal); > + ASSERT_EFI_ERROR (Status); > +} > + > +STATIC > +VOID > EFIAPI > OnPciIoProtocolNotify ( > IN EFI_EVENT Event, > @@ -101,6 +118,7 @@ OnPciIoProtocolNotify ( > EFI_STATUS Status; > EFI_HANDLE HandleBuffer; > UINTN BufferSize; > + UINT16 PciVidPid[2]; > > while (TRUE) { > BufferSize = sizeof (EFI_HANDLE); > @@ -114,18 +132,45 @@ OnPciIoProtocolNotify ( > (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); > + 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)); > + continue; > + } > + > + if (PciVidPid[0] != ASMEDIA_VID) { > + continue; > + } > + > + switch (PciVidPid[1]) { > + case ASM1061_PID: > + // > + // The ASM1061 SATA controller as integrated into the DeveloperBox design > + // emits too much electromagnetic radiation. So enable spread spectrum > + // mode. > + // > + EnableAsm1061SpreadSpectrum (PciIo); > + break; > + case ASM1182E_PID: > + case ASM1184E_PID: > + // > + // 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. > + // > + if (mHiiSettings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) { > + RetrainAsm1184eDownstreamPort (PciIo); > + } > + break; > + } > } > } > > EFI_STATUS > EFIAPI > -RegisterAsm1184Notifier ( > +RegisterPciIoNotifier ( > VOID > ) > { > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > index 11b31e77bd3f..f3b05fc973ed 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c > @@ -14,6 +14,9 @@ > > #include "PlatformDxe.h" > > +UINT64 mHiiSettingsVal; > +SYNQUACER_PLATFORM_VARSTORE_DATA *mHiiSettings; > + > typedef struct { > VENDOR_DEVICE_PATH VendorDevicePath; > EFI_DEVICE_PATH_PROTOCOL End; > @@ -253,8 +256,9 @@ PlatformDxeEntryPoint ( > VOID *Dtb; > UINTN DtbSize; > EFI_HANDLE Handle; > - UINT64 SettingsVal; > - SYNQUACER_PLATFORM_VARSTORE_DATA *Settings; > + > + mHiiSettingsVal = PcdGet64 (PcdPlatformSettings); > + mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal; > > Dtb = NULL; > Status = DtPlatformLoadDtb (&Dtb, &DtbSize); > @@ -294,17 +298,13 @@ PlatformDxeEntryPoint ( > > SmmuEnableCoherentDma (); > > - SettingsVal = PcdGet64 (PcdPlatformSettings); > - Settings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&SettingsVal; > - if (Settings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) { > - Status = RegisterAsm1184Notifier (); > - ASSERT_EFI_ERROR (Status); > - } > + Status = RegisterPciIoNotifier (); > + ASSERT_EFI_ERROR (Status); > > Status = EnableSettingsForm (); > ASSERT_EFI_ERROR (Status); > > - if (Settings->EnableEmmc == EMMC_ENABLED) { > + if (mHiiSettings->EnableEmmc == EMMC_ENABLED) { > Status = RegisterEmmc (); > ASSERT_EFI_ERROR (Status); > } > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h > index de7de12cec97..b634e8be99ad 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h > @@ -40,9 +40,12 @@ > extern UINT8 PlatformDxeHiiBin[]; > extern UINT8 PlatformDxeStrings[]; > > +extern UINT64 mHiiSettingsVal; > +extern SYNQUACER_PLATFORM_VARSTORE_DATA *mHiiSettings; > + > EFI_STATUS > EFIAPI > -RegisterAsm1184Notifier ( > +RegisterPciIoNotifier ( > VOID > ); > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > index 16412b999a40..e13e16f3da6b 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > @@ -23,8 +23,8 @@ [Defines] > ENTRY_POINT = PlatformDxeEntryPoint > > [Sources] > - Asmedia118x.c > Emmc.c > + Pci.c > PlatformDxe.c > PlatformDxeHii.uni > PlatformDxeHii.vfr > -- > 2.11.0 >