Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-12-04 Thread Leif Lindholm
On Tue, Dec 04, 2018 at 10:33:23AM +0100, Ard Biesheuvel wrote:
> > +#pragma pack(push, 1)
>  
>  I don't see this #pragma making any difference to the structs below,
>  can it be dropped?
> >>> 
> >>> The pragma pack is defensive. Without it, we rely on the compiler
> >>> packing structures by default and this may not happen on 64 bit
> >>> compiles.
> >> 
> >> I understand that is what the pragma does. My comment was because all of 
> >> the
> >> variables in the below structs are naturally aligned.
> >> The reason I dislike its use when effectively a no-op, is that it makes it 
> >> look like it
> >> it isn't a no-op.
> >> 
> >> If it covers a larger set of structs, some of which require the packed 
> >> attribute I'm
> >> OK with that. But I'm not a fan of adding it "just in case" without 
> >> contemplating
> >> the statement's (lack of) effect.
> >> 
> >> Regards,
> >> 
> >> Leif
> >> 
> > 
> > Makes sense. I am checking to make sure this pragma wasn't
> > included due to some observed compiler behavior on our end, since
> > this header is also used on our ARM64 work.
> > Will remove it once confirmed it is safe.
> 
> I’d prefer to keep the pragma. It doesn’t only remove padding, it
> also informs the compiler that the whole struct may appear at any
> unaligned offset.
> On 32 bit ARM, this means the compiler will
> refrain from using load/store double or load/store multiple to
> access the contents when dereferencing a pointer to such a struct,
> which would result in a crash otherwise.

OK, if it is a real concern that the structs themselves may appear
unaligned in memory, please ignore this feedback.

/
Leif


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-12-04 Thread Ard Biesheuvel


> On 4 Dec 2018, at 02:44, Chris Co  wrote:
> 
> 
> 
>> -Original Message-
>> From: Leif Lindholm 
>> Sent: Monday, December 3, 2018 1:43 AM
>> To: Chris Co 
>> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
>> Michael D Kinney 
>> Subject: Re: [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-
>> specific i.MX packages to use
>> 
>> On Sat, Dec 01, 2018 at 12:22:17AM +, Chris Co wrote:
>>>> If using EFI_ACPI prefix, these #defines really should be in edk2
>>>> MdePkg. And CSRT itself is, so that might not be a bad idea.
>>>> 
>>>>> +
>>>>> +#pragma pack(push, 1)
>>>> 
>>>> I don't see this #pragma making any difference to the structs below,
>>>> can it be dropped?
>>> 
>>> The pragma pack is defensive. Without it, we rely on the compiler
>>> packing structures by default and this may not happen on 64 bit
>>> compiles.
>> 
>> I understand that is what the pragma does. My comment was because all of the
>> variables in the below structs are naturally aligned.
>> The reason I dislike its use when effectively a no-op, is that it makes it 
>> look like it
>> it isn't a no-op.
>> 
>> If it covers a larger set of structs, some of which require the packed 
>> attribute I'm
>> OK with that. But I'm not a fan of adding it "just in case" without 
>> contemplating
>> the statement's (lack of) effect.
>> 
>> Regards,
>> 
>> Leif
>> 
> 
> Makes sense. I am checking to make sure this pragma wasn't included due to 
> some observed compiler behavior on our end, since this header is also used on 
> our ARM64 work.
> Will remove it once confirmed it is safe.


I’d prefer to keep the pragma. It doesn’t only remove padding, it also informs 
the compiler that the whole struct may appear at any unaligned offset. On 32 
bit ARM, this means the compiler will refrain from using load/store double or 
load/store multiple to access the contents when dereferencing a pointer to such 
a struct, which would result in a crash otherwise.

> 
>>> I have addressed the remaining feedback and will resubmit with v2.
>>> 
>>> Thanks,
>>> Chris
>>> 
>>>>> +//---
>>>>> +
>>>>> +- // CSRT Resource Group header 24 bytes long
>>>>> +//---
>>>>> +
>>>>> +-
>>>>> +typedef struct {
>>>>> +  UINT32 Length;
>>>>> +  UINT32 VendorID;
>>>>> +  UINT32 SubVendorId;
>>>>> +  UINT16 DeviceId;
>>>>> +  UINT16 SubdeviceId;
>>>>> +  UINT16 Revision;
>>>>> +  UINT16 Reserved;
>>>>> +  UINT32 SharedInfoLength;
>>>>> +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
>>>>> +
>>>>> +//---
>>>>> +
>>>>> +- // CSRT Resource Descriptor 12 bytes total
>>>>> +//---
>>>>> +
>>>>> +-
>>>>> +typedef struct {
>>>>> +  UINT32 Length;
>>>>> +  UINT16 ResourceType;
>>>>> +  UINT16 ResourceSubType;
>>>>> +  UINT32 UID;
>>>>> +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
>>>>> +#pragma pack (pop)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-12-03 Thread Chris Co via edk2-devel



> -Original Message-
> From: Leif Lindholm 
> Sent: Monday, December 3, 2018 1:43 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-
> specific i.MX packages to use
> 
> On Sat, Dec 01, 2018 at 12:22:17AM +, Chris Co wrote:
> > > If using EFI_ACPI prefix, these #defines really should be in edk2
> > > MdePkg. And CSRT itself is, so that might not be a bad idea.
> > >
> > > > +
> > > > +#pragma pack(push, 1)
> > >
> > > I don't see this #pragma making any difference to the structs below,
> > > can it be dropped?
> >
> > The pragma pack is defensive. Without it, we rely on the compiler
> > packing structures by default and this may not happen on 64 bit
> > compiles.
> 
> I understand that is what the pragma does. My comment was because all of the
> variables in the below structs are naturally aligned.
> The reason I dislike its use when effectively a no-op, is that it makes it 
> look like it
> it isn't a no-op.
> 
> If it covers a larger set of structs, some of which require the packed 
> attribute I'm
> OK with that. But I'm not a fan of adding it "just in case" without 
> contemplating
> the statement's (lack of) effect.
> 
> Regards,
> 
> Leif
> 

Makes sense. I am checking to make sure this pragma wasn't included due to some 
observed compiler behavior on our end, since this header is also used on our 
ARM64 work.
Will remove it once confirmed it is safe.

Thanks,
Chris

> > I have addressed the remaining feedback and will resubmit with v2.
> >
> > Thanks,
> > Chris
> >
> > > > +//---
> > > > +
> > > > +- // CSRT Resource Group header 24 bytes long
> > > > +//---
> > > > +
> > > > +-
> > > > +typedef struct {
> > > > +  UINT32 Length;
> > > > +  UINT32 VendorID;
> > > > +  UINT32 SubVendorId;
> > > > +  UINT16 DeviceId;
> > > > +  UINT16 SubdeviceId;
> > > > +  UINT16 Revision;
> > > > +  UINT16 Reserved;
> > > > +  UINT32 SharedInfoLength;
> > > > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > > > +
> > > > +//---
> > > > +
> > > > +- // CSRT Resource Descriptor 12 bytes total
> > > > +//---
> > > > +
> > > > +-
> > > > +typedef struct {
> > > > +  UINT32 Length;
> > > > +  UINT16 ResourceType;
> > > > +  UINT16 ResourceSubType;
> > > > +  UINT32 UID;
> > > > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > > > +#pragma pack (pop)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-12-03 Thread Leif Lindholm
On Sat, Dec 01, 2018 at 12:22:17AM +, Chris Co wrote:
> > If using EFI_ACPI prefix, these #defines really should be in edk2 MdePkg. 
> > And
> > CSRT itself is, so that might not be a bad idea.
> > 
> > > +
> > > +#pragma pack(push, 1)
> > 
> > I don't see this #pragma making any difference to the structs below, can it 
> > be
> > dropped?
> 
> The pragma pack is defensive. Without it, we rely on the compiler
> packing structures by default and this may not happen on 64 bit
> compiles.

I understand that is what the pragma does. My comment was because all
of the variables in the below structs are naturally aligned.
The reason I dislike its use when effectively a no-op, is that it
makes it look like it it isn't a no-op.

If it covers a larger set of structs, some of which require the packed
attribute I'm OK with that. But I'm not a fan of adding it "just in
case" without contemplating the statement's (lack of) effect.

Regards,

Leif

> I have addressed the remaining feedback and will resubmit with v2.
> 
> Thanks,
> Chris
> 
> > > +//---
> > > +- // CSRT Resource Group header 24 bytes long
> > > +//---
> > > +-
> > > +typedef struct {
> > > +  UINT32 Length;
> > > +  UINT32 VendorID;
> > > +  UINT32 SubVendorId;
> > > +  UINT16 DeviceId;
> > > +  UINT16 SubdeviceId;
> > > +  UINT16 Revision;
> > > +  UINT16 Reserved;
> > > +  UINT32 SharedInfoLength;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > > +
> > > +//---
> > > +- // CSRT Resource Descriptor 12 bytes total
> > > +//---
> > > +-
> > > +typedef struct {
> > > +  UINT32 Length;
> > > +  UINT16 ResourceType;
> > > +  UINT16 ResourceSubType;
> > > +  UINT32 UID;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > > +#pragma pack (pop)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-11-30 Thread Chris Co via edk2-devel
Hi Leif,

> -Original Message-
> From: Leif Lindholm 
> Sent: Thursday, November 1, 2018 11:20 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-
> specific i.MX packages to use
> 
> On Fri, Sep 21, 2018 at 08:26:00AM +, Chris Co wrote:
> > This adds common headers for other NXP i.MX SoC packages.
> > More specifically, this adds i.MX-generic GPIO, IoMux, and Platform
> > definitions.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > ---
> >  Silicon/NXP/iMXPlatformPkg/Include/Platform.h | 67 ++
> > Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h  | 92
> >   Silicon/NXP/iMXPlatformPkg/Include/iMXIoMux.h |
> > 24 +
> >  3 files changed, 183 insertions(+)
> >
> > diff --git a/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > new file mode 100644
> > index ..8a1e828f68ea
> > --- /dev/null
> > +++ b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > @@ -0,0 +1,67 @@
> > +/** @file
> > +*
> > +*  i.MX Platform specific defines for constructing ACPI tables
> > +*
> > +*  Copyright (c) 2018 Microsoft Corporation. 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
> > +*
> > +https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopens
> > +ource.org%2Flicenses%2Fbsd-
> license.phpdata=02%7C01%7CChristopher
> >
> +.Co%40microsoft.com%7C53adf4afe364416fab0c08d64026b170%7C72f988bf8
> 6f1
> >
> +41af91ab2d7cd011db47%7C1%7C0%7C636766932265064004sdata=lEf
> %2Bq2l
> > +nuxJ3cw%2BL91rhCTJ2e3jzWZfvgZZY8cyHswY%3Dreserved=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 _PLATFORM_IMX_H_
> > +#define _PLATFORM_IMX_H_
> > +
> > +#include 
> > +
> > +#define EFI_ACPI_OEM_ID   {'M','C','R','S','F','T'} // OEMID 6 
> > bytes
> > +#define EFI_ACPI_VENDOR_IDSIGNATURE_32('N','X','P','I')
> > +#define EFI_ACPI_CSRT_REVISION0x0005
> > +#define EFI_ACPI_5_0_CSRT_REVISION0x
> > +
> > +// Resource Descriptor Types
> > +#define EFI_ACPI_CSRT_RD_TYPE_INTERRUPT 1 #define
> > +EFI_ACPI_CSRT_RD_TYPE_TIMER 2 #define EFI_ACPI_CSRT_RD_TYPE_DMA 3
> > +#define EFI_ACPI_CSRT_RD_TYPE_CACHE 4
> > +
> > +// Resource Descriptor Subtypes
> > +#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_LINES 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_CONTROLLER 1 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_TIMER 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CHANNEL 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CONTROLLER 1 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_CACHE 0
> 
> If using EFI_ACPI prefix, these #defines really should be in edk2 MdePkg. And
> CSRT itself is, so that might not be a bad idea.
> 
> > +
> > +#pragma pack(push, 1)
> 
> I don't see this #pragma making any difference to the structs below, can it be
> dropped?
> 

The pragma pack is defensive. Without it, we rely on the compiler packing 
structures by default and this may not happen on 64 bit compiles.

I have addressed the remaining feedback and will resubmit with v2.

Thanks,
Chris

> > +//---
> > +- // CSRT Resource Group header 24 bytes long
> > +//---
> > +-
> > +typedef struct {
> > +  UINT32 Length;
> > +  UINT32 VendorID;
> > +  UINT32 SubVendorId;
> > +  UINT16 DeviceId;
> > +  UINT16 SubdeviceId;
> > +  UINT16 Revision;
> > +  UINT16 Reserved;
> > +  UINT32 SharedInfoLength;
> > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > +
> > +//---
> > +- // CSRT Resource Descriptor 12 bytes total
> > +//---
> > +-
> > +typedef struct {
&g

Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-11-01 Thread Leif Lindholm
On Fri, Sep 21, 2018 at 08:26:00AM +, Chris Co wrote:
> This adds common headers for other NXP i.MX SoC packages.
> More specifically, this adds i.MX-generic GPIO, IoMux, and
> Platform definitions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> ---
>  Silicon/NXP/iMXPlatformPkg/Include/Platform.h | 67 ++
>  Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h  | 92 
>  Silicon/NXP/iMXPlatformPkg/Include/iMXIoMux.h | 24 +
>  3 files changed, 183 insertions(+)
> 
> diff --git a/Silicon/NXP/iMXPlatformPkg/Include/Platform.h 
> b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> new file mode 100644
> index ..8a1e828f68ea
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> @@ -0,0 +1,67 @@
> +/** @file
> +*
> +*  i.MX Platform specific defines for constructing ACPI tables
> +*
> +*  Copyright (c) 2018 Microsoft Corporation. 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_IMX_H_
> +#define _PLATFORM_IMX_H_
> +
> +#include 
> +
> +#define EFI_ACPI_OEM_ID   {'M','C','R','S','F','T'} // OEMID 6 
> bytes
> +#define EFI_ACPI_VENDOR_IDSIGNATURE_32('N','X','P','I')
> +#define EFI_ACPI_CSRT_REVISION0x0005
> +#define EFI_ACPI_5_0_CSRT_REVISION0x
> +
> +// Resource Descriptor Types
> +#define EFI_ACPI_CSRT_RD_TYPE_INTERRUPT 1
> +#define EFI_ACPI_CSRT_RD_TYPE_TIMER 2
> +#define EFI_ACPI_CSRT_RD_TYPE_DMA 3
> +#define EFI_ACPI_CSRT_RD_TYPE_CACHE 4
> +
> +// Resource Descriptor Subtypes
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_LINES 0
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_CONTROLLER 1
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_TIMER 0
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CHANNEL 0
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CONTROLLER 1
> +#define EFI_ACPI_CSRT_RD_SUBTYPE_CACHE 0

If using EFI_ACPI prefix, these #defines really should be in edk2
MdePkg. And CSRT itself is, so that might not be a bad idea.

> +
> +#pragma pack(push, 1)

I don't see this #pragma making any difference to the structs below,
can it be dropped?

> +//
> +// CSRT Resource Group header 24 bytes long
> +//
> +typedef struct {
> +  UINT32 Length;
> +  UINT32 VendorID;
> +  UINT32 SubVendorId;
> +  UINT16 DeviceId;
> +  UINT16 SubdeviceId;
> +  UINT16 Revision;
> +  UINT16 Reserved;
> +  UINT32 SharedInfoLength;
> +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> +
> +//
> +// CSRT Resource Descriptor 12 bytes total
> +//
> +typedef struct {
> +  UINT32 Length;
> +  UINT16 ResourceType;
> +  UINT16 ResourceSubType;
> +  UINT32 UID;
> +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> +#pragma pack (pop)
> +
> +#endif // !_PLATFORM_IMX_H_
> diff --git a/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h 
> b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> new file mode 100644
> index ..dce01f789058
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> @@ -0,0 +1,92 @@
> +/** @file
> +*
> +*  Copyright (c) 2018 Microsoft Corporation. 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 _IMX_GPIO_H_
> +#define _IMX_GPIO_H_
> +
> +#include 
> +
> +typedef enum {
> +  IMX_GPIO_LOW = 0,
> +  IMX_GPIO_HIGH = 1
> +} IMX_GPIO_VALUE;
> +
> +typedef enum {
> +  IMX_GPIO_DIR_INPUT,
> +  IMX_GPIO_DIR_OUTPUT
> +} IMX_GPIO_DIR;
> +
> +typedef enum {
> +  IMX_GPIO_BANK1 = 1,
> +  IMX_GPIO_BANK2,
> +  IMX_GPIO_BANK3,
> +  IMX_GPIO_BANK4,
> +  IMX_GPIO_BANK5,
> +  IMX_GPIO_BANK6,
> +  IMX_GPIO_BANK7,
> +} IMX_GPIO_BANK;
> +
> +#pragma pack(push, 1)

I don't see what effect this is supposed to have, can it be dropped?

> +
> +#define GPIO_RESERVED_SIZE \
> +((FixedPcdGet32(PcdGpioBankMemoryRange) / 4) - 8)
> +
> +typedef struct {
> + 

[edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

2018-09-21 Thread Chris Co
This adds common headers for other NXP i.MX SoC packages.
More specifically, this adds i.MX-generic GPIO, IoMux, and
Platform definitions.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Christopher Co 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
---
 Silicon/NXP/iMXPlatformPkg/Include/Platform.h | 67 ++
 Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h  | 92 
 Silicon/NXP/iMXPlatformPkg/Include/iMXIoMux.h | 24 +
 3 files changed, 183 insertions(+)

diff --git a/Silicon/NXP/iMXPlatformPkg/Include/Platform.h 
b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
new file mode 100644
index ..8a1e828f68ea
--- /dev/null
+++ b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
@@ -0,0 +1,67 @@
+/** @file
+*
+*  i.MX Platform specific defines for constructing ACPI tables
+*
+*  Copyright (c) 2018 Microsoft Corporation. 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_IMX_H_
+#define _PLATFORM_IMX_H_
+
+#include 
+
+#define EFI_ACPI_OEM_ID   {'M','C','R','S','F','T'} // OEMID 6 
bytes
+#define EFI_ACPI_VENDOR_IDSIGNATURE_32('N','X','P','I')
+#define EFI_ACPI_CSRT_REVISION0x0005
+#define EFI_ACPI_5_0_CSRT_REVISION0x
+
+// Resource Descriptor Types
+#define EFI_ACPI_CSRT_RD_TYPE_INTERRUPT 1
+#define EFI_ACPI_CSRT_RD_TYPE_TIMER 2
+#define EFI_ACPI_CSRT_RD_TYPE_DMA 3
+#define EFI_ACPI_CSRT_RD_TYPE_CACHE 4
+
+// Resource Descriptor Subtypes
+#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_LINES 0
+#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_CONTROLLER 1
+#define EFI_ACPI_CSRT_RD_SUBTYPE_TIMER 0
+#define EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CHANNEL 0
+#define EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CONTROLLER 1
+#define EFI_ACPI_CSRT_RD_SUBTYPE_CACHE 0
+
+#pragma pack(push, 1)
+//
+// CSRT Resource Group header 24 bytes long
+//
+typedef struct {
+  UINT32 Length;
+  UINT32 VendorID;
+  UINT32 SubVendorId;
+  UINT16 DeviceId;
+  UINT16 SubdeviceId;
+  UINT16 Revision;
+  UINT16 Reserved;
+  UINT32 SharedInfoLength;
+} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
+
+//
+// CSRT Resource Descriptor 12 bytes total
+//
+typedef struct {
+  UINT32 Length;
+  UINT16 ResourceType;
+  UINT16 ResourceSubType;
+  UINT32 UID;
+} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
+#pragma pack (pop)
+
+#endif // !_PLATFORM_IMX_H_
diff --git a/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h 
b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
new file mode 100644
index ..dce01f789058
--- /dev/null
+++ b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
@@ -0,0 +1,92 @@
+/** @file
+*
+*  Copyright (c) 2018 Microsoft Corporation. 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 _IMX_GPIO_H_
+#define _IMX_GPIO_H_
+
+#include 
+
+typedef enum {
+  IMX_GPIO_LOW = 0,
+  IMX_GPIO_HIGH = 1
+} IMX_GPIO_VALUE;
+
+typedef enum {
+  IMX_GPIO_DIR_INPUT,
+  IMX_GPIO_DIR_OUTPUT
+} IMX_GPIO_DIR;
+
+typedef enum {
+  IMX_GPIO_BANK1 = 1,
+  IMX_GPIO_BANK2,
+  IMX_GPIO_BANK3,
+  IMX_GPIO_BANK4,
+  IMX_GPIO_BANK5,
+  IMX_GPIO_BANK6,
+  IMX_GPIO_BANK7,
+} IMX_GPIO_BANK;
+
+#pragma pack(push, 1)
+
+#define GPIO_RESERVED_SIZE \
+((FixedPcdGet32(PcdGpioBankMemoryRange) / 4) - 8)
+
+typedef struct {
+  UINT32 DR;// 0x00 GPIO data register (GPIO1_DR)
+  UINT32 GDIR;  // 0x04 GPIO direction register 
(GPIO1_GDIR)
+  UINT32 PSR;   // 0x08 GPIO pad status register 
(GPIO1_PSR)
+  UINT32 ICR1;  // 0x0C GPIO interrupt configuration 
register1 (GPIO1_ICR1)
+  UINT32 ICR2;  // 0x10 GPIO interrupt configuration 
register2 (GPIO1_ICR2)
+  UINT32 IMR;   // 0x14 GPIO interrupt mask register 
(GPIO1_IMR)
+  UINT32 ISR;   // 0x18 GPIO interrupt status register 
(GPIO1_ISR)
+