Re: [Qemu-devel] [PATCH 1/2] ACPI: Add definitions for the SPCR table

[Shannon Zhao]

Hi Andrew,

Some minor comments below. Use scripts/checkpatch.pl before submitting.

On 2015/6/8 22:00, Andrew Jones wrote:
> SPCR is the Serial Port Console Redirection table. See the document
> linked from http://uefi.org/acpi. For serial port types, "Interface
> Type", see the documentation for the Debug Port Table 2 (DBG2).
> 
> Signed-off-by: Andrew Jones <address@hidden>
> ---
>  include/hw/acpi/acpi-defs.h | 72 
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 59cf277434b37..e579d4c509fc8 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -197,6 +197,78 @@ enum {
>  };
>  
>  /*
> + * ACPI Serial Port Console Redirection Table
> + */
> +enum {
> +    ACPI_SERIAL_16550_COMPAT        = 0,
> +    ACPI_SERIAL_16550_SUBSET_COMPAT = 1,
> +    ACPI_SERIAL_ARM_PL011_UART      = 3,
> +};
> +
> +enum {
> +    ACPI_SERIAL_ITYPE_PC            = 1,
> +    ACPI_SERIAL_ITYPE_APIC          = 2,
> +    ACPI_SERIAL_ITYPE_SAPIC         = 4,
> +    ACPI_SERIAL_ITYPE_ARMH_GIC      = 8,
> +};
> +
> +enum {
> +    ACPI_SERIAL_BAUD_9600           = 3,
> +    ACPI_SERIAL_BAUD_19200          = 4,
> +    ACPI_SERIAL_BAUD_57600          = 6,
> +    ACPI_SERIAL_BAUD_115200         = 7,
> +};
> +
> +enum {
> +    ACPI_SERIAL_FLOW_DCD_REQUIRED   = 1,
> +    ACPI_SERIAL_FLOW_HW             = 2,
> +    ACPI_SERIAL_FLOW_SW             = 4,
> +};
> +
> +enum {
> +    ACPI_SERIAL_TERM_VT100          = 0,
> +    ACPI_SERIAL_TERM_VT100_PLUS     = 1,
> +    ACPI_SERIAL_TERM_VT_UTF8        = 2,
> +    ACPI_SERIAL_TERM_ANSI           = 3,
> +};
> +
> +struct AcpiSerialPortConsoleRedirection
> +{

open brace '{' following struct go on the same line

> +    ACPI_TABLE_HEADER_DEF            /* ACPI common table header */
> +    uint8_t  interface_type;         /* See ACPI_SERIAL_ enums */
> +    uint8_t  reserved1[3];           /* Reserved, must be 0 */
> +    struct AcpiGenericAddress base_address;
> +    uint8_t  interrupt_types;        /* Interrupt type support mask,
> +                                      * see ACPI_SERIAL_ITYPE_ enums */
> +    uint8_t  irq;                    /* PC irq used. 0 if 
> ACPI_SERIAL_ITYPE_PC
> +                                      * is not set, else must be one of
> +                                      * 2-7,9-12,14-15 */
> +    uint32_t gsi;                    /* Not valid if only 
> ACPI_SERIAL_ITYPE_PC
> +                                      * is set in the interrupt_types mask */
> +    uint8_t  baud;                   /* See ACPI_SERIAL_BAUD_ enums */
> +    uint8_t  parity;                 /* 0 = no parity, 1-255 reserved */
> +    uint8_t  stopbits;               /* Must be 1 */
> +    uint8_t  flowctrl;               /* See ACPI_SERIAL_FLOW_ enums */
> +    uint8_t  term_type;              /* See ACPI_SERIAL_TERM_ enums */
> +    uint8_t  reserved2;              /* Reserved, must be 0 */
> +    uint16_t pci_device_id;          /* 0xffff, if not a pci dev */
> +    uint16_t pci_vendor_id;          /* 0xffff, if not a pci dev */
> +    uint8_t  pci_bus;                /* PCI bus number, 0 if not a pci dev */
> +    uint8_t  pci_slot;               /* PCI slot number, 0 if not a pci dev 
> */
> +    uint8_t  pci_func;               /* PCI function num, 0 if not a pci dev 
> */
> +    uint32_t pci_flags;              /* PCI compatibility mask,
> +                                      *   bits 1-31 reserved
> +                                      *   bit0 = 0, if not a pci dev
> +                                      *   bit0 = 1, don't suppress PNP or
> +                                      *             power management
> +                                      */
> +    uint8_t  pci_seg;                /* PCI segment number, 0 for systems
> +                                        with less than 255 PCI busses */
> +    uint32_t reserved3;              /* Reserved, must be 0 */
> +} QEMU_PACKED;
> +typedef struct AcpiSerialPortConsoleRedirection 
> AcpiSerialPortConsoleRedirection;

line over 80 characters

> +
> +/*
>   * ACPI 1.0 Root System Description Table (RSDT)
>   */
>  struct AcpiRsdtDescriptorRev1
> 

-- 
Shannon