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::242; helo=mail-wm0-x242.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (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 D7530212B7DB1 for ; Tue, 12 Jun 2018 11:37:38 -0700 (PDT) Received: by mail-wm0-x242.google.com with SMTP id p126-v6so668041wmb.2 for ; Tue, 12 Jun 2018 11:37:38 -0700 (PDT) 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=tpEIwH4rPa6B9OzlwLr+iFSj0dhd0pb8jGdVzuxtxD8=; b=YYP1SAYVtysSK34QkIzQmokAMlEKsyo09VkLKWoBuHtZjQD7920ckGSaYsxmcF5maj ZcsMZf9oaI8dI9kudgkTZcczp3X+SbGLs7SoUy6VmH2Qr2TcYDWuQ4YfuEaPd2ZcQNm0 JX69dZP/TIg6IO99PXLW9FpCBLKnl4OR636ZY= 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=tpEIwH4rPa6B9OzlwLr+iFSj0dhd0pb8jGdVzuxtxD8=; b=a9+kW4y4fWEYcqHbrTDBFQbsnQFTzQYJm+ISipWL1LsdYJMQxthiM8BmKv1MH/PLZJ I17quR3nbEfps6JZbeeS6RDajYpaSuzFQkJs0tVgmr8gMPsKnv2ZPnMUvQZjjKx+zhYV Au44rWmBQbMXJhn6+GKhkbKTWTbxtZwWDK8lZb2NeSRgVHUz7j/8rdCTFePZNGQNHB6L A19jc3E+DS4aljbJ2WgA7NDOY1UlgDMCmwnuy8a99MLIwfauE0Ysgh8dtRy3zPQz0jAB uslzyY+osuz/HeWX79I8r6B7gEOaxKo/2FiaNTCmJIXrhFXsP+H5bQrLKqc7dkiCLB1U Npaw== X-Gm-Message-State: APt69E1Ivq9OIoIATmOvKfIZvFm42q46+OSkVUCAtjLwuYgSUQnWmKmx TnqyqlEYKcfR40QdLiKHSVw7pg== X-Google-Smtp-Source: ADUXVKI345DpBFnwpzVT0aZgWKuGvEVNRfY4p347jKeET/Q8Nc5mvS7NnzxUtTiOvU20cmQ9ITVHag== X-Received: by 2002:a7b:c058:: with SMTP id u24-v6mr1182070wmc.136.1528828657323; Tue, 12 Jun 2018 11:37:37 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id g129-v6sm1169866wmf.5.2018.06.12.11.37.35 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 12 Jun 2018 11:37:36 -0700 (PDT) Date: Tue, 12 Jun 2018 19:37:34 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, jinghua@marvell.com, jsd@semihalf.com, jaz@semihalf.com Message-ID: <20180612183734.6bzz5yr73uwigcm5@bivouac.eciton.net> References: <1528472063-1660-1-git-send-email-mw@semihalf.com> <1528472063-1660-12-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1528472063-1660-12-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms PATCH 11/25] Marvell/Armada7k8k: Extend ArmadaSoCDescLib with AHCI/SDMMC/XHCI X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jun 2018 18:37:39 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jun 08, 2018 at 05:34:09PM +0200, Marcin Wojtas wrote: > This patch introduces new library callbacks for NonDiscoverable devices > i.e. AHCI/XHCI/SDMMC. They dynamically allocate and fill according > structures with the SoC description of the devices. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > Reviewed-by: Hua Jing > --- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 114 ++++++++++++++++++++ > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 48 +++++++++ > 2 files changed, 162 insertions(+) > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > index 36b445e..de57b47 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > @@ -32,6 +32,120 @@ > #define MV_SOC_CP_BASE(Cp) (0xF2000000 + (Cp) * 0x2000000) > > // > +// Platform description of NonDiscoverableDevices > +// > + > +// > +// Platform description of AHCI controllers > +// > +#define MV_SOC_AHCI_BASE(Cp) MV_SOC_CP_BASE ((Cp)) + 0x540000 > +#define MV_SOC_AHCI_ID(Cp) ((Cp) % 2) I know I didn't comment on this before, but can you please move all of these #defines (at least the added ones) into a .h file? > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescAhciGet ( > + IN OUT MV_SOC_AHCI_DESC **AhciDesc, > + IN OUT UINT8 *DescCount > + ) > +{ > + MV_SOC_AHCI_DESC *Desc; > + UINT8 CpCount = FixedPcdGet8 (PcdMaxCpCount); > + UINT8 CpIndex; UINTN for Count and Index please. > + > + Desc = AllocateZeroPool (CpCount * sizeof (MV_SOC_AHCI_DESC)); > + if (Desc == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)); > + return EFI_OUT_OF_RESOURCES; > + } > + > + for (CpIndex = 0; CpIndex < CpCount; CpIndex++) { > + Desc[CpIndex].AhciId = MV_SOC_AHCI_ID (CpIndex); > + Desc[CpIndex].AhciBaseAddress = MV_SOC_AHCI_BASE (CpIndex); > + Desc[CpIndex].AhciMemSize = SIZE_8KB; > + Desc[CpIndex].AhciDmaType = NonDiscoverableDeviceDmaTypeCoherent; > + } > + > + *AhciDesc = Desc; > + *DescCount = CpCount; > + > + return EFI_SUCCESS; > +} > + > +// > +// Platform description of SDMMC controllers > +// > +#define MV_SOC_MAX_SDMMC_COUNT 2 > +#define MV_SOC_SDMMC_BASE(Index) ((Index) == 0 ? 0xF06E0000 : 0xF2780000) Separate .h please. > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescSdMmcGet ( > + IN OUT MV_SOC_SDMMC_DESC **SdMmcDesc, > + IN OUT UINT8 *DescCount > + ) > +{ > + MV_SOC_SDMMC_DESC *Desc; > + UINT8 Index; UINTN Index. > + > + Desc = AllocateZeroPool (MV_SOC_MAX_SDMMC_COUNT * sizeof (MV_SOC_SDMMC_DESC)); > + if (Desc == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)); > + return EFI_OUT_OF_RESOURCES; > + } > + > + for (Index = 0; Index < MV_SOC_MAX_SDMMC_COUNT; Index++) { > + Desc[Index].SdMmcBaseAddress = MV_SOC_SDMMC_BASE (Index); > + Desc[Index].SdMmcMemSize = SIZE_1KB; > + Desc[Index].SdMmcDmaType = NonDiscoverableDeviceDmaTypeCoherent; > + } > + > + *SdMmcDesc = Desc; > + *DescCount = MV_SOC_MAX_SDMMC_COUNT; > + > + return EFI_SUCCESS; > +} > + > +// > +// Platform description of XHCI controllers > +// > +#define MV_SOC_XHCI_PER_CP_COUNT 2 > +#define MV_SOC_XHCI_BASE(Xhci) (0x500000 + (Xhci) * 0x10000) > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescXhciGet ( > + IN OUT MV_SOC_XHCI_DESC **XhciDesc, > + IN OUT UINT8 *DescCount > + ) > +{ > + MV_SOC_XHCI_DESC *Desc; > + UINT8 CpCount = FixedPcdGet8 (PcdMaxCpCount); > + UINT8 Index, CpIndex, XhciIndex = 0; UINTN x4. > + > + Desc = AllocateZeroPool (CpCount * MV_SOC_XHCI_PER_CP_COUNT * > + sizeof (MV_SOC_XHCI_DESC)); > + if (Desc == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)); > + return EFI_OUT_OF_RESOURCES; > + } > + > + for (CpIndex = 0; CpIndex < CpCount; CpIndex++) { > + for (Index = 0; Index < MV_SOC_XHCI_PER_CP_COUNT; Index++) { > + Desc[XhciIndex].XhciBaseAddress = > + MV_SOC_CP_BASE (CpIndex) + MV_SOC_XHCI_BASE (Index); > + Desc[XhciIndex].XhciMemSize = SIZE_16KB; > + Desc[XhciIndex].XhciDmaType = NonDiscoverableDeviceDmaTypeCoherent; > + XhciIndex++; > + } > + } > + > + *XhciDesc = Desc; > + *DescCount = XhciIndex; Can you do the same shuffle I suggested to an earlier patch? It isn't needed for the line length inside the inner loop as badly here, but I really don't like the last line above here (it suggests the number of descriptors isn't known until after loop traversation). And you could get the allocate statement down to one line as well. / Leif > + > + return EFI_SUCCESS; > +} > + > +// > // Platform description of PP2 NIC > // > #define MV_SOC_PP2_BASE(Cp) MV_SOC_CP_BASE ((Cp)) > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > index 559642b..438f838 100644 > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > @@ -14,6 +14,54 @@ > #ifndef __ARMADA_SOC_DESC_LIB_H__ > #define __ARMADA_SOC_DESC_LIB_H__ > > +#include > + > +// > +// NonDiscoverable devices SoC description > +// > +// AHCI > +typedef struct { > + UINT8 AhciId; > + UINTN AhciBaseAddress; > + UINTN AhciMemSize; > + NON_DISCOVERABLE_DEVICE_DMA_TYPE AhciDmaType; > +} MV_SOC_AHCI_DESC; > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescAhciGet ( > + IN OUT MV_SOC_AHCI_DESC **AhciDesc, > + IN OUT UINT8 *DescCount > + ); > + > +// SDMMC > +typedef struct { > + UINTN SdMmcBaseAddress; > + UINTN SdMmcMemSize; > + NON_DISCOVERABLE_DEVICE_DMA_TYPE SdMmcDmaType; > +} MV_SOC_SDMMC_DESC; > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescSdMmcGet ( > + IN OUT MV_SOC_SDMMC_DESC **SdMmcDesc, > + IN OUT UINT8 *DescCount > + ); > + > +// XHCI > +typedef struct { > + UINTN XhciBaseAddress; > + UINTN XhciMemSize; > + NON_DISCOVERABLE_DEVICE_DMA_TYPE XhciDmaType; > +} MV_SOC_XHCI_DESC; > + > +EFI_STATUS > +EFIAPI > +ArmadaSoCDescXhciGet ( > + IN OUT MV_SOC_XHCI_DESC **XhciDesc, > + IN OUT UINT8 *DescCount > + ); > + > // > // PP2 NIC devices SoC description > // > -- > 2.7.4 >