>-----Original Message-----
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Friday, April 20, 2018 12:57 AM
>To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com>; Vabhav Sharma
><vabhav.sha...@nxp.com>
>Subject: Re: [PATCH edk2-platforms 32/39] Silicon/NXP: Implement
>PciSegmentLib to support multiple RCs
>
>On Fri, Feb 16, 2018 at 02:20:28PM +0530, Meenakshi wrote:
>> From: Vabhav <vabhav.sha...@nxp.com>
>>
>> Multiple root complex support is not provided by standard library
>> PciLib/PciExpressLib/PciSegmentLib, Reimplementing it and provide
>> function for reading/writing into PCIe configuration Space.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vabhav <vabhav.sha...@nxp.com>
>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
>> ---
>> Silicon/NXP/Include/Pcie.h | 143 +++++
>> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c | 604
>+++++++++++++++++++++
>> .../NXP/Library/PciSegmentLib/PciSegmentLib.inf | 41 ++
>> 3 files changed, 788 insertions(+)
>> create mode 100644 Silicon/NXP/Include/Pcie.h create mode 100644
>> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
>> create mode 100644
>> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>>
>> diff --git a/Silicon/NXP/Include/Pcie.h b/Silicon/NXP/Include/Pcie.h
>> new file mode 100644 index 0000000..a7e6f9b
>> --- /dev/null
>> +++ b/Silicon/NXP/Include/Pcie.h
>> @@ -0,0 +1,143 @@
>> +/** @file
>> + PCI memory configuration for NXP
>> +
>> + Copyright 2018 NXP
>> +
>> + 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
>https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
>rce.org%2Flicenses%2Fbsd-
>license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
>daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
>36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
>egCISP%2BU%3D&reserved=0.
>> +
>> + 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 __PCI_H__
>> +#define __PCI_H__
>
>I'm not super happy about reusing such a generic name for the include guard -
>or
>really even the filename. (MdePkg/Include/Pci.h has
>_PCI_H_.)
>
>Could you rename this header NxpPcie.h and change the include guard to
>_NXP_PCIE_H_?
I see, Sure.
>
>> +
>> +// Segment 0
>> +#define PCI_SEG0_NUM 0
>> +
>> +#define PCI_SEG0_BUSNUM_MIN 0x0
>> +#define PCI_SEG0_BUSNUM_MAX 0xff
>> +
>> +#define PCI_SEG0_PORTIO_MIN 0x0
>> +#define PCI_SEG0_PORTIO_MAX 0xffff
>> +
>> +#define PCI_SEG0_MMIO32_MIN 0x40000000
>> +#define PCI_SEG0_MMIO32_MAX 0x4fffffff
>> +#define PCI_SEG0_MMIO64_MIN PCI_SEG0_MMIO_MEMBASE +
>SEG_MEM_SIZE
>> +#define PCI_SEG0_MMIO64_MAX PCI_SEG0_MMIO_MEMBASE +
>SEG_MEM_LIMIT
>> +#define PCI_SEG0_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp1BaseAddr)
>> +
>> +#define PCI_SEG0_DBI_BASE 0x03400000
>> +
>> +// Segment 1
>> +#define PCI_SEG1_NUM 1
>> +
>> +#define PCI_SEG1_BUSNUM_MIN 0x0
>> +#define PCI_SEG1_BUSNUM_MAX 0xff
>> +
>> +#define PCI_SEG1_PORTIO_MIN 0x10000
>> +#define PCI_SEG1_PORTIO_MAX 0x1ffff
>> +
>> +#define PCI_SEG1_MMIO32_MIN 0x50000000
>> +#define PCI_SEG1_MMIO32_MAX 0x5fffffff
>> +#define PCI_SEG1_MMIO64_MIN PCI_SEG1_MMIO_MEMBASE +
>SEG_MEM_SIZE
>> +#define PCI_SEG1_MMIO64_MAX PCI_SEG1_MMIO_MEMBASE +
>SEG_MEM_LIMIT
>> +#define PCI_SEG1_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp2BaseAddr)
>> +
>> +#define PCI_SEG1_DBI_BASE 0x03500000
>> +
>> +// Segment 2
>> +#define PCI_SEG2_NUM 2
>> +
>> +#define PCI_SEG2_BUSNUM_MIN 0x0
>> +#define PCI_SEG2_BUSNUM_MAX 0xff
>> +
>> +#define PCI_SEG2_PORTIO_MIN 0x20000
>> +#define PCI_SEG2_PORTIO_MAX 0x2ffff
>> +
>> +#define PCI_SEG2_MMIO32_MIN 0x60000000
>> +#define PCI_SEG2_MMIO32_MAX 0x6fffffff
>> +#define PCI_SEG2_MMIO64_MIN PCI_SEG2_MMIO_MEMBASE +
>SEG_MEM_SIZE
>> +#define PCI_SEG2_MMIO64_MAX PCI_SEG2_MMIO_MEMBASE +
>SEG_MEM_LIMIT
>> +#define PCI_SEG2_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp3BaseAddr)
>> +
>> +#define PCI_SEG2_DBI_BASE 0x03600000
>> +
>> +// Segment 3
>> +#define PCI_SEG3_NUM 3
>> +
>> +#define PCI_SEG3_BUSNUM_MIN 0x0
>> +#define PCI_SEG3_BUSNUM_MAX 0xff
>> +
>> +#define PCI_SEG3_PORTIO_MIN 0x30000
>> +#define PCI_SEG3_PORTIO_MAX 0x3ffff
>> +
>> +#define PCI_SEG3_MMIO32_MIN 0x70000000
>> +#define PCI_SEG3_MMIO32_MAX 0x7fffffff
>> +#define PCI_SEG3_MMIO64_MIN PCI_SEG3_MMIO_MEMBASE +
>SEG_MEM_SIZE
>> +#define PCI_SEG3_MMIO64_MAX PCI_SEG3_MMIO_MEMBASE +
>SEG_MEM_LIMIT
>> +#define PCI_SEG3_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp4BaseAddr)
>> +
>> +#define PCI_SEG3_DBI_BASE 0x03700000
>> +
>> +// Segment configuration
>> +#define SEG_CFG_SIZE 0x00001000
>> +#define SEG_CFG_BUS 0x00000000
>> +#define SEG_MEM_SIZE 0x40000000
>> +#define SEG_MEM_LIMIT 0x7fffffff
>> +#define SEG_MEM_BUS 0x40000000
>> +#define SEG_IO_SIZE 0x00010000
>> +#define SEG_IO_BUS 0x00000000
>> +#define PCI_BASE_DIFF 0x800000000
>> +#define PCI_DBI_SIZE_DIFF 0x100000
>> +#define PCI_SEG0_PHY_CFG0_BASE PCI_SEG0_MMIO_MEMBASE
>> +#define PCI_SEG0_PHY_CFG1_BASE PCI_SEG0_PHY_CFG0_BASE +
>SEG_CFG_SIZE
>> +#define PCI_SEG0_PHY_MEM_BASE PCI_SEG0_MMIO64_MIN
>> +#define PCI_SEG0_PHY_IO_BASE PCI_SEG0_MMIO_MEMBASE +
>SEG_IO_SIZE
>> +
>> +// iATU configuration
>> +#define IATU_VIEWPORT_OFF 0x900
>> +#define IATU_VIEWPORT_OUTBOUND 0
>> +
>> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0 0x904
>> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM 0x0
>> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_IO 0x2
>> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_CFG0 0x4 #define
>> +IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_CFG1 0x5
>> +
>> +#define IATU_REGION_CTRL_2_OFF_OUTBOUND_0 0x908
>> +#define IATU_REGION_CTRL_2_OFF_OUTBOUND_0_REGION_EN BIT31
>> +
>> +#define IATU_LWR_BASE_ADDR_OFF_OUTBOUND_0 0x90C
>> +#define IATU_UPPER_BASE_ADDR_OFF_OUTBOUND_0 0x910
>> +#define IATU_LIMIT_ADDR_OFF_OUTBOUND_0 0x914
>> +#define IATU_LWR_TARGET_ADDR_OFF_OUTBOUND_0 0x918
>> +#define IATU_UPPER_TARGET_ADDR_OFF_OUTBOUND_0 0x91C
>> +
>> +#define IATU_REGION_INDEX0 0x0
>> +#define IATU_REGION_INDEX1 0x1
>> +#define IATU_REGION_INDEX2 0x2
>> +#define IATU_REGION_INDEX3 0x3
>> +
>> +// PCIe Controller configuration
>> +#define NUM_PCIE_CONTROLLER FixedPcdGet32 (PcdNumPciController)
>> +#define PCI_LUT_DBG FixedPcdGet32 (PcdPcieLutDbg)
>> +#define PCI_LUT_BASE FixedPcdGet32 (PcdPcieLutBase)
>> +#define LTSSM_STATE_MASK 0x3f
>> +#define LTSSM_PCIE_L0 0x11
>> +#define PCI_LINK_CAP 0x7c
>> +#define PCI_LINK_SPEED_MASK 0xf
>> +#define PCI_CLASS_BRIDGE_PCI 0x6040010
>> +#define PCI_CLASS_DEVICE 0x8
>> +#define PCI_DBI_RO_WR_EN 0x8bc
>> +#define PCI_BASE_ADDRESS_0 0x10
>> +
>> +VOID GetSerdesProtocolMaps (UINT64 *);
>> +
>> +BOOLEAN IsSerDesLaneProtocolConfigured (UINT64, UINT16);
>> +
>> +#endif
>> diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
>> b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
>> new file mode 100644
>> index 0000000..acb614d
>> --- /dev/null
>> +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
>> @@ -0,0 +1,604 @@
>> +/** @file
>> + PCI Segment Library for NXP SoCs with multiple RCs
>> +
>> + Copyright 2018 NXP
>> +
>> + 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
>> +
>https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
>rce.org%2Flicenses%2Fbsd-
>license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
>daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
>36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
>egCISP%2BU%3D&reserved=0.
>> +
>> + 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 <Base.h>
>> +#include <Library/PciSegmentLib.h>
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Pcie.h>
>> +
>> +typedef enum {
>> + PciCfgWidthUint8 = 0,
>> + PciCfgWidthUint16,
>> + PciCfgWidthUint32,
>> + PciCfgWidthMax
>> +} PCI_CFG_WIDTH;
>> +
>> +/**
>> + Assert the validity of a PCI Segment address.
>> + A valid PCI Segment address should not contain 1's in bits 28..31
>> +and 48..63
>> +
>> + @param A The address to validate.
>> + @param M Additional bits to assert to be zero.
>> +
>> +**/
>> +#define ASSERT_INVALID_PCI_SEGMENT_ADDRESS(A,M) \
>> + ASSERT (((A) & (0xffff0000f0000000ULL | (M))) == 0)
>> +
>> +/**
>> + Function to return PCIe Physical Address(PCIe view) or Controller
>> + Address(CPU view) for different RCs
>> +
>> + @param Address Address passed from bus layer.
>> + @param Segment Segment number for Root Complex.
>> +
>> + @return Return PCIe CPU or Controller address.
>> +
>> +**/
>> +STATIC
>> +UINT64
>> +PciSegmentLibGetConfigBase (
>> + IN UINT64 Address,
>> + IN UINT16 Segment
>> + )
>> +{
>> +
>> + switch (Segment) {
>> + // Root Complex 1
>> + case PCI_SEG0_NUM:
>> + // Reading bus number(bits 20-27)
>> + if ((Address >> 20) & 1) {
>> + return PCI_SEG0_MMIO_MEMBASE;
>> + } else {
>> + // On Bus 0 RCs are connected
>> + return PCI_SEG0_DBI_BASE;
>> + }
>> + // Root Complex 2
>> + case PCI_SEG1_NUM:
>> + // Reading bus number(bits 20-27)
>> + if ((Address >> 20) & 1) {
>> + return PCI_SEG1_MMIO_MEMBASE;
>> + } else {
>> + // On Bus 0 RCs are connected
>> + return PCI_SEG1_DBI_BASE;
>> + }
>> + // Root Complex 3
>> + case PCI_SEG2_NUM:
>> + // Reading bus number(bits 20-27)
>> + if ((Address >> 20) & 1) {
>> + return PCI_SEG2_MMIO_MEMBASE;
>> + } else {
>> + // On Bus 0 RCs are connected
>> + return PCI_SEG2_DBI_BASE;
>> + }
>> + // Root Complex 4
>> + case PCI_SEG3_NUM:
>> + // Reading bus number(bits 20-27)
>> + if ((Address >> 20) & 1) {
>> + return PCI_SEG3_MMIO_MEMBASE;
>> + } else {
>> + // On Bus 0 RCs are connected
>> + return PCI_SEG3_DBI_BASE;
>> + }
>> + default:
>> + return 0;
>> + }
>> +
>> +}
>> +
>> +/**
>> + Internal worker function to read a PCI configuration register.
>> +
>> + @param Address The address that encodes the Segment, PCI Bus, Device,
>> + Function and Register.
>> + @param Width The width of data to read
>> +
>> + @return The value read from the PCI configuration register.
>> +
>> +**/
>> +STATIC
>> +UINT32
>> +PciSegmentLibReadWorker (
>> + IN UINT64 Address,
>> + IN PCI_CFG_WIDTH Width
>> + )
>> +{
>> + UINT64 Base;
>> + UINT16 Offset;
>> + UINT16 Segment;
>> +
>> + //
>> + // Reading Segment number(47-32) bits in Address // Segment =
>> + (Address >> 32); // // Reading Function(12-0) bits in Address //
>> + Offset = (Address & 0xfff );
>> +
>> + Base = PciSegmentLibGetConfigBase (Address, Segment);
>> +
>> + //
>> + // ignore devices > 0 on bus 0
>> + //
>> + if ((Address & 0xff00000) == 0 && (Address & 0xf8000) != 0) {
>> + return MAX_UINT32;
>> + }
>> +
>> + //
>> + // ignore device > 0 on bus 1
>> + //
>> + if ((Address & 0xfe00000) == 0 && (Address & 0xf8000) != 0) {
>> + return MAX_UINT32;
>> + }
>> +
>> + switch (Width) {
>> + case PciCfgWidthUint8:
>> + return MmioRead8 (Base + (UINT8)Offset); case PciCfgWidthUint16:
>> + return MmioRead16 (Base + (UINT16)Offset); case
>> + PciCfgWidthUint32:
>> + return MmioRead32 (Base + (UINT32)Offset);
>> + default:
>> + ASSERT (FALSE);
>> + }
>> +
>> + return CHAR_NULL;
>> +}
>> +
>> +/**
>> + Internal worker function to writes a PCI configuration register.
>> +
>> + @param Address The address that encodes the Segment, PCI Bus, Device,
>> + Function and Register.
>> + @param Width The width of data to write
>> + @param Data The value to write.
>> +
>> + @return The value written to the PCI configuration register.
>> +
>> +**/
>> +STATIC
>> +UINT32
>> +PciSegmentLibWriteWorker (
>> + IN UINT64 Address,
>> + IN PCI_CFG_WIDTH Width,
>> + IN UINT32 Data
>> + )
>> +{
>> + UINT64 Base;
>> + UINT32 Offset;
>> + UINT16 Segment;
>> +
>> + //
>> + // Reading Segment number(47-32 bits) in Address Segment =
>> + (Address >> 32); // // Reading Function(12-0 bits) in Address //
>> + Offset = (Address & 0xfff );
>
>Spurious space after 0xfff.
Alright, Thanks.
Same applies to PciSegmentLibReadWorker()
>
>Could we have some macros and #defines instead of live-coded values in this
>function?
Sure, I assume Similar changes in PciSegmentLibReadWorker() required.
>
>> +
>> + Base = PciSegmentLibGetConfigBase (Address, Segment);
>> +
>> + //
>> + // ignore devices > 0 on bus 0
>> + //
>> + if ((Address & 0xff00000) == 0 && (Address & 0xf8000) != 0) {
>> + return Data;
>> + }
>> +
>> + //
>> + // ignore device > 0 on bus 1
>> + //
>> + if ((Address & 0xfe00000) == 0 && (Address & 0xf8000) != 0) {
>> + return MAX_UINT32;
>> + }
>> +
>> + switch (Width) {
>> + case PciCfgWidthUint8:
>> + MmioWrite8 (Base + (UINT8)Offset, Data);
>> + break;
>> + case PciCfgWidthUint16:
>> + MmioWrite16 (Base + (UINT16)Offset, Data);
>> + break;
>> + case PciCfgWidthUint32:
>> + MmioWrite32 (Base + (UINT16)Offset, Data);
>> + break;
>> + default:
>> + ASSERT (FALSE);
>> + }
>> +
>> + return Data;
>> +}
>> +
>> +/**
>> + Register a PCI device so PCI configuration registers may be
>> +accessed after
>> + SetVirtualAddressMap().
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Bus,
>> Device,
>> + Function and Register.
>> +
>> + @retval RETURN_SUCCESS The PCI device was registered for runtime
>access.
>> + @retval RETURN_UNSUPPORTED An attempt was made to call this
>function
>> + after ExitBootServices().
>> + @retval RETURN_UNSUPPORTED The resources required to access the
>PCI device
>> + at runtime could not be mapped.
>> + @retval RETURN_OUT_OF_RESOURCES There are not enough resources
>available to
>> + complete the registration.
>> +
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +PciSegmentRegisterForRuntimeAccess (
>> + IN UINTN Address
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
>> + return RETURN_UNSUPPORTED;
>> +}
>> +
>> +/**
>> + Reads an 8-bit PCI configuration register.
>> +
>> + Reads and returns the 8-bit PCI configuration register specified by
>> Address.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>Function,
>> + and Register.
>> +
>> + @return The 8-bit PCI configuration register specified by Address.
>> +
>> +**/
>> +UINT8
>> +EFIAPI
>> +PciSegmentRead8 (
>> + IN UINT64 Address
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
>> +
>> + return (UINT8) PciSegmentLibReadWorker (Address, PciCfgWidthUint8);
>> +}
>> +
>> +/**
>> + Writes an 8-bit PCI configuration register.
>> +
>> + Writes the 8-bit PCI configuration register specified by Address with the
>> value
>specified by Value.
>> + Value is returned. This function must guarantee that all PCI read and
>> write
>operations are serialized.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>Function, and Register.
>> + @param Value The value to write.
>> +
>> + @return The value written to the PCI configuration register.
>> +
>> +**/
>> +UINT8
>> +EFIAPI
>> +PciSegmentWrite8 (
>> + IN UINT64 Address,
>> + IN UINT8 Value
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
>> +
>> + return (UINT8) PciSegmentLibWriteWorker (Address, PciCfgWidthUint8,
>> +Value); }
>> +
>> +/**
>> + Reads a 16-bit PCI configuration register.
>> +
>> + Reads and returns the 16-bit PCI configuration register specified by
>> Address.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> + If Address is not aligned on a 16-bit boundary, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>Function, and Register.
>> +
>> + @return The 16-bit PCI configuration register specified by Address.
>> +
>> +**/
>> +UINT16
>> +EFIAPI
>> +PciSegmentRead16 (
>> + IN UINT64 Address
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 1);
>> +
>> + return (UINT16) PciSegmentLibReadWorker (Address,
>> +PciCfgWidthUint16); }
>> +
>> +/**
>> + Writes a 16-bit PCI configuration register.
>> +
>> + Writes the 16-bit PCI configuration register specified by Address
>> + with the value specified by Value.
>> +
>> + Value is returned.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> + If Address is not aligned on a 16-bit boundary, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>Function, and Register.
>> + @param Value The value to write.
>> +
>> + @return The parameter of Value.
>> +
>> +**/
>> +UINT16
>> +EFIAPI
>> +PciSegmentWrite16 (
>> + IN UINT64 Address,
>> + IN UINT16 Value
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 1);
>> +
>> + return (UINT16) PciSegmentLibWriteWorker (Address,
>> +PciCfgWidthUint16, Value); }
>> +
>> +/**
>> + Reads a 32-bit PCI configuration register.
>> +
>> + Reads and returns the 32-bit PCI configuration register specified by
>> Address.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> + If Address is not aligned on a 32-bit boundary, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>Function,
>> + and Register.
>> +
>> + @return The 32-bit PCI configuration register specified by Address.
>> +
>> +**/
>> +UINT32
>> +EFIAPI
>> +PciSegmentRead32 (
>> + IN UINT64 Address
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 3);
>> +
>> + return PciSegmentLibReadWorker (Address, PciCfgWidthUint32); }
>> +
>> +/**
>> + Writes a 32-bit PCI configuration register.
>> +
>> + Writes the 32-bit PCI configuration register specified by Address
>> + with the value specified by Value.
>> +
>> + Value is returned.
>> +
>> + If any reserved bits in Address are set, then ASSERT().
>> + If Address is not aligned on a 32-bit boundary, then ASSERT().
>> +
>> + @param Address The address that encodes the PCI Segment, Bus, Device,
>> + Function, and Register.
>> + @param Value The value to write.
>> +
>> + @return The parameter of Value.
>> +
>> +**/
>> +UINT32
>> +EFIAPI
>> +PciSegmentWrite32 (
>> + IN UINT64 Address,
>> + IN UINT32 Value
>> + )
>> +{
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 3);
>> +
>> + return PciSegmentLibWriteWorker (Address, PciCfgWidthUint32,
>> +Value); }
>> +
>> +/**
>> + Reads a range of PCI configuration registers into a caller supplied
>> buffer.
>> +
>> + Reads the range of PCI configuration registers specified by
>> + StartAddress and Size into the buffer specified by Buffer. This
>> + function only allows the PCI configuration registers from a single
>> + PCI function to be read. Size is returned.
>> +
>> + If any reserved bits in StartAddress are set, then ASSERT().
>> + If ((StartAddress & 0xFFF) + Size) > 0x1000, then ASSERT().
>> + If Size > 0 and Buffer is NULL, then ASSERT().
>> +
>> + @param StartAddress The starting address that encodes the PCI Segment,
>Bus,
>> + Device, Function and Register.
>> + @param Size The size in bytes of the transfer.
>> + @param Buffer The pointer to a buffer receiving the data read.
>> +
>> + @return Size
>> +
>> +**/
>> +UINTN
>> +EFIAPI
>> +PciSegmentReadBuffer (
>> + IN UINT64 StartAddress,
>> + IN UINTN Size,
>> + OUT VOID *Buffer
>> + )
>> +{
>> + UINTN ReturnValue;
>> +
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (StartAddress, 0); // 0xFFF is
>> + used as limit for 4KB config space ASSERT (((StartAddress & 0xFFF)
>> + + Size) <= SIZE_4KB);
>> +
>> + if (Size == 0) {
>> + return Size;
>> + }
>> +
>> + ASSERT (Buffer != NULL);
>> +
>> + //
>> + // Save Size for return
>> + //
>> + ReturnValue = Size;
>> +
>> + if ((StartAddress & BIT0) != 0) {
>> + //
>> + // Read a byte if StartAddress is byte aligned
>> + //
>> + *(volatile UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
>
>Why volatile on Buffer?
I took reference from Socionext, Not required .
PCIe region has device attribute.
>
>> + StartAddress += sizeof (UINT8);
>> + Size -= sizeof (UINT8);
>> + Buffer = (UINT8*)Buffer + BIT0;
>
>sizeof (UINT8) instead of BIT0?
>
>And this is a VOID *, so can just write.
> Buffer += sizeof (UINT8);
>
Yes,Sure.
>> + }
>> +
>> + if (Size >= sizeof (UINT16) && (StartAddress & BIT1) != 0) {
>> + //
>> + // Read a word if StartAddress is word aligned
>> + //
>> + WriteUnaligned16 (Buffer, PciSegmentRead16 (StartAddress));
>> + StartAddress += sizeof (UINT16);
>> + Size -= sizeof (UINT16);
>> + Buffer = (UINT16*)Buffer + BIT0;
>
>That is a very confusing use of pointer arithmetic.
> Buffer += sizeof (UINT16);
>
Agree, alright.
>> + }
>> +
>> + while (Size >= sizeof (UINT32)) {
>> + //
>> + // Read as many double words as possible
>> + //
>> + WriteUnaligned32 (Buffer, PciSegmentRead32 (StartAddress));
>> + StartAddress += sizeof (UINT32);
>> + Size -= sizeof (UINT32);
>> + Buffer = (UINT32*)Buffer + BIT0;
>
> Buffer += sizeof (UINT32);
Ok
>
>> + }
>> +
>> + if (Size >= sizeof (UINT16)) {
>> + //
>> + // Read the last remaining word if exist
>> + //
>> + WriteUnaligned16 (Buffer, PciSegmentRead16 (StartAddress));
>> + StartAddress += sizeof (UINT16);
>> + Size -= sizeof (UINT16);
>> + Buffer = (UINT16*)Buffer + BIT0;
>
> Buffer += sizeof (UINT16);
Ok
>
>> + }
>> +
>> + if (Size >= sizeof (UINT8)) {
>> + //
>> + // Read the last remaining byte if exist
>> + //
>> + *(volatile UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
>
>There is that scary volatile again.
Not required.
>
>> + }
>> +
>> + return ReturnValue;
>> +}
>> +
>> +
>> +/**
>> + Copies the data in a caller supplied buffer to a specified range of
>> +PCI
>> + configuration space.
>> +
>> + Writes the range of PCI configuration registers specified by
>> + StartAddress and Size from the buffer specified by Buffer. This
>> + function only allows the PCI configuration registers from a single
>> + PCI function to be written. Size is returned.
>> +
>> + If any reserved bits in StartAddress are set, then ASSERT().
>> + If ((StartAddress & 0xFFF) + Size) > 0x1000, then ASSERT().
>> + If Size > 0 and Buffer is NULL, then ASSERT().
>> +
>> + @param StartAddress The starting address that encodes the PCI Segment,
>Bus,
>> + Device, Function and Register.
>> + @param Size The size in bytes of the transfer.
>> + @param Buffer The pointer to a buffer containing the data to
>> write.
>> +
>> + @return The parameter of Size.
>> +
>> +**/
>> +UINTN
>> +EFIAPI
>> +PciSegmentWriteBuffer (
>> + IN UINT64 StartAddress,
>> + IN UINTN Size,
>> + IN VOID *Buffer
>> + )
>> +{
>> + UINTN ReturnValue;
>> +
>> + ASSERT_INVALID_PCI_SEGMENT_ADDRESS (StartAddress, 0); // 0xFFF is
>> + used as limit for 4KB config space ASSERT (((StartAddress & 0xFFF)
>> + + Size) <= SIZE_4KB);
>
>Can you use (SIZE_4KB - 1) instead of 0xFFF?
Yes,Sure.
>
>> +
>> + if (Size == 0) {
>> + return Size;
>> + }
>> +
>> + ASSERT (Buffer != NULL);
>> +
>> + //
>> + // Save Size for return
>> + //
>> + ReturnValue = Size;
>> +
>> + if ((StartAddress & BIT0) != 0) {
>> + //
>> + // Write a byte if StartAddress is byte aligned
>> + //
>> + PciSegmentWrite8 (StartAddress, *(UINT8*)Buffer);
>> + StartAddress += sizeof (UINT8);
>> + Size -= sizeof (UINT8);
>> + Buffer = (UINT8*)Buffer + BIT0;
>
>Same comments for Buffer pointer update as previous function, throughout.
Ok
>
>/
> Leif
>
>> + }
>> +
>> + if (Size >= sizeof (UINT16) && (StartAddress & BIT1) != 0) {
>> + //
>> + // Write a word if StartAddress is word aligned
>> + //
>> + PciSegmentWrite16 (StartAddress, ReadUnaligned16 (Buffer));
>> + StartAddress += sizeof (UINT16);
>> + Size -= sizeof (UINT16);
>> + Buffer = (UINT16*)Buffer + BIT0;
>> + }
>> +
>> + while (Size >= sizeof (UINT32)) {
>> + //
>> + // Write as many double words as possible
>> + //
>> + PciSegmentWrite32 (StartAddress, ReadUnaligned32 (Buffer));
>> + StartAddress += sizeof (UINT32);
>> + Size -= sizeof (UINT32);
>> + Buffer = (UINT32*)Buffer + BIT0;
>> + }
>> +
>> + if (Size >= sizeof (UINT16)) {
>> + //
>> + // Write the last remaining word if exist
>> + //
>> + PciSegmentWrite16 (StartAddress, ReadUnaligned16 (Buffer));
>> + StartAddress += sizeof (UINT16);
>> + Size -= sizeof (UINT16);
>> + Buffer = (UINT16*)Buffer + BIT0;
>> + }
>> +
>> + if (Size >= sizeof (UINT8)) {
>> + //
>> + // Write the last remaining byte if exist
>> + //
>> + PciSegmentWrite8 (StartAddress, *(UINT8*)Buffer); }
>> +
>> + return ReturnValue;
>> +}
>> diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> new file mode 100644
>> index 0000000..1ac83d4
>> --- /dev/null
>> +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
>> @@ -0,0 +1,41 @@
>> +## @file
>> +# PCI Segment Library for NXP SoCs with multiple RCs # # Copyright
>> +2018 NXP # # 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 #
>https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
>rce.org%2Flicenses%2Fbsd-
>license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
>daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
>36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
>egCISP%2BU%3D&reserved=0.
>> +# 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 = 0x0001001A
>> + BASE_NAME = PciSegmentLib
>> + FILE_GUID = c9f59261-5a60-4a4c-82f6-1f520442e100
>> + MODULE_TYPE = BASE
>> + VERSION_STRING = 1.0
>> + LIBRARY_CLASS = PciSegmentLib
>> +
>> +[Sources]
>> + PciSegmentLib.c
>> +
>> +[Packages]
>> + MdePkg/MdePkg.dec
>> + Silicon/NXP/NxpQoriqLs.dec
>> +
>> +[LibraryClasses]
>> + BaseLib
>> + DebugLib
>> + IoLib
>> + PcdLib
>> +
>> +[Pcd]
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr
>> + gNxpQoriqLsTokenSpaceGuid.PcdPciExp4BaseAddr
>> --
>> 1.9.1
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
