Re: [Qemu-devel] [PATCH pic32 v3 05/16] pic32: add file pic32_peripherals.h

[Aurelien Jarno <aurelien@aurel32.net>]

On 2015-07-05 23:14, Serge Vakulenko wrote:
> Data definitions and function declarations for simulation
> of pic32 microcontrollers.
> 
> Signed-off-by: Serge Vakulenko <serge.vakulenko@gmail.com>
> ---
>  hw/mips/pic32_peripherals.h | 210 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 210 insertions(+)
>  create mode 100644 hw/mips/pic32_peripherals.h
> 
> diff --git a/hw/mips/pic32_peripherals.h b/hw/mips/pic32_peripherals.h
> new file mode 100644
> index 0000000..4435991
> --- /dev/null
> +++ b/hw/mips/pic32_peripherals.h
> @@ -0,0 +1,210 @@
> +/*
> + * Define memory map for PIC32 microcontroller.
> + *
> + * Copyright (C) 2015 Serge Vakulenko
> + *
> + * Permission to use, copy, modify, and distribute this software
> + * and its documentation for any purpose and without fee is hereby
> + * granted, provided that the above copyright notice appear in all
> + * copies and that both that the copyright notice and this
> + * permission notice and warranty disclaimer appear in supporting
> + * documentation, and that the name of the author not be used in
> + * advertising or publicity pertaining to distribution of the
> + * software without specific, written prior permission.
> + *
> + * The author disclaim all warranties with regard to this
> + * software, including all implied warranties of merchantability
> + * and fitness.  In no event shall the author be liable for any
> + * special, indirect or consequential damages or any damages
> + * whatsoever resulting from loss of use, data or profits, whether
> + * in an action of contract, negligence or other tortious action,
> + * arising out of or in connection with the use or performance of
> + * this software.
> + */
> +#include "hw/sysbus.h"                  /* SysBusDevice */
> +#include "net/net.h"
> +
> +#define IO_MEM_SIZE     (1024*1024)     /* 1 Mbyte */
> +
> +typedef struct _uart_t uart_t;
> +typedef struct _spi_t spi_t;
> +typedef struct _sdcard_t sdcard_t;
> +typedef struct _pic32_t pic32_t;
> +typedef struct _eth_t eth_t;
> +
> +/*
> + * UART private data.
> + */
> +struct _uart_t {
> +    pic32_t     *mcu;                   /* back pointer to pic32 object */
> +    unsigned    irq;                    /* interrupt number */
> +    int         oactive;                /* output active */
> +    unsigned    sta;                    /* UxSTA address */
> +    unsigned    mode;                   /* UxMODE address */
> +    unsigned    rxbyte;                 /* received byte */
> +    CharDriverState *chr;               /* pointer to serial_hds[i] */
> +    QEMUTimer   *transmit_timer;        /* needed to delay TX interrupt */
> +};
> +
> +/*
> + * SPI private data.
> + */
> +struct _spi_t {
> +    unsigned    buf[4];                 /* transmit and receive buffer */
> +    unsigned    rfifo;                  /* read fifo counter */
> +    unsigned    wfifo;                  /* write fifo counter */
> +    unsigned    irq;                    /* interrupt numbers */
> +    unsigned    con;                    /* SPIxCON address */
> +    unsigned    stat;                   /* SPIxSTAT address */
> +};
> +
> +/*
> + * SD card private data.
> + */
> +struct _sdcard_t {
> +    const char  *name;                  /* Device name */
> +    unsigned    gpio_port;              /* GPIO port number of CS0 signal */
> +    unsigned    gpio_cs;                /* GPIO pin mask of CS0 signal */
> +    unsigned    kbytes;                 /* Disk size */
> +    int         unit;                   /* Index (sd0 or sd1) */
> +    int         fd;                     /* Image file */
> +    int         select;                 /* Selected */
> +    int         read_multiple;          /* Read-multiple mode */
> +    unsigned    blen;                   /* Block length */
> +    unsigned    wbecnt;                 /* Write block erase count */
> +    unsigned    offset;                 /* Read/write offset */
> +    unsigned    count;                  /* Byte count */
> +    unsigned    limit;                  /* Reply length */
> +    unsigned    char buf[1024 + 16];
> +};
> +
> +/*
> + * PIC32 data structure.
> + */
> +struct _pic32_t {
> +    SysBusDevice parent_obj;
> +    MIPSCPU     *cpu;                   /* back pointer to cpu object */
> +    uint32_t    *iomem;                 /* backing storage for I/O area */
> +
> +    int         board_type;             /* board variant */
> +    int         stop_on_reset;          /* halt simulation on soft reset */
> +    unsigned    syskey_unlock;          /* syskey state */
> +
> +#define NUM_UART 6                      /* number of UART ports */
> +    uart_t      uart[NUM_UART];         /* UART data */
> +
> +#define NUM_SPI 6                       /* max number of SPI ports */
> +    spi_t       spi[NUM_SPI];           /* SPI data */
> +
> +    unsigned    sdcard_spi_port;        /* SPI port number of SD card */
> +    sdcard_t    sdcard[2];              /* SD card data */
> +
> +    DeviceState *eth_dev;               /* Ethernet device */
> +    eth_t       *eth;                   /* Ethernet driver data */
> +
> +    void (*irq_raise)(pic32_t *s, int irq); /* set interrupt request */
> +    void (*irq_clear)(pic32_t *s, int irq); /* clear interrupt request */
> +};

I don't think this is the correct way of modeling the PIC32 peripherals.
Instead of modeling one "big PIC32 peripheral", you should model single
UART, Ethernet device, SPI port, etc. separately. They have their own
address space and IRQ, so each one will have their own ioread/iowrite
function.

Then you can instanciate the peripherals depending on the
micro-controller you emulate (for example 2 UART for the PIC32MX4 or 6
UART for the PIC32MX7) instead of relying on #ifdef at compilation time.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net