Re: [Qemu-devel] [PATCH target-arm v1 10/15] char: cadence_uart: Split state struct and type into header

[Peter Crosthwaite <peter.crosthwaite@xilinx.com>]

On Thu, Feb 26, 2015 at 7:26 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> To allow using the device with modern SoC programming conventions. The
>> state struct needs to be visible to embed the device in SoC containers.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  hw/char/cadence_uart.c         | 29 +----------------------------
>>  include/hw/char/cadence_uart.h | 35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+), 28 deletions(-)
>>  create mode 100644 include/hw/char/cadence_uart.h
>>
>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> index 23f548d..4509e01 100644
>> --- a/hw/char/cadence_uart.c
>> +++ b/hw/char/cadence_uart.c
>> @@ -16,9 +16,7 @@
>>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> -#include "hw/sysbus.h"
>> -#include "sysemu/char.h"
>> -#include "qemu/timer.h"
>> +#include "hw/char/cadence_uart.h"
>>
>>  #ifdef CADENCE_UART_ERR_DEBUG
>>  #define DB_PRINT(...) do { \
>> @@ -85,8 +83,6 @@
>>  #define LOCAL_LOOPBACK         (0x2 << UART_MR_CHMODE_SH)
>>  #define REMOTE_LOOPBACK        (0x3 << UART_MR_CHMODE_SH)
>>
>> -#define CADENCE_UART_RX_FIFO_SIZE           16
>> -#define CADENCE_UART_TX_FIFO_SIZE           16
>>  #define UART_INPUT_CLK         50000000
>>
>>  #define R_CR       (0x00/4)
>> @@ -108,29 +104,6 @@
>>  #define R_PWID     (0x40/4)
>>  #define R_TTRIG    (0x44/4)
>>
>> -#define CADENCE_UART_R_MAX (0x48/4)
>> -
>> -#define TYPE_CADENCE_UART "cadence_uart"
>> -#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
>> -                                       TYPE_CADENCE_UART)
>> -
>> -typedef struct {
>> -    /*< private >*/
>> -    SysBusDevice parent_obj;
>> -    /*< public >*/
>> -
>> -    MemoryRegion iomem;
>
> The same nit pick as the one I had for gem applies here as well.
> Although it is so minor.
>

Fixed

> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>

Thanks,

Regards,
Peter

> Thanks,
>
> Alistair
>
>> -    uint32_t r[CADENCE_UART_R_MAX];
>> -    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
>> -    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
>> -    uint32_t rx_wpos;
>> -    uint32_t rx_count;
>> -    uint32_t tx_count;
>> -    uint64_t char_tx_time;
>> -    CharDriverState *chr;
>> -    qemu_irq irq;
>> -    QEMUTimer *fifo_trigger_handle;
>> -} CadenceUARTState;
>>
>>  static void uart_update_status(CadenceUARTState *s)
>>  {
>> diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
>> new file mode 100644
>> index 0000000..0404785
>> --- /dev/null
>> +++ b/include/hw/char/cadence_uart.h
>> @@ -0,0 +1,35 @@
>> +#ifndef CADENCE_UART_H_
>> +
>> +#include "hw/sysbus.h"
>> +#include "sysemu/char.h"
>> +#include "qemu/timer.h"
>> +
>> +#define CADENCE_UART_RX_FIFO_SIZE           16
>> +#define CADENCE_UART_TX_FIFO_SIZE           16
>> +
>> +#define CADENCE_UART_R_MAX (0x48/4)
>> +
>> +#define TYPE_CADENCE_UART "cadence_uart"
>> +#define CADENCE_UART(obj) OBJECT_CHECK(CadenceUARTState, (obj), \
>> +                                       TYPE_CADENCE_UART)
>> +
>> +typedef struct {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    MemoryRegion iomem;
>> +    uint32_t r[CADENCE_UART_R_MAX];
>> +    uint8_t rx_fifo[CADENCE_UART_RX_FIFO_SIZE];
>> +    uint8_t tx_fifo[CADENCE_UART_TX_FIFO_SIZE];
>> +    uint32_t rx_wpos;
>> +    uint32_t rx_count;
>> +    uint32_t tx_count;
>> +    uint64_t char_tx_time;
>> +    CharDriverState *chr;
>> +    qemu_irq irq;
>> +    QEMUTimer *fifo_trigger_handle;
>> +} CadenceUARTState;
>> +
>> +#define CADENCE_UART_H_
>> +#endif
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>