Star,

There are a few comments included below for the SerialPortSetAttributes() 
function header comment block.  

With those comment block fixes: 

Reviewed-by: Michael Kinney <michael.d.kin...@intel.com>

Best regards,

Mike    

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
> Zeng
> Sent: Tuesday, November 17, 2015 3:07 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Tian, Feng 
> <feng.t...@intel.com>; Gao, Liming <liming....@intel.com>
> Subject: [edk2] [PATCH V2 03/12] MdeModulePkg BaseSerialPortLib16550: Add 
> GetControl/SetControl/SetAttributes implementation
> 
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Cc: Feng Tian <feng.t...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.z...@intel.com>
> ---
>  .../BaseSerialPortLib16550.c                       | 341 
> +++++++++++++++++++++
>  1 file changed, 341 insertions(+)
> 
> diff --git 
> a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> index 3209115..03c9960 100644
> --- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> +++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
> +++ .c
> @@ -40,7 +40,9 @@
>  #define R_UART_LCR            3
>  #define   B_UART_LCR_DLAB     BIT7
>  #define R_UART_MCR            4
> +#define   B_UART_MCR_DTRC     BIT0
>  #define   B_UART_MCR_RTS      BIT1
> +#define   B_UART_MCR_LME      BIT4
>  #define R_UART_LSR            5
>  #define   B_UART_LSR_RXRDY    BIT0
>  #define   B_UART_LSR_TXRDY    BIT5
> @@ -48,6 +50,8 @@
>  #define R_UART_MSR            6
>  #define   B_UART_MSR_CTS      BIT4
>  #define   B_UART_MSR_DSR      BIT5
> +#define   B_UART_MSR_RI       BIT6
> +#define   B_UART_MSR_DCD      BIT7
> 
>  //
>  // 4-byte structure for each PCI node in PcdSerialPciDeviceInfo @@ -761,3 
> +765,340 @@ SerialPortPoll (
> 
>    return FALSE;
>  }
> +
> +/**
> +  Sets the control bits on a serial device.
> +
> +  @param Control                Sets the bits of Control that are settable.
> +
> +  @retval RETURN_SUCCESS        The new control bits were set on the serial 
> device.
> +  @retval RETURN_UNSUPPORTED    The serial device does not support this 
> operation.
> +  @retval RETURN_DEVICE_ERROR   The serial device is not functioning 
> correctly.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SerialPortSetControl (
> +  IN UINT32 Control
> +  )
> +{
> +  UINTN SerialRegisterBase;
> +  UINT8 Mcr;
> +
> +  //
> +  // First determine the parameter is invalid.
> +  //
> +  if ((Control & (~(EFI_SERIAL_REQUEST_TO_SEND | 
> EFI_SERIAL_DATA_TERMINAL_READY |
> +                    EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE | 
> EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE |
> +                    EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE))) != 0) {
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  SerialRegisterBase = GetSerialRegisterBase ();  if
> + (SerialRegisterBase ==0) {
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Read the Modem Control Register.
> +  //
> +  Mcr = SerialPortReadRegister (SerialRegisterBase, R_UART_MCR);  Mcr
> + &= (~(B_UART_MCR_DTRC | B_UART_MCR_RTS | B_UART_MCR_LME));
> +
> +  if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == 
> EFI_SERIAL_DATA_TERMINAL_READY) {
> +    Mcr |= B_UART_MCR_DTRC;
> +  }
> +
> +  if ((Control & EFI_SERIAL_REQUEST_TO_SEND) == EFI_SERIAL_REQUEST_TO_SEND) {
> +    Mcr |= B_UART_MCR_RTS;
> +  }
> +
> +  if ((Control & EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE) == 
> EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE) {
> +    Mcr |= B_UART_MCR_LME;
> +  }
> +
> +  //
> +  // Write the Modem Control Register.
> +  //
> +  SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, Mcr);
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Retrieve the status of the control bits on a serial device.
> +
> +  @param Control                A pointer to return the current control 
> signals from the serial device.
> +
> +  @retval RETURN_SUCCESS        The control bits were read from the serial 
> device.
> +  @retval RETURN_UNSUPPORTED    The serial device does not support this 
> operation.
> +  @retval RETURN_DEVICE_ERROR   The serial device is not functioning 
> correctly.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SerialPortGetControl (
> +  OUT UINT32 *Control
> +  )
> +{
> +  UINTN SerialRegisterBase;
> +  UINT8 Msr;
> +  UINT8 Mcr;
> +  UINT8 Lsr;
> +
> +  SerialRegisterBase = GetSerialRegisterBase ();  if
> + (SerialRegisterBase ==0) {
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  *Control = 0;
> +
> +  //
> +  // Read the Modem Status Register.
> +  //
> +  Msr = SerialPortReadRegister (SerialRegisterBase, R_UART_MSR);
> +
> +  if ((Msr & B_UART_MSR_CTS) == B_UART_MSR_CTS) {
> +    *Control |= EFI_SERIAL_CLEAR_TO_SEND;  }
> +
> +  if ((Msr & B_UART_MSR_DSR) == B_UART_MSR_DSR) {
> +    *Control |= EFI_SERIAL_DATA_SET_READY;  }
> +
> +  if ((Msr & B_UART_MSR_RI) == B_UART_MSR_RI) {
> +    *Control |= EFI_SERIAL_RING_INDICATE;  }
> +
> +  if ((Msr & B_UART_MSR_DCD) == B_UART_MSR_DCD) {
> +    *Control |= EFI_SERIAL_CARRIER_DETECT;  }
> +
> +  //
> +  // Read the Modem Control Register.
> +  //
> +  Mcr = SerialPortReadRegister (SerialRegisterBase, R_UART_MCR);
> +
> +  if ((Mcr & B_UART_MCR_DTRC) == B_UART_MCR_DTRC) {
> +    *Control |= EFI_SERIAL_DATA_TERMINAL_READY;  }
> +
> +  if ((Mcr & B_UART_MCR_RTS) == B_UART_MCR_RTS) {
> +    *Control |= EFI_SERIAL_REQUEST_TO_SEND;  }
> +
> +  if ((Mcr & B_UART_MCR_LME) == B_UART_MCR_LME) {
> +    *Control |= EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE;
> +  }
> +
> +  if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
> +    *Control |= EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE;
> +  }
> +
> +  //
> +  // Read the Line Status Register.
> +  //
> +  Lsr = SerialPortReadRegister (SerialRegisterBase, R_UART_LSR);
> +
> +  if ((Lsr & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) == (B_UART_LSR_TEMT | 
> B_UART_LSR_TXRDY)) {
> +    *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY;  }
> +
> +  if ((Lsr & B_UART_LSR_RXRDY) == 0) {
> +    *Control |= EFI_SERIAL_INPUT_BUFFER_EMPTY;  }
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Sets the baud rate, receive FIFO depth, transmit/receice time out,
> +parity,
> +  data buts, and stop bits on a serial device.

Typo in comment above.  Should be "data bits".

> +
> +  @param BaudRate           The requested baud rate. A BaudRate value of 0 
> will use the
> +                            device's default interface speed.
> +  @param ReveiveFifoDepth   The requested depth of the FIFO on the receive 
> side of the
> +                            serial interface. A ReceiveFifoDepth value of 0 
> will use
> +                            the device's default FIFO depth.
> +  @param Timeout            The requested time out for a single character in 
> microseconds.
> +                            This timeout applies to both the transmit and 
> receive side of the
> +                            interface. A Timeout value of 0 will use the 
> device's default time
> +                            out value.
> +  @param Parity             The type of parity to use on this serial device. 
> A Parity value of
> +                            DefaultParity will use the device's default 
> parity value.
> +  @param DataBits           The number of data bits to use on the serial 
> device. A DataBits
> +                            vaule of 0 will use the device's default data 
> bit setting.
> +  @param StopBits           The number of stop bits to use on this serial 
> device. A StopBits
> +                            value of DefaultStopBits will use the device's 
> default number of
> +                            stop bits.

The parameters above are declared as IN OUT in function prototype below, and 
the parameters
are updated with the value actually set, but the description of the parameters 
above does not
describe that the parameter value is updated and returned to the caller.  
Please update 
parameter description for all IN OUT parameters to describe the OUT behavior.

> +
> +  @retval RETURN_SUCCESS            The new attributes were set on the 
> serial device.
> +  @retval RETURN_UNSUPPORTED        The serial device does not support this 
> operation.
> +  @retval RETURN_INVALID_PARAMETER  One or more of the attributes has an 
> unsupported value.
> +  @retval RETURN_DEVICE_ERROR       The serial device is not functioning 
> correctly.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SerialPortSetAttributes (
> +  IN OUT UINT64             *BaudRate,
> +  IN OUT UINT32             *ReceiveFifoDepth,
> +  IN OUT UINT32             *Timeout,
> +  IN OUT EFI_PARITY_TYPE    *Parity,
> +  IN OUT UINT8              *DataBits,
> +  IN OUT EFI_STOP_BITS_TYPE *StopBits
> +  )
> +{
> +  UINTN     SerialRegisterBase;
> +  UINT32    SerialBaudRate;
> +  UINTN     Divisor;
> +  UINT8     Lcr;
> +  UINT8     LcrData;
> +  UINT8     LcrParity;
> +  UINT8     LcrStop;
> +
> +  SerialRegisterBase = GetSerialRegisterBase ();  if
> + (SerialRegisterBase ==0) {
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Check for default settings and fill in actual values.
> +  //
> +  if (*BaudRate == 0) {
> +    *BaudRate = PcdGet32 (PcdSerialBaudRate);  }  SerialBaudRate =
> + (UINT32) *BaudRate;
> +
> +  if (*DataBits == 0) {
> +    LcrData = (UINT8) (PcdGet8 (PcdSerialLineControl) & 0x3);
> +    *DataBits = LcrData + 5;
> +  } else {
> +    if ((*DataBits < 5) || (*DataBits > 8)) {
> +      return RETURN_INVALID_PARAMETER;
> +    }
> +    //
> +    // Map 5..8 to 0..3
> +    //
> +    LcrData = (UINT8) (*DataBits - (UINT8) 5);  }
> +
> +  if (*Parity == DefaultParity) {
> +    LcrParity = (UINT8) ((PcdGet8 (PcdSerialLineControl) >> 3) & 0x7);
> +    switch (LcrParity) {
> +      case 0:
> +        *Parity = NoParity;
> +        break;
> +
> +      case 3:
> +        *Parity = EvenParity;
> +        break;
> +
> +      case 1:
> +        *Parity = OddParity;
> +        break;
> +
> +      case 7:
> +        *Parity = SpaceParity;
> +        break;
> +
> +      case 5:
> +        *Parity = MarkParity;
> +        break;
> +
> +      default:
> +        break;
> +    }
> +  } else {
> +    if ((*Parity < NoParity) || (*Parity > SpaceParity)) {
> +      return RETURN_INVALID_PARAMETER;
> +    }
> +    switch (*Parity) {
> +      case NoParity:
> +        LcrParity = 0;
> +        break;
> +
> +      case EvenParity:
> +        LcrParity = 3;
> +        break;
> +
> +      case OddParity:
> +        LcrParity = 1;
> +        break;
> +
> +      case SpaceParity:
> +        LcrParity = 7;
> +        break;
> +
> +      case MarkParity:
> +        LcrParity = 5;
> +        break;
> +
> +      default:
> +        break;
> +    }
> +  }
> +
> +  if (*StopBits == DefaultStopBits) {
> +    LcrStop = (UINT8) ((PcdGet8 (PcdSerialLineControl) >> 2) & 0x1);
> +    switch (LcrStop) {
> +      case 0:
> +        *StopBits = OneStopBit;
> +        break;
> +
> +      case 1:
> +        if (*DataBits == 5) {
> +          *StopBits = OneFiveStopBits;
> +        } else {
> +          *StopBits = TwoStopBits;
> +        }
> +        break;
> +
> +      default:
> +        break;
> +    }
> +  } else {
> +    if ((*StopBits < OneStopBit) || (*StopBits > TwoStopBits)) {
> +      return RETURN_INVALID_PARAMETER;
> +    }
> +    switch (*StopBits) {
> +      case OneStopBit:
> +        LcrStop = 0;
> +        break;
> +
> +      case OneFiveStopBits:
> +      case TwoStopBits:
> +        LcrStop = 1;
> +        break;
> +
> +      default:
> +        break;
> +    }
> +  }
> +
> +  //
> +  // Calculate divisor for baud generator
> +  //    Ref_Clk_Rate / Baud_Rate / 16
> +  //
> +  Divisor = PcdGet32 (PcdSerialClockRate) / (SerialBaudRate * 16);  if
> + ((PcdGet32 (PcdSerialClockRate) % (SerialBaudRate * 16)) >= SerialBaudRate 
> * 8) {
> +    Divisor++;
> +  }
> +
> +  //
> +  // Configure baud rate
> +  //
> +  SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR,
> + B_UART_LCR_DLAB);  SerialPortWriteRegister (SerialRegisterBase,
> + R_UART_BAUD_HIGH, (UINT8) (Divisor >> 8));  SerialPortWriteRegister
> + (SerialRegisterBase, R_UART_BAUD_LOW, (UINT8) (Divisor & 0xff));
> +
> +  //
> +  // Clear DLAB and configure Data Bits, Parity, and Stop Bits.
> +  // Strip reserved bits from line control value  //  Lcr = (UINT8)
> + ((LcrParity << 3) | (LcrStop << 2) | LcrData);
> + SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, (UINT8) (Lcr
> + & 0x3F));
> +
> +  return RETURN_SUCCESS;
> +}
> +
> --
> 1.9.5.msysgit.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to