qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC][MIPS][PATCH 1/6] Initial support of bonito north


From: Blue Swirl
Subject: Re: [Qemu-devel] [RFC][MIPS][PATCH 1/6] Initial support of bonito north bridge used by fulong mini pc
Date: Mon, 10 May 2010 22:14:56 +0300

On 5/9/10, chen huacai <address@hidden> wrote:
> This patch add initial support of bonito north bridge used by fulong mini pc
>
>  Signed-off-by: Huacai Chen <address@hidden>
>  -----
>  diff --git a/Makefile.target b/Makefile.target
>  index c092900..fc4c59f 100644
>  --- a/Makefile.target
>  +++ b/Makefile.target
>  @@ -218,7 +218,7 @@ obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o
>  mips_mipssim.o
>   obj-mips-y += mips_addr.o mips_timer.o mips_int.o
>   obj-mips-y += dma.o vga.o i8259.o
>   obj-mips-y += g364fb.o jazz_led.o
>  -obj-mips-y += gt64xxx.o pckbd.o mc146818rtc.o
>  +obj-mips-y += gt64xxx.o bonito.o pckbd.o mc146818rtc.o
>   obj-mips-y += piix4.o cirrus_vga.o
>
>   obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>  diff --git a/hw/bonito.c b/hw/bonito.c
>  new file mode 100644
>  index 0000000..7d1c8eb
>  --- /dev/null
>  +++ b/hw/bonito.c
>  @@ -0,0 +1,921 @@
>  +/*
>  + * bonito north bridge support
>  + *
>  + * Copyright (c) 2008 yajin (address@hidden)
>  + * Copyright (c) 2010 Huacai Chen (address@hidden)
>  + *
>  + * This code is licensed under the GNU GPL v2.
>  + */
>  +
>  +/*
>  +fulong 2e mini pc has a bonito north bridge.

Please add '*' before fulong.

Links to chipset docs would be nice.

>  +*/
>  +#include <assert.h>
>  +
>  +#include "hw.h"
>  +#include "mips.h"
>  +#include "pci.h"
>  +#include "pc.h"
>  +
>  +
>  +typedef target_phys_addr_t pci_addr_t;
>  +#include "pci_host.h"
>  +
>  +//#define DEBUG

Please use a more specific name, like DEBUG_BONITO.

>  +#ifdef DEBUG
>  +#define dprintf(fmt, ...) fprintf(stderr, "%s: " fmt, __FUNCTION__,
>  ##__VA_ARGS__)
>  +#define PCI_DPRINTF(fmt, ...) \
>  +do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
>  +#else
>  +#define dprintf(fmt, ...)
>  +#define PCI_DPRINTF(fmt, ...)
>  +#endif

I think this macro should be named just DPRINTF.

>  +
>  +/*from linux soure code. include/asm-mips/mips-boards/bonito64.h*/
>  +#define BONITO_BOOT_BASE               0x1fc00000
>  +#define BONITO_BOOT_SIZE               0x00100000
>  +#define BONITO_BOOT_TOP                (BONITO_BOOT_BASE+BONITO_BOOT_SIZE-1)
>  +#define BONITO_FLASH_BASE              0x1c000000
>  +#define BONITO_FLASH_SIZE              0x03000000
>  +#define BONITO_FLASH_TOP               
> (BONITO_FLASH_BASE+BONITO_FLASH_SIZE-1)
>  +#define BONITO_SOCKET_BASE             0x1f800000
>  +#define BONITO_SOCKET_SIZE             0x00400000
>  +#define BONITO_SOCKET_TOP              
> (BONITO_SOCKET_BASE+BONITO_SOCKET_SIZE-1)
>  +#define BONITO_REG_BASE                0x1fe00000
>  +#define BONITO_REG_SIZE                0x00040000
>  +#define BONITO_REG_TOP                 (BONITO_REG_BASE+BONITO_REG_SIZE-1)
>  +#define BONITO_DEV_BASE                0x1ff00000
>  +#define BONITO_DEV_SIZE                0x00100000
>  +#define BONITO_DEV_TOP                 (BONITO_DEV_BASE+BONITO_DEV_SIZE-1)
>  +#define BONITO_PCILO_BASE              0x10000000
>  +#define BONITO_PCILO_BASE_VA    0xb0000000
>  +#define BONITO_PCILO_SIZE              0x0c000000
>  +#define BONITO_PCILO_TOP               
> (BONITO_PCILO_BASE+BONITO_PCILO_SIZE-1)
>  +#define BONITO_PCILO0_BASE             0x10000000
>  +#define BONITO_PCILO1_BASE             0x14000000
>  +#define BONITO_PCILO2_BASE             0x18000000
>  +#define BONITO_PCIHI_BASE              0x20000000
>  +#define BONITO_PCIHI_SIZE              0x20000000
>  +#define BONITO_PCIHI_TOP               
> (BONITO_PCIHI_BASE+BONITO_PCIHI_SIZE-1)
>  +#define BONITO_PCIIO_BASE              0x1fd00000
>  +#define BONITO_PCIIO_BASE_VA           0xbfd00000
>  +#define BONITO_PCIIO_SIZE              0x00010000
>  +#define BONITO_PCIIO_TOP               
> (BONITO_PCIIO_BASE+BONITO_PCIIO_SIZE-1)
>  +#define BONITO_PCICFG_BASE             0x1fe80000
>  +#define BONITO_PCICFG_SIZE             0x00080000
>  +#define BONITO_PCICFG_TOP              
> (BONITO_PCICFG_BASE+BONITO_PCICFG_SIZE-1)
>  +
>  +
>  +
>  +#define BONITO_PCICONFIGBASE           0x00
>  +#define BONITO_REGBASE                 0x100
>  +
>  +#define BONITO_PCICONFIG_BASE  (BONITO_PCICONFIGBASE+BONITO_REG_BASE)
>  +#define BONITO_PCICONFIG_SIZE  (0x100)
>  +
>  +#define BONITO_INTERNAL_REG_BASE  (BONITO_REGBASE+BONITO_REG_BASE)
>  +#define BONITO_INTERNAL_REG_SIZE  (0x70)
>  +
>  +#define BONITO_SPCICONFIG_BASE  (BONITO_PCICFG_BASE)
>  +#define BONITO_SPCICONFIG_SIZE  (BONITO_PCICFG_SIZE)
>  +
>  +
>  +
>  +/* 1. Bonito h/w Configuration */
>  +/* Power on register */
>  +
>  +#define BONITO_BONPONCFG               ( 0x00>>2)      /*0x100 */

Extra space between '(' and '0x00'. There should be spaces between
'0x00', '>>' and '2' as well as between '/*' and '0x100'.

In general, please pay attention to spaces, consistent use (and also
leaving the spaces out consistently) makes the code much easier to
read.

>  +#define BONITO_BONGENCFG_OFFSET         0x4
>  +#define BONITO_BONGENCFG               ( BONITO_BONGENCFG_OFFSET>>2)   
> /*0x104 */
>  +
>  +/* 2. IO & IDE configuration */
>  +#define BONITO_IODEVCFG                ( 0x08>>2)      /*0x108 */
>  +
>  +/* 3. IO & IDE configuration */
>  +#define BONITO_SDCFG                   ( 0x0c>>2)      /*0x10c */
>  +
>  +/* 4. PCI address map control */
>  +#define BONITO_PCIMAP                  ( 0x10>>2)      /*0x110 */
>  +#define BONITO_PCIMEMBASECFG           ( 0x14>>2)      /*0x114 */
>  +#define BONITO_PCIMAP_CFG              ( 0x18>>2)      /*0x118 */
>  +
>  +/* 5. ICU & GPIO regs */
>  +/* GPIO Regs - r/w */
>  +#define BONITO_GPIODATA_OFFSET          0x1c
>  +#define BONITO_GPIODATA                ( BONITO_GPIODATA_OFFSET>>2)    
> /*0x11c */
>  +#define BONITO_GPIOIE                  ( 0x20>>2)      /*0x120 */
>  +
>  +/* ICU Configuration Regs - r/w */
>  +
>  +#define BONITO_INTEDGE                 ( 0x24>>2)      /*0x124 */
>  +#define BONITO_INTSTEER                ( 0x28>>2)      /*0x128 */
>  +#define BONITO_INTPOL                  ( 0x2c>>2)      /*0x12c */
>  +
>  +/* ICU Enable Regs - IntEn & IntISR are r/o. */
>  +
>  +#define BONITO_INTENSET                ( 0x30>>2)      /*0x130 */
>  +#define BONITO_INTENCLR                ( 0x34>>2)      /*0x134 */
>  +#define BONITO_INTEN                   ( 0x38>>2)      /*0x138 */
>  +#define BONITO_INTISR                  ( 0x3c>>2)      /*0x13c */
>  +
>  +/* PCI mail boxes */
>  +
>  +#define BONITO_PCIMAIL0_OFFSET          0x40
>  +#define BONITO_PCIMAIL1_OFFSET          0x44
>  +#define BONITO_PCIMAIL2_OFFSET          0x48
>  +#define BONITO_PCIMAIL3_OFFSET          0x4c
>  +#define BONITO_PCIMAIL0                ( 0x40>>2)      /*0x140 */
>  +#define BONITO_PCIMAIL1                ( 0x44>>2)      /*0x144 */
>  +#define BONITO_PCIMAIL2                ( 0x48>>2)      /*0x148 */
>  +#define BONITO_PCIMAIL3                ( 0x4c>>2)      /*0x14c */
>  +
>  +/* 6. PCI cache */
>  +
>  +#define BONITO_PCICACHECTRL            ( 0x50>>2)      /*0x150 */
>  +#define BONITO_PCICACHETAG             ( 0x54>>2)      /*0x154 */
>  +#define BONITO_PCIBADADDR              ( 0x58>>2)      /*0x158 */
>  +#define BONITO_PCIMSTAT                ( 0x5c>>2)      /*0x15c */
>  +
>  +/* 7. other*/
>  +#define BONITO_TIMECFG         ( 0x60>>2)      /*0x160 */
>  +#define BONITO_CPUCFG          ( 0x64>>2)      /*0x164 */
>  +#define BONITO_DQCFG           ( 0x68>>2)      /*0x168 */
>  +#define BONITO_MEMSIZE         ( 0x6C>>2)      /*0x16c */
>  +
>  +
>  +#define BONITO_REGS    (0x70>>2)
>  +
>  +
>  +/* PCI config for south bridge. type 0 */
>  +#define BONITO_PCICONF_IDSEL_MASK        0xfffff800     /*[31:11] */
>  +#define BONITO_PCICONF_IDSEL_OFFSET        11
>  +#define BONITO_PCICONF_FUN_MASK        0x700    /*[10:8] */
>  +#define BONITO_PCICONF_FUN_OFFSET        8
>  +#define BONITO_PCICONF_REG_MASK        0xFC
>  +#define BONITO_PCICONF_REG_OFFSET        0
>  +
>  +
>  +/* idsel BIT = pci slot number +12 */
>  +#define PCI_SLOT_BASE              12
>  +#define PCI_IDSEL_VIA686B_BIT       (17)
>  +#define PCI_IDSEL_VIA686B       (1<<PCI_IDSEL_VIA686B_BIT)
>  +
>  +#define DEVFN(slot,fun) (((slot)<<3)+((fun)&0x7))
>  +
>  +#define PCI_ADDR(busno,devno,funno,regno)  \
>  +    ((((busno)<<16)&0xff0000) + (((devno)<<11)&0xf800) +
>  (((funno)<<8)&0x700) + (regno))
>  +
>  +
>  +typedef PCIHostState BonitoPCIState;
>  +
>  +
>  +typedef struct BonitoState
>  +{
>  +    BonitoPCIState *pci;
>  +    uint32_t regs[BONITO_REGS];
>  +
>  +    struct bonldma {
>  +        uint32_t ldmactrl;
>  +        uint32_t ldmastat;
>  +        uint32_t ldmaaddr;
>  +        uint32_t ldmago;
>  +    }bonldma;

For example here, a space should be added between '}' and 'b'.

>  +
>  +    /* Based at 1fe00300, bonito Copier */
>  +    struct boncop {
>  +        uint32_t copctrl;
>  +        uint32_t copstat;
>  +        uint32_t coppaddr;
>  +        uint32_t copgo;
>  +    }boncop;
>  +
>  +    /*north brige pci config */
>  +    uint8_t config[256];
>  +
>  +    /*Bonito registers */
>  +    target_phys_addr_t bonito_reg_start;
>  +    target_phys_addr_t bonito_reg_length;
>  +    int bonito_reg_handle;
>  +
>  +    target_phys_addr_t bonito_pciconf_start;
>  +    target_phys_addr_t bonito_pciconf_length;
>  +    int bonito_pciconf_handle;
>  +
>  +    target_phys_addr_t bonito_spciconf_start;
>  +    target_phys_addr_t bonito_spciconf_length;
>  +    int bonito_spciconf_handle;
>  +
>  +    target_phys_addr_t bonito_pciio_start;
>  +    target_phys_addr_t bonito_pciio_length;
>  +    int bonito_pciio_handle;
>  +
>  +    target_phys_addr_t bonito_localio_start;
>  +    target_phys_addr_t bonito_localio_length;
>  +    int bonito_localio_handle;
>  +
>  +    target_phys_addr_t bonito_ldma_start;
>  +    target_phys_addr_t bonito_ldma_length;
>  +    int bonito_ldma_handle;
>  +
>  +    target_phys_addr_t bonito_cop_start;
>  +    target_phys_addr_t bonito_cop_length;
>  +    int bonito_cop_handle;
>  +
>  +} BonitoState;
>  +
>  +BonitoState * bonito_state;
>  +
>  +static void bonito_writeb(void *opaque, target_phys_addr_t addr, uint32_t 
> val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_writeb %llx val %x  \n", addr, val);
>  +#endif

You should instead use the DPRINTF macro instead of the #ifdeffery.

>  +}
>  +
>  +static void bonito_writew(void *opaque, target_phys_addr_t addr, uint32_t 
> val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_writew %llx val %x  \n", addr, val);
>  +#endif
>  +}
>  +
>  +static void bonito_writel(void *opaque, target_phys_addr_t addr, uint32_t 
> val)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t saddr;
>  +    int reset = 0;
>  +
>  +    saddr = (addr - 0x100) >> 2;
>  +
>  +#ifdef DEBUG
>  +    printf("bonito_writel %llx val %x saddr %x \n", addr, val, saddr);
>  +#endif
>  +    switch (saddr) {
>  +    case BONITO_BONPONCFG:
>  +    case BONITO_IODEVCFG:
>  +    case BONITO_SDCFG:
>  +    case BONITO_PCIMAP:
>  +    case BONITO_PCIMEMBASECFG:
>  +    case BONITO_PCIMAP_CFG:
>  +    case BONITO_GPIODATA:
>  +    case BONITO_GPIOIE:
>  +    case BONITO_INTEDGE:
>  +    case BONITO_INTSTEER:
>  +    case BONITO_INTPOL:
>  +    case BONITO_PCIMAIL0:
>  +    case BONITO_PCIMAIL1:
>  +    case BONITO_PCIMAIL2:
>  +    case BONITO_PCIMAIL3:
>  +    case BONITO_PCICACHECTRL:
>  +    case BONITO_PCICACHETAG:
>  +    case BONITO_PCIBADADDR:
>  +    case BONITO_PCIMSTAT:
>  +    case BONITO_TIMECFG:
>  +    case BONITO_CPUCFG:
>  +    case BONITO_DQCFG:
>  +    case BONITO_MEMSIZE:
>  +        s->regs[saddr] = val;
>  +        break;
>  +    case BONITO_BONGENCFG:
>  +        if (!(s->regs[saddr] & 0x04) && val & 0x04) {
>  +            reset = 1; //bit 2 jump from 0 to 1 cause reset
>  +        }
>  +        s->regs[saddr] = val;
>  +        if (reset)
>  +            qemu_system_reset_request();

Please add braces in places like this.

>  +        break;
>  +    case BONITO_INTENSET:
>  +        s->regs[BONITO_INTENSET] = val;
>  +        s->regs[BONITO_INTEN] |= val;
>  +        break;
>  +    case BONITO_INTENCLR:
>  +        s->regs[BONITO_INTENCLR] = val;
>  +        s->regs[BONITO_INTEN] &= ~val;
>  +        break;
>  +    case BONITO_INTEN:
>  +    case BONITO_INTISR:
>  +#ifdef DEBUG
>  +    printf("write to readonly bonito register %x \n", saddr);
>  +#endif
>  +        break;
>  +    default:
>  +#ifdef DEBUG
>  +    printf("write to unknown bonito register %x \n", saddr);
>  +#endif
>  +        break;
>  +    }
>  +}
>  +
>  +static uint32_t bonito_readb(void *opaque, target_phys_addr_t addr)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_readb %llx  \n", addr);
>  +#endif
>  +    return 0;
>  +}
>  +
>  +static uint32_t bonito_readw(void *opaque, target_phys_addr_t addr)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_readw %llx  \n", addr);
>  +#endif
>  +    return 0;
>  +}
>  +
>  +static uint32_t bonito_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t saddr;
>  +
>  +    saddr = (addr - 0x100) >> 2;
>  +
>  +#ifdef DEBUG
>  +    printf("bonito_readl %llx  \n", addr);
>  +#endif
>  +    switch (saddr)
>  +    {
>  +    case BONITO_INTISR:
>  +        return s->regs[saddr];
>  +
>  +    default:
>  +        return s->regs[saddr];
>  +    }
>  +}
>  +
>  +static CPUWriteMemoryFunc *bonito_write[] = {

Please make all CPU*MemoryFunc structs 'const'.

>  +    &bonito_writeb,
>  +    &bonito_writew,
>  +    &bonito_writel,
>  +};
>  +
>  +static CPUReadMemoryFunc *bonito_read[] = {
>  +    &bonito_readb,
>  +    &bonito_readw,
>  +    &bonito_readl,
>  +};
>  +
>  +
>  +
>  +static void bonito_pciconf_writeb(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_writeb %llx val %x \n", addr, val);
>  +#endif
>  +}
>  +static void bonito_pciconf_writew(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_writew %llx val %x  \n", addr, val);
>  +#endif
>  +}
>  +
>  +static void bonito_pciconf_writel(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t saddr;
>  +    uint32_t *ptr = (uint32_t *) s->config;
>  +
>  +    saddr = (addr) >> 2;
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_writel %llx val %x saddr %x \n", addr, val, 
> saddr);
>  +#endif
>  +    ptr += saddr;
>  +    *ptr = val;
>  +}
>  +
>  +static uint32_t bonito_pciconf_readb(void *opaque, target_phys_addr_t addr)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_readb %llx\n", addr);
>  +#endif
>  +    return 0;
>  +}
>  +static uint32_t bonito_pciconf_readw(void *opaque, target_phys_addr_t addr)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_readw %llx\n", addr);
>  +#endif
>  +    return 0;
>  +}
>  +
>  +static uint32_t bonito_pciconf_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +
>  +    BonitoState *s = opaque;
>  +    uint32_t saddr;
>  +    uint32_t *ptr = (uint32_t *) s->config;
>  +
>  +    saddr = (addr) >> 2;
>  +#ifdef DEBUG
>  +    printf("bonito_pciconf_readl %llx\n", addr);
>  +#endif
>  +    ptr += saddr;
>  +
>  +    return (*ptr);
>  +}
>  +
>  +/*north bridge PCI configure space. 0x1fe0 0000 - 0x1fe0 00ff */
>  +static CPUWriteMemoryFunc *bonito_pciconf_write[] = {
>  +    &bonito_pciconf_writeb,
>  +    &bonito_pciconf_writew,
>  +    &bonito_pciconf_writel,
>  +};
>  +
>  +static CPUReadMemoryFunc *bonito_pciconf_read[] = {
>  +    &bonito_pciconf_readb,
>  +    &bonito_pciconf_readw,
>  +    &bonito_pciconf_readl,
>  +};
>  +
>  +
>  +

Extra lines.

>  +static uint32_t bonito_localio_readb(void *opaque, target_phys_addr_t addr)
>  +{
>  +    uint32_t val;
>  +
>  +#ifdef DEBUG
>  +    printf("bonito_localio_readb %llx\n", addr);
>  +#endif
>  +    val = cpu_inb(addr & 0xffff);
>  +    return val;
>  +}
>  +static uint32_t bonito_localio_readw(void *opaque, target_phys_addr_t addr)
>  +{
>  +    uint32_t val;
>  +
>  +#ifdef DEBUG
>  +    printf("bonito_localio_readw %llx\n", addr);
>  +#endif
>  +    val = cpu_inw(addr & 0xffff);
>  +#ifdef TARGET_WORDS_BIGENDIAN
>  +    val = bswap16(val);
>  +#endif
>  +    return val;
>  +}
>  +
>  +static uint32_t bonito_localio_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +    uint32_t val;
>  +
>  +#ifdef DEBUG
>  +    printf("bonito_localio_readl %llx\n", addr);
>  +#endif
>  +    val = cpu_inl(addr & 0xffff);
>  +#ifdef TARGET_WORDS_BIGENDIAN
>  +    val = bswap32(val);
>  +#endif
>  +    return val;
>  +}
>  +
>  +static void bonito_localio_writeb(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_localio_writeb %llx\n", addr);
>  +#endif
>  +    cpu_outb(addr & 0xffff, val);
>  +}
>  +
>  +static void bonito_localio_writew(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_localio_writew %llx\n", addr);
>  +#endif
>  +#ifdef TARGET_WORDS_BIGENDIAN
>  +    val = bswap16(val);
>  +#endif
>  +    cpu_outw(addr & 0xffff, val);
>  +}
>  +
>  +static void bonito_localio_writel(void *opaque, target_phys_addr_t addr,
>  +                                  uint32_t val)
>  +{
>  +#ifdef DEBUG
>  +    printf("bonito_localio_writel %llx\n", addr);
>  +#endif
>  +#ifdef TARGET_WORDS_BIGENDIAN
>  +    val = bswap32(val);
>  +#endif
>  +    cpu_outl(addr & 0xffff, val);
>  +}
>  +
>  +static CPUReadMemoryFunc *bonito_localio_read[] = {
>  +    &bonito_localio_readb,
>  +    &bonito_localio_readw,
>  +    &bonito_localio_readl,
>  +};
>  +
>  +static CPUWriteMemoryFunc *bonito_localio_write[] = {
>  +    &bonito_localio_writeb,
>  +    &bonito_localio_writew,
>  +    &bonito_localio_writel,
>  +};
>  +
>  +static uint32_t bonito_ldma_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +    uint32_t val;
>  +    BonitoState *s = opaque;
>  +
>  +    val = ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)];
>  +
>  +    return val;
>  +}
>  +
>  +static void bonito_ldma_writel(void *opaque, target_phys_addr_t addr,
>  +                               uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +
>  +    ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)] = val & 0xffffffff;
>  +}
>  +
>  +static CPUWriteMemoryFunc *bonito_ldma_write[] = {
>  +    &bonito_ldma_writel,
>  +    &bonito_ldma_writel,
>  +    &bonito_ldma_writel,
>  +};
>  +
>  +static CPUReadMemoryFunc *bonito_ldma_read[] = {
>  +    &bonito_ldma_readl,
>  +    &bonito_ldma_readl,
>  +    &bonito_ldma_readl,
>  +};
>  +
>  +static uint32_t bonito_cop_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +    uint32_t val;
>  +    BonitoState *s = opaque;
>  +
>  +    val = ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)];
>  +
>  +    return val;
>  +}
>  +
>  +static void bonito_cop_writel(void *opaque, target_phys_addr_t addr,
>  +                              uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +
>  +    ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)] = val & 0xffffffff;
>  +}
>  +
>  +static CPUWriteMemoryFunc *bonito_cop_write[] = {
>  +    &bonito_cop_writel,
>  +    &bonito_cop_writel,
>  +    &bonito_cop_writel,
>  +};
>  +
>  +static CPUReadMemoryFunc *bonito_cop_read[] = {
>  +    &bonito_cop_readl,
>  +    &bonito_cop_readl,
>  +    &bonito_cop_readl,
>  +};
>  +
>  +static uint32_t bonito_sbridge_pciaddr(void *opaque, target_phys_addr_t 
> addr)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t cfgaddr;
>  +    uint32_t idsel;
>  +    uint32_t devno;
>  +    uint32_t funno;
>  +    uint32_t regno;
>  +    uint32_t pciaddr;
>  +
>  +    /*support type0 pci config */
>  +    assert((s->regs[BONITO_PCIMAP_CFG] & 0x10000) == 0x0);

This assertion would be triggered if the guest writes bad values to
the register. In general, no action by the guest should cause QEMU to
abort, so assert() should be used only for cases where the assertion
would be triggered by a bug in QEMU code.

>  +
>  +    cfgaddr = addr & 0xffff;
>  +    cfgaddr |= (s->regs[BONITO_PCIMAP_CFG] & 0xffff) << 16;
>  +
>  +    idsel = (cfgaddr & BONITO_PCICONF_IDSEL_MASK) >>
>  BONITO_PCICONF_IDSEL_OFFSET;
>  +    devno = ffs(idsel) - 1;
>  +    funno = (cfgaddr & BONITO_PCICONF_FUN_MASK) >> 
> BONITO_PCICONF_FUN_OFFSET;
>  +    regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >> 
> BONITO_PCICONF_REG_OFFSET;
>  +
>  +    if (idsel == 0)
>  +    {

Time to reread CODING_STYLE?

>  +        fprintf(stderr, "error in bonito pci config address" TARGET_FMT_plx
>  +            ",pcimap_cfg=%x\n", addr, s->regs[BONITO_PCIMAP_CFG]);
>  +        exit(1);
>  +    }
>  +    pciaddr = PCI_ADDR(pci_bus_num(s->pci->bus), devno, funno, regno);
>  +#ifdef DEBUG
>  +    printf("cfgaddr %x pciaddr %x busno %x devno %d funno %d regno %d \n",
>  +        cfgaddr, pciaddr, pci_bus_num(s->pci->bus), devno, funno, regno);
>  +#endif
>  +    return pciaddr;
>  +
>  +}
>  +
>  +static void bonito_spciconf_writeb(void *opaque, target_phys_addr_t addr,
>  +                                   uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_writeb %llx val %x \n", addr, val);
>  +#endif
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /* set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    pci_data_write(s->pci->bus, s->pci->config_reg, val & 0xff, 1);
>  +
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +}
>  +
>  +static void bonito_spciconf_writew(void *opaque, target_phys_addr_t addr,
>  +                                   uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_writew %llx val %x \n", addr, val);
>  +#endif
>  +    assert((addr&0x1)==0);
>  +
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /*set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    pci_data_write(s->pci->bus, s->pci->config_reg, val, 2);
>  +
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +}
>  +
>  +static void bonito_spciconf_writel(void *opaque, target_phys_addr_t addr,
>  +                                   uint32_t val)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_writel %llx val %x \n", addr, val);
>  +#endif
>  +    assert((addr&0x3)==0);
>  +
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /* set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    pci_data_write(s->pci->bus, s->pci->config_reg, val, 4);
>  +
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +}
>  +static uint32_t bonito_spciconf_readb(void *opaque, target_phys_addr_t addr)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_readb %llx  \n", addr);
>  +#endif
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /* set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +    return pci_data_read(s->pci->bus, s->pci->config_reg, 1);
>  +}
>  +
>  +static uint32_t bonito_spciconf_readw(void *opaque, target_phys_addr_t addr)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_readw %llx  \n", addr);
>  +#endif
>  +    assert((addr&0x1)==0);
>  +
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /* set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +    return pci_data_read(s->pci->bus, s->pci->config_reg, 2);
>  +}
>  +
>  +static uint32_t bonito_spciconf_readl(void *opaque, target_phys_addr_t addr)
>  +{
>  +    BonitoState *s = opaque;
>  +    uint32_t pciaddr;
>  +#ifdef DEBUG
>  +    printf("bonito_spciconf_readl %llx  \n", addr);
>  +#endif
>  +     assert((addr&0x3)==0);
>  +
>  +    pciaddr = bonito_sbridge_pciaddr(s, addr);
>  +
>  +    /* set the pci address in s->config_reg */
>  +    s->pci->config_reg = (pciaddr) | (1u << 31);
>  +    /* clear PCI_STATUS_MASTER_ABORT and PCI_STATUS_MASTER_TARGET_ABORT */
>  +    s->config[0x07] &= 0xcf;
>  +    return pci_data_read(s->pci->bus, s->pci->config_reg, 4);
>  +}
>  +
>  +/*south bridge PCI configure space. 0x1fe8 0000 - 0x1fef ffff */
>  +static CPUWriteMemoryFunc *bonito_spciconf_write[] = {
>  +    &bonito_spciconf_writeb,
>  +    &bonito_spciconf_writew,
>  +    &bonito_spciconf_writel,
>  +};
>  +
>  +static CPUReadMemoryFunc *bonito_spciconf_read[] = {
>  +    &bonito_spciconf_readb,
>  +    &bonito_spciconf_readw,
>  +    &bonito_spciconf_readl,
>  +};
>  +
>  +
>  +#define BONITO_IRQ_BASE 32
>  +
>  +static void pci_bonito_set_irq(void *opaque, int irq_num, int level)
>  +{
>  +    qemu_irq pic = opaque;
>  +    int internal_irq = irq_num - BONITO_IRQ_BASE;
>  +
>  +    if (bonito_state->regs[BONITO_INTEDGE] & (1<<internal_irq)) {
>  +        qemu_irq_pulse(pic);
>  +    } else {   //level triggered

C99 comments shouldn't be used except for commenting out the #define
DEBUG_BONITO earlier.

>  +        if (bonito_state->regs[BONITO_INTPOL] & (1<<internal_irq))
>  +            qemu_irq_raise(pic);
>  +        else
>  +            qemu_irq_lower(pic);
>  +    }
>  +}
>  +
>  +/* map the original irq (0~3) to bonito irq (16~47, but 16~31 are unused) */
>  +static int pci_bonito_map_irq(PCIDevice * pci_dev, int irq_num)
>  +{
>  +    int slot;
>  +
>  +    slot = (pci_dev->devfn >> 3);
>  +
>  +    switch (slot) {
>  +      /* FULONG2E_VIA_SLOT, SouthBridge, IDE, USB, ACPI, AC97, MC97 */

I think the comment should be on the following or same line as the
corresponding 'case', not earlier.

>  +    case 5:
>  +        return irq_num % 4 + BONITO_IRQ_BASE;
>  +      /* FULONG2E_ATI_SLOT, VGA */
>  +    case 6:
>  +        return 4 + BONITO_IRQ_BASE;
>  +      /* FULONG2E_RTL_SLOT, RTL8139 */
>  +    case 7:
>  +        return 5 + BONITO_IRQ_BASE;
>  +      /* PCI slot 1 to 4 */
>  +    case 8 ... 12:
>  +        return (slot - 8 + irq_num) + 6 + BONITO_IRQ_BASE;
>  +      /* Unknown device, don't do any translation */
>  +    default:
>  +        return irq_num;
>  +    }
>  +}
>  +
>  +static void bonito_reset(void *opaque)
>  +{
>  +    BonitoState *s = opaque;
>  +
>  +    /* set the default value of north bridge pci config */
>  +    s->config[0x04] = 0x00;
>  +    s->config[0x05] = 0x00;
>  +    s->config[0x06] = 0x00;
>  +    s->config[0x07] = 0x00;
>  +
>  +    s->config[0x08] = 0x01;
>  +    s->config[0x09] = 0x00;
>  +    s->config[0x0a] = 0x00;
>  +    s->config[0x0b] = 0x06;
>  +
>  +    s->config[0x2c] = 0x00;
>  +    s->config[0x2d] = 0x00;
>  +    s->config[0x2e] = 0x00;
>  +    s->config[0x2f] = 0x00;
>  +
>  +    s->config[0x3c] = 0x00;
>  +    s->config[0x3d] = 0x01;
>  +    s->config[0x3e] = 0x3c;
>  +    s->config[0x3f] = 0x00;
>  +
>  +    /*set the default value of north bridge registers */
>  +
>  +    s->regs[BONITO_BONPONCFG] = 0xc40;
>  +    s->regs[BONITO_BONGENCFG] = 0x1384;
>  +    s->regs[BONITO_IODEVCFG] = 0x2bff8010;
>  +    s->regs[BONITO_SDCFG] = 0x255e0091;
>  +
>  +    s->regs[BONITO_GPIODATA] = 0x1ff;
>  +    s->regs[BONITO_GPIOIE] = 0x1ff;
>  +    s->regs[BONITO_DQCFG] = 0x8;
>  +    s->regs[BONITO_MEMSIZE] = 0x10000000;
>  +    s->regs[BONITO_PCIMAP] = 0x6140;
>  +}
>  +
>  +static uint32_t bonito_read_config(PCIDevice *d, uint32_t address, int len)
>  +{
>  +    return pci_default_read_config(d, address, len);
>  +}
>  +
>  +static void bonito_write_config(PCIDevice *d, uint32_t address, uint32_t 
> val,
>  +                                int len)
>  +{
>  +    pci_default_write_config(d, address, val, len);
>  +}
>  +
>  +PCIBus *bonito_init_2e(qemu_irq pic)
>  +{
>  +
>  +    BonitoState *s;
>  +    PCIDevice *d;
>  +
>  +    s = qemu_mallocz(sizeof(*s));
>  +    assert(s != NULL);

These asserts should be removed, qemu_mallocz will never return NULL.

>  +    s->pci = qemu_mallocz(sizeof(*s->pci));
>  +    assert(s->pci != NULL);
>  +    bonito_state = s;
>  +
>  +    /* get the north bridge pci bus */
>  +    s->pci->bus = pci_register_bus(NULL, "pci", pci_bonito_set_irq,
>  +                                   pci_bonito_map_irq, pic, 0x28, 32);
>  +
>  +    /* set the north bridge register mapping */
>  +    s->bonito_reg_handle = cpu_register_io_memory(bonito_read,
>  bonito_write, s);
>  +    s->bonito_reg_start = BONITO_INTERNAL_REG_BASE;

Usually the devices don't specify their addresses, but these are
passed from the board level.

>  +    s->bonito_reg_length = BONITO_INTERNAL_REG_SIZE;
>  +    cpu_register_physical_memory(s->bonito_reg_start,
>  +                                 s->bonito_reg_length, 
> s->bonito_reg_handle);
>  +
>  +    /* set the north bridge pci configure  mapping */
>  +    s->bonito_pciconf_handle = cpu_register_io_memory(bonito_pciconf_read,
>  +                                                      bonito_pciconf_write, 
> s);
>  +    s->bonito_pciconf_start = BONITO_PCICONFIG_BASE;
>  +    s->bonito_pciconf_length = BONITO_PCICONFIG_SIZE;
>  +    cpu_register_physical_memory(s->bonito_pciconf_start,
>  +                                 s->bonito_pciconf_length,
>  s->bonito_pciconf_handle);
>  +
>  +    /* set the south bridge pci configure  mapping */
>  +    s->bonito_spciconf_handle = cpu_register_io_memory(bonito_spciconf_read,
>  +
>  bonito_spciconf_write, s);
>  +    s->bonito_spciconf_start = BONITO_SPCICONFIG_BASE;
>  +    s->bonito_spciconf_length = BONITO_SPCICONFIG_SIZE;
>  +    cpu_register_physical_memory(s->bonito_spciconf_start,
>  +                                 s->bonito_spciconf_length,
>  s->bonito_spciconf_handle);
>  +
>  +    /* add pci local io  mapping */
>  +    s->bonito_localio_handle = cpu_register_io_memory(bonito_localio_read,
>  +                                                      bonito_localio_write, 
> s);
>  +    s->bonito_localio_start = BONITO_DEV_BASE;
>  +    s->bonito_localio_length = BONITO_DEV_SIZE;
>  +    cpu_register_physical_memory(s->bonito_localio_start,
>  +                                 s->bonito_localio_length,
>  s->bonito_localio_handle);
>  +
>  +    s->bonito_ldma_handle = cpu_register_io_memory(bonito_ldma_read,
>  +                                                   bonito_ldma_write, s);
>  +    s->bonito_ldma_start = 0xbfe00200;
>  +    s->bonito_ldma_length = 0x100;
>  +    cpu_register_physical_memory(s->bonito_ldma_start,
>  +                                 s->bonito_ldma_length, 
> s->bonito_ldma_handle);
>  +
>  +    s->bonito_cop_handle = cpu_register_io_memory(bonito_cop_read,
>  +                                                  bonito_cop_write, s);
>  +    s->bonito_cop_start = 0xbfe00300;
>  +    s->bonito_cop_length = 0x100;
>  +    cpu_register_physical_memory(s->bonito_cop_start,
>  +                                 s->bonito_cop_length, 
> s->bonito_cop_handle);
>  +
>  +    /*add PCI io space */
>  +    /*PCI IO Space  0x1fd0 0000 - 0x1fd1 0000 */
>  +    if (s->bonito_pciio_length)
>  +    {
>  +        cpu_register_physical_memory(s->bonito_pciio_start,
>  +                                     s->bonito_pciio_length,
>  IO_MEM_UNASSIGNED);

Why would this be needed?

>  +    }
>  +    /* Map new IO address */
>  +    s->bonito_pciio_start = BONITO_PCIIO_BASE;
>  +    s->bonito_pciio_length = BONITO_PCIIO_SIZE;
>  +    isa_mem_base = s->bonito_pciio_start;
>  +    isa_mmio_init(s->bonito_pciio_start, s->bonito_pciio_length, 0);
>  +
>  +    d = pci_register_device(s->pci->bus, "Bonito PCI Bus", 
> sizeof(PCIDevice),
>  +                            0, bonito_read_config, bonito_write_config);
>  +
>  +    pci_config_set_vendor_id(d->config, 0xdf53); //Bonito North Bridge
>  +    pci_config_set_device_id(d->config, 0x00d5);

Please put the above constants to hw/pci_ids.h.

>  +    pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
>  +
>  +    pci_bus_set_mem_base(s->pci->bus, 0x10000000);
>  +
>  +   /* FIXME: I do not know how to set this config reg.
>  +    * Just set it to 1<<31 because read/write need this bit */
>  +    s->pci->config_reg |= 1u << 31;

For historical reasons, PCI code in QEMU uses x86 config reg format.
There have been some attempts to fix it, but for now, it must be used.
I think you should just add the bit (convert to x86 format) whenever
the guest writes to the config space or register.

>  +    bonito_reset(s);
>  +
>  +    return s->pci->bus;
>  +}
>  +

Recent QEMU devices have been converted to use qdev. This makes the
device initialization a bit different. There are also vmstate
structures that make device save/load easy.

>  diff --git a/hw/mips.h b/hw/mips.h
>  index 30791a8..2c650f7 100644
>  --- a/hw/mips.h
>  +++ b/hw/mips.h
>  @@ -5,6 +5,9 @@
>   /* gt64xxx.c */
>   PCIBus *pci_gt64120_init(qemu_irq *pic);
>
>  +/* bonito.c */
>  +PCIBus *bonito_init_2e(qemu_irq pic);
>  +
>   /* ds1225y.c */
>   void *ds1225y_init(target_phys_addr_t mem_base, const char *filename);
>   void ds1225y_set_protection(void *opaque, int protection);
>  -----
>  --
>
> Huacai Chen
>
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]