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

Jun Nie posted 1 patch 6 years, 9 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
ArmPlatformPkg/ArmPlatformPkg.dec              |  1 +
ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +
ArmPlatformPkg/Include/Drivers/PL011Uart.h     | 29 ++++++++++++++++++++++++++
3 files changed, 31 insertions(+)
[edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset
Posted by Jun Nie 6 years, 9 months ago
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
 
   ## 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
-- 
1.9.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset
Posted by Leif Lindholm 6 years, 9 months ago
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
Re: [edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset
Posted by Jun Nie 6 years, 9 months ago
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
Re: [edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset
Posted by Ard Biesheuvel 6 years, 9 months ago
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