diff mbox

[edk2,v2] ArmPlatformPkg: Support different PL011 reg offset

Message ID 1499183018-16297-1-git-send-email-jun.nie@linaro.org
State New
Headers show

Commit Message

Jun Nie July 4, 2017, 3:43 p.m. UTC 
ZTE/SanChip version pl011 has different reg offset and bit offset
for some registers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun.nie@linaro.org>

---
 ArmPlatformPkg/ArmPlatformPkg.dec              |  1 +
 ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +
 ArmPlatformPkg/Include/Drivers/PL011Uart.h     | 29 ++++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

-- 
1.9.1

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

Comments

Leif Lindholm July 5, 2017, 4:36 p.m. UTC | #1 
On Tue, Jul 04, 2017 at 11:43:38PM +0800, Jun Nie wrote:
> ZTE/SanChip version pl011 has different reg offset and bit offset

> for some registers.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Jun Nie <jun.nie@linaro.org>

> ---

>  ArmPlatformPkg/ArmPlatformPkg.dec              |  1 +

>  ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +

>  ArmPlatformPkg/Include/Drivers/PL011Uart.h     | 29 ++++++++++++++++++++++++++

>  3 files changed, 31 insertions(+)

> 

> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec

> index d756fd2..3dd613c 100644

> --- a/ArmPlatformPkg/ArmPlatformPkg.dec

> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec

> @@ -97,6 +97,7 @@

>    gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x00000020

>    gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x0000002D

>    gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x00000000|UINT32|0x0000002F

> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0


I'm basically OK with this patch, but if we have multiple variants of
this, as the Linux driver suggests, I think we're looking at something
more like PL011UartRegOffsetVariant, with a numerical value for each
special flavour.

>  

>    ## PL011 Serial Debug UART

>    gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x00000000|UINT64|0x00000030

> diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

> index 0154f3b..257fbc7 100644

> --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

> +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

> @@ -39,3 +39,4 @@

>  

>    gArmPlatformTokenSpaceGuid.PL011UartInteger

>    gArmPlatformTokenSpaceGuid.PL011UartFractional

> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset

> diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

> index d5e88e8..09d548b 100644

> --- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h

> +++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

> @@ -19,6 +19,7 @@

>  #include <Protocol/SerialIo.h>

>  

>  // PL011 Registers

> +#if !FixedPcdGet8 (PL011UartZxRegOffset)


And as such, more a test like...
#if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE

/
    Leif

>  #define UARTDR                    0x000

>  #define UARTRSR                   0x004

>  #define UARTECR                   0x004

> @@ -34,6 +35,22 @@

>  #define UARTMIS                   0x040

>  #define UARTICR                   0x044

>  #define UARTDMACR                 0x048

> +#else

> +#define UARTDR                    0x004

> +#define UARTRSR                   0x010

> +#define UARTECR                   0x010

> +#define UARTFR                    0x014

> +#define UARTIBRD                  0x024

> +#define UARTFBRD                  0x028

> +#define UARTLCR_H                 0x030

> +#define UARTCR                    0x034

> +#define UARTIFLS                  0x038

> +#define UARTIMSC                  0x040

> +#define UARTRIS                   0x044

> +#define UARTMIS                   0x048

> +#define UARTICR                   0x04c

> +#define UARTDMACR                 0x050

> +#endif

>  

>  #define UARTPID0                  0xFE0

>  #define UARTPID1                  0xFE4

> @@ -47,6 +64,7 @@

>  #define UART_STATUS_ERROR_MASK    0x0F

>  

>  // Flag reg bits

> +#if !FixedPcdGet8 (PL011UartZxRegOffset)

>  #define PL011_UARTFR_RI           (1 << 8)  // Ring indicator

>  #define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty

>  #define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full

> @@ -56,6 +74,17 @@

>  #define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect

>  #define PL011_UARTFR_DSR          (1 << 1)  // Data set ready

>  #define PL011_UARTFR_CTS          (1 << 0)  // Clear to send

> +#else

> +#define PL011_UARTFR_RI           (1 << 0)  // Ring indicator

> +#define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty

> +#define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full

> +#define PL011_UARTFR_TXFF         (1 << 5)  // Transmit FIFO full

> +#define PL011_UARTFR_RXFE         (1 << 4)  // Receive  FIFO empty

> +#define PL011_UARTFR_BUSY         (1 << 8)  // UART busy

> +#define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect

> +#define PL011_UARTFR_DSR          (1 << 3)  // Data set ready

> +#define PL011_UARTFR_CTS          (1 << 1)  // Clear to send

> +#endif

>  

>  // Flag reg bits - alternative names

>  #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE

> -- 

> 1.9.1

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Jun Nie July 6, 2017, 2:54 p.m. UTC | #2 
2017-07-06 0:36 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Tue, Jul 04, 2017 at 11:43:38PM +0800, Jun Nie wrote:

>> ZTE/SanChip version pl011 has different reg offset and bit offset

>> for some registers.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Jun Nie <jun.nie@linaro.org>

>> ---

>>  ArmPlatformPkg/ArmPlatformPkg.dec              |  1 +

>>  ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +

>>  ArmPlatformPkg/Include/Drivers/PL011Uart.h     | 29 ++++++++++++++++++++++++++

>>  3 files changed, 31 insertions(+)

>>

>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec

>> index d756fd2..3dd613c 100644

>> --- a/ArmPlatformPkg/ArmPlatformPkg.dec

>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec

>> @@ -97,6 +97,7 @@

>>    gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x00000020

>>    gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x0000002D

>>    gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x00000000|UINT32|0x0000002F

>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0

>

> I'm basically OK with this patch, but if we have multiple variants of

> this, as the Linux driver suggests, I think we're looking at something

> more like PL011UartRegOffsetVariant, with a numerical value for each

> special flavour.

>

>>

>>    ## PL011 Serial Debug UART

>>    gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x00000000|UINT64|0x00000030

>> diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>> index 0154f3b..257fbc7 100644

>> --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>> +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>> @@ -39,3 +39,4 @@

>>

>>    gArmPlatformTokenSpaceGuid.PL011UartInteger

>>    gArmPlatformTokenSpaceGuid.PL011UartFractional

>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset

>> diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>> index d5e88e8..09d548b 100644

>> --- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>> +++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>> @@ -19,6 +19,7 @@

>>  #include <Protocol/SerialIo.h>

>>

>>  // PL011 Registers

>> +#if !FixedPcdGet8 (PL011UartZxRegOffset)

>

> And as such, more a test like...

> #if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE


Yes, it is more generic. Which header is proper file to add value
definition of  PL011_VARIANT_ZTE that can be included by platform.dsc?

Jun

>

> /

>     Leif

>

>>  #define UARTDR                    0x000

>>  #define UARTRSR                   0x004

>>  #define UARTECR                   0x004

>> @@ -34,6 +35,22 @@

>>  #define UARTMIS                   0x040

>>  #define UARTICR                   0x044

>>  #define UARTDMACR                 0x048

>> +#else

>> +#define UARTDR                    0x004

>> +#define UARTRSR                   0x010

>> +#define UARTECR                   0x010

>> +#define UARTFR                    0x014

>> +#define UARTIBRD                  0x024

>> +#define UARTFBRD                  0x028

>> +#define UARTLCR_H                 0x030

>> +#define UARTCR                    0x034

>> +#define UARTIFLS                  0x038

>> +#define UARTIMSC                  0x040

>> +#define UARTRIS                   0x044

>> +#define UARTMIS                   0x048

>> +#define UARTICR                   0x04c

>> +#define UARTDMACR                 0x050

>> +#endif

>>

>>  #define UARTPID0                  0xFE0

>>  #define UARTPID1                  0xFE4

>> @@ -47,6 +64,7 @@

>>  #define UART_STATUS_ERROR_MASK    0x0F

>>

>>  // Flag reg bits

>> +#if !FixedPcdGet8 (PL011UartZxRegOffset)

>>  #define PL011_UARTFR_RI           (1 << 8)  // Ring indicator

>>  #define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty

>>  #define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full

>> @@ -56,6 +74,17 @@

>>  #define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect

>>  #define PL011_UARTFR_DSR          (1 << 1)  // Data set ready

>>  #define PL011_UARTFR_CTS          (1 << 0)  // Clear to send

>> +#else

>> +#define PL011_UARTFR_RI           (1 << 0)  // Ring indicator

>> +#define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty

>> +#define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full

>> +#define PL011_UARTFR_TXFF         (1 << 5)  // Transmit FIFO full

>> +#define PL011_UARTFR_RXFE         (1 << 4)  // Receive  FIFO empty

>> +#define PL011_UARTFR_BUSY         (1 << 8)  // UART busy

>> +#define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect

>> +#define PL011_UARTFR_DSR          (1 << 3)  // Data set ready

>> +#define PL011_UARTFR_CTS          (1 << 1)  // Clear to send

>> +#endif

>>

>>  // Flag reg bits - alternative names

>>  #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE

>> --

>> 1.9.1

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel July 6, 2017, 2:56 p.m. UTC | #3 
On 6 July 2017 at 15:54, Jun Nie <jun.nie@linaro.org> wrote:
> 2017-07-06 0:36 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>:

>> On Tue, Jul 04, 2017 at 11:43:38PM +0800, Jun Nie wrote:

>>> ZTE/SanChip version pl011 has different reg offset and bit offset

>>> for some registers.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>

>>> ---

>>>  ArmPlatformPkg/ArmPlatformPkg.dec              |  1 +

>>>  ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +

>>>  ArmPlatformPkg/Include/Drivers/PL011Uart.h     | 29 ++++++++++++++++++++++++++

>>>  3 files changed, 31 insertions(+)

>>>

>>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec

>>> index d756fd2..3dd613c 100644

>>> --- a/ArmPlatformPkg/ArmPlatformPkg.dec

>>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec

>>> @@ -97,6 +97,7 @@

>>>    gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x00000020

>>>    gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x0000002D

>>>    gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x00000000|UINT32|0x0000002F

>>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0

>>

>> I'm basically OK with this patch, but if we have multiple variants of

>> this, as the Linux driver suggests, I think we're looking at something

>> more like PL011UartRegOffsetVariant, with a numerical value for each

>> special flavour.

>>

>>>

>>>    ## PL011 Serial Debug UART

>>>    gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x00000000|UINT64|0x00000030

>>> diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>>> index 0154f3b..257fbc7 100644

>>> --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>>> +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf

>>> @@ -39,3 +39,4 @@

>>>

>>>    gArmPlatformTokenSpaceGuid.PL011UartInteger

>>>    gArmPlatformTokenSpaceGuid.PL011UartFractional

>>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset

>>> diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>>> index d5e88e8..09d548b 100644

>>> --- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>>> +++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h

>>> @@ -19,6 +19,7 @@

>>>  #include <Protocol/SerialIo.h>

>>>

>>>  // PL011 Registers

>>> +#if !FixedPcdGet8 (PL011UartZxRegOffset)

>>

>> And as such, more a test like...

>> #if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE

>

> Yes, it is more generic. Which header is proper file to add value

> definition of  PL011_VARIANT_ZTE that can be included by platform.dsc?

>


Please look at

gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType

for an example. It is fine to have the C level #defines in PL011Uart.h
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index d756fd2..3dd613c 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -97,6 +97,7 @@ 
   gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x00000020
   gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x0000002D
   gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x00000000|UINT32|0x0000002F
+  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0
 
   ## PL011 Serial Debug UART
   gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x00000000|UINT64|0x00000030
diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
index 0154f3b..257fbc7 100644
--- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
+++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
@@ -39,3 +39,4 @@ 
 
   gArmPlatformTokenSpaceGuid.PL011UartInteger
   gArmPlatformTokenSpaceGuid.PL011UartFractional
+  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset
diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
index d5e88e8..09d548b 100644
--- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h
+++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
@@ -19,6 +19,7 @@ 
 #include <Protocol/SerialIo.h>
 
 // PL011 Registers
+#if !FixedPcdGet8 (PL011UartZxRegOffset)
 #define UARTDR                    0x000
 #define UARTRSR                   0x004
 #define UARTECR                   0x004
@@ -34,6 +35,22 @@ 
 #define UARTMIS                   0x040
 #define UARTICR                   0x044
 #define UARTDMACR                 0x048
+#else
+#define UARTDR                    0x004
+#define UARTRSR                   0x010
+#define UARTECR                   0x010
+#define UARTFR                    0x014
+#define UARTIBRD                  0x024
+#define UARTFBRD                  0x028
+#define UARTLCR_H                 0x030
+#define UARTCR                    0x034
+#define UARTIFLS                  0x038
+#define UARTIMSC                  0x040
+#define UARTRIS                   0x044
+#define UARTMIS                   0x048
+#define UARTICR                   0x04c
+#define UARTDMACR                 0x050
+#endif
 
 #define UARTPID0                  0xFE0
 #define UARTPID1                  0xFE4
@@ -47,6 +64,7 @@ 
 #define UART_STATUS_ERROR_MASK    0x0F
 
 // Flag reg bits
+#if !FixedPcdGet8 (PL011UartZxRegOffset)
 #define PL011_UARTFR_RI           (1 << 8)  // Ring indicator
 #define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty
 #define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full
@@ -56,6 +74,17 @@ 
 #define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect
 #define PL011_UARTFR_DSR          (1 << 1)  // Data set ready
 #define PL011_UARTFR_CTS          (1 << 0)  // Clear to send
+#else
+#define PL011_UARTFR_RI           (1 << 0)  // Ring indicator
+#define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty
+#define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full
+#define PL011_UARTFR_TXFF         (1 << 5)  // Transmit FIFO full
+#define PL011_UARTFR_RXFE         (1 << 4)  // Receive  FIFO empty
+#define PL011_UARTFR_BUSY         (1 << 8)  // UART busy
+#define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect
+#define PL011_UARTFR_DSR          (1 << 3)  // Data set ready
+#define PL011_UARTFR_CTS          (1 << 1)  // Clear to send
+#endif
 
 // Flag reg bits - alternative names
 #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE