[RFC,MIPS,3/6] Initial support of VIA IDE controller used by fulong mini pc

This patch add initial support of VIA IDE controller used by fulong mini pc

Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
-----
+    pci_qdev_register_many(via_ide_info);
+}
+device_init(via_ide_register);
+
-----

On 5/9/10, chen huacai <zltjiangshi@gmail.com> wrote:
> This patch add initial support of VIA IDE controller used by fulong mini pc
>
>  Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>  -----
>  diff --git a/Makefile.objs b/Makefile.objs
>  index ecdd53e..75be9ce 100644
>  --- a/Makefile.objs
>  +++ b/Makefile.objs
>  @@ -195,6 +195,7 @@ hw-obj-$(CONFIG_IDE_ISA) += ide/isa.o
>   hw-obj-$(CONFIG_IDE_PIIX) += ide/piix.o
>   hw-obj-$(CONFIG_IDE_CMD646) += ide/cmd646.o
>   hw-obj-$(CONFIG_IDE_MACIO) += ide/macio.o
>  +hw-obj-$(CONFIG_IDE_VIA) += ide/via.o
>
>   # SCSI layer
>   hw-obj-y += lsi53c895a.o
>  diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
>  index 7793dbc..02fea47 100644
>  --- a/default-configs/mips-softmmu.mak
>  +++ b/default-configs/mips-softmmu.mak
>  @@ -18,6 +18,7 @@ CONFIG_IDE_QDEV=y
>   CONFIG_IDE_PCI=y
>   CONFIG_IDE_ISA=y
>   CONFIG_IDE_PIIX=y
>  +CONFIG_IDE_VIA=y
>   CONFIG_NE2000_ISA=y
>   CONFIG_SOUND=y
>   CONFIG_VIRTIO_PCI=y
>  diff --git a/default-configs/mips64-softmmu.mak
>  b/default-configs/mips64-softmmu.mak
>  index aa65d92..8c13da6 100644
>  --- a/default-configs/mips64-softmmu.mak
>  +++ b/default-configs/mips64-softmmu.mak
>  @@ -18,6 +18,7 @@ CONFIG_IDE_QDEV=y
>   CONFIG_IDE_PCI=y
>   CONFIG_IDE_ISA=y
>   CONFIG_IDE_PIIX=y
>  +CONFIG_IDE_VIA=y
>   CONFIG_NE2000_ISA=y
>   CONFIG_SOUND=y
>   CONFIG_VIRTIO_PCI=y
>  diff --git a/default-configs/mips64el-softmmu.mak
>  b/default-configs/mips64el-softmmu.mak
>  index b9b8c71..7e3a5ef 100644
>  --- a/default-configs/mips64el-softmmu.mak
>  +++ b/default-configs/mips64el-softmmu.mak
>  @@ -18,6 +18,7 @@ CONFIG_IDE_QDEV=y
>   CONFIG_IDE_PCI=y
>   CONFIG_IDE_ISA=y
>   CONFIG_IDE_PIIX=y
>  +CONFIG_IDE_VIA=y
>   CONFIG_NE2000_ISA=y
>   CONFIG_SOUND=y
>   CONFIG_VIRTIO_PCI=y
>  diff --git a/default-configs/mipsel-softmmu.mak
>  b/default-configs/mipsel-softmmu.mak
>  index e14831e..c329fb2 100644
>  --- a/default-configs/mipsel-softmmu.mak
>  +++ b/default-configs/mipsel-softmmu.mak
>  @@ -18,6 +18,7 @@ CONFIG_IDE_QDEV=y
>   CONFIG_IDE_PCI=y
>   CONFIG_IDE_ISA=y
>   CONFIG_IDE_PIIX=y
>  +CONFIG_IDE_VIA=y
>   CONFIG_NE2000_ISA=y
>   CONFIG_SOUND=y
>   CONFIG_VIRTIO_PCI=y
>  diff --git a/hw/ide.h b/hw/ide.h
>  index 0e7d540..bb635b6 100644
>  --- a/hw/ide.h
>  +++ b/hw/ide.h
>  @@ -12,6 +12,7 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
>                           int secondary_ide_enabled);
>   void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>   void pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>  +void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
>
>   /* ide-macio.c */
>   int pmac_ide_init (DriveInfo **hd_table, qemu_irq irq,
>  diff --git a/hw/ide/via.c b/hw/ide/via.c
>  new file mode 100644
>  index 0000000..9adc5b5
>  --- /dev/null
>  +++ b/hw/ide/via.c
>  @@ -0,0 +1,189 @@
>  +/*
>  + * QEMU IDE Emulation: PCI VIA82C686B support.
>  + *
>  + * Copyright (c) 2003 Fabrice Bellard
>  + * Copyright (c) 2006 Openedhand Ltd.
>  + * Copyright (c) 2010 Huacai Chen <zltjiangshi@gmail.com>
>  + *
>  + * Permission is hereby granted, free of charge, to any person obtaining a copy
>  + * of this software and associated documentation files (the
>  "Software"), to deal
>  + * in the Software without restriction, including without limitation the rights
>  + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>  + * copies of the Software, and to permit persons to whom the Software is
>  + * furnished to do so, subject to the following conditions:
>  + *
>  + * The above copyright notice and this permission notice shall be included in
>  + * all copies or substantial portions of the Software.
>  + *
>  + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>  + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>  + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>  + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>  + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>  ARISING FROM,
>  + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>  + * THE SOFTWARE.
>  + */
>  +#include <hw/hw.h>
>  +#include <hw/pc.h>
>  +#include <hw/pci.h>
>  +#include <hw/isa.h>
>  +#include "block.h"
>  +#include "block_int.h"
>  +#include "sysemu.h"
>  +#include "dma.h"
>  +
>  +#include <hw/ide/pci.h>
>  +
>  +static uint32_t bmdma_readb(void *opaque, uint32_t addr)
>  +{
>  +    BMDMAState *bm = opaque;
>  +    uint32_t val;
>  +
>  +    switch(addr & 3) {
>  +    case 0:
>  +        val = bm->cmd;
>  +        break;
>  +    case 2:
>  +        val = bm->status;
>  +        break;
>  +    default:
>  +        val = 0xff;
>  +        break;
>  +    }
>  +#ifdef DEBUG_IDE
>  +    printf("bmdma: readb 0x%02x : 0x%02x\n", addr, val);
>  +#endif
>  +    return val;
>  +}
>  +
>  +static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
>  +{
>  +    BMDMAState *bm = opaque;
>  +#ifdef DEBUG_IDE
>  +    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
>  +#endif
>  +    switch(addr & 3) {
>  +    case 2:
>  +        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status &
>  ~val & 0x06);
>  +        break;

Some gccs complain if there is no default case.

>  +    }
>  +}
>  +
>  +static void bmdma_map(PCIDevice *pci_dev, int region_num,
>  +                    pcibus_t addr, pcibus_t size, int type)
>  +{
>  +    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, pci_dev);
>  +    int i;
>  +
>  +    for(i = 0;i < 2; i++) {
>  +        BMDMAState *bm = &d->bmdma[i];
>  +        d->bus[i].bmdma = bm;
>  +        bm->bus = d->bus+i;
>  +        qemu_add_vm_change_state_handler(ide_dma_restart_cb, bm);
>  +
>  +        register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);
>  +
>  +        register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm);
>  +        register_ioport_read(addr, 4, 1, bmdma_readb, bm);
>  +
>  +        register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
>  +        register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
>  +        register_ioport_write(addr + 4, 4, 2, bmdma_addr_writew, bm);
>  +        register_ioport_read(addr + 4, 4, 2, bmdma_addr_readw, bm);
>  +        register_ioport_write(addr + 4, 4, 4, bmdma_addr_writel, bm);
>  +        register_ioport_read(addr + 4, 4, 4, bmdma_addr_readl, bm);
>  +        addr += 8;
>  +    }
>  +}
>  +
>  +static void via_reset(void *opaque)
>  +{
>  +    PCIIDEState *d = opaque;
>  +    uint8_t *pci_conf = d->dev.config;
>  +    int i;
>  +
>  +    for (i = 0; i < 2; i++) {
>  +        ide_bus_reset(&d->bus[i]);
>  +        ide_dma_reset(&d->bmdma[i]);
>  +    }
>  +
>  +    /* TODO: this is the default. do not override. */
>  +    pci_conf[PCI_COMMAND] = 0x80;
>  +    /* TODO: this is the default. do not override. */
>  +    pci_conf[PCI_COMMAND + 1] = 0x00;
>  +    /* TODO: use pci_set_word */
>  +    pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
>  +    pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
>  +
>  +    pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, 0x000001f0);
>  +    pci_set_long(pci_conf + PCI_BASE_ADDRESS_1, 0x000003f6);
>  +    pci_set_long(pci_conf + PCI_BASE_ADDRESS_2, 0x00000170);
>  +    pci_set_long(pci_conf + PCI_BASE_ADDRESS_3, 0x00000376);
>  +    pci_set_long(pci_conf + PCI_BASE_ADDRESS_4, 0x0000cc01); /*
>  BMIBA: 20-23h */
>  +    pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
>  +    pci_set_long(pci_conf +  PCI_INTERRUPT_LINE, 0x0000010e);
>  +    pci_set_long(pci_conf +  0x40, 0x0a090208);
>  +    pci_set_long(pci_conf +  0x44, 0x00c00068);
>  +    pci_set_long(pci_conf +  0x48, 0xa8a8a8a8);
>  +    pci_set_long(pci_conf +  0x4c, 0x000000ff);
>  +    pci_set_long(pci_conf +  0x50, 0x07070707);
>  +    pci_set_long(pci_conf +  0x54, 0x00000004);
>  +    pci_set_long(pci_conf +  0x60, 0x00000200);
>  +    pci_set_long(pci_conf +  0x68, 0x00000200);
>  +    pci_set_long(pci_conf +  0xc0, 0x00020001);
>  +}
>  +
>  +/*via ide func*/
>  +static int vt82c686b_ide_initfn(PCIDevice *dev)
>  +{
>  +    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, dev);;
>  +    uint8_t *pci_conf = d->dev.config;
>  +
>  +    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_VIA);
>  +    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_VIA_IDE);
>  +    pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
>  +    pci_config_set_prog_interface(pci_conf, 0x8a); // legacy ATA mode
>  +    pci_config_set_revision(pci_conf,0x06); /*  Revision 0.6  */
>  +    pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>  +
>  +    qemu_register_reset(via_reset, d);

Reset function can be registered with the qdev info structure.

>  +    pci_register_bar((PCIDevice *)d, 4, 0x10,
>  +                           PCI_BASE_ADDRESS_SPACE_IO, bmdma_map);
>  +
>  +    vmstate_register(0, &vmstate_ide_pci, d);

Is this correct?

Like reset, this can be registered via qdev info.

>  +
>  +    ide_bus_new(&d->bus[0], &d->dev.qdev);
>  +    ide_bus_new(&d->bus[1], &d->dev.qdev);
>  +    ide_init2(&d->bus[0], NULL, NULL, isa_reserve_irq(14));
>  +    ide_init2(&d->bus[1], NULL, NULL, isa_reserve_irq(15));
>  +    ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
>  +    ide_init_ioport(&d->bus[1], 0x170, 0x376);
>  +
>  +    return 0;
>  +}
>  +
>  +void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
>  +{
>  +    PCIDevice *dev;
>  +
>  +    dev = pci_create_simple(bus, devfn, "via-ide");
>  +    pci_ide_create_devs(dev, hd_table);
>  +}
>  +
>  +static PCIDeviceInfo via_ide_info[] = {
>  +    {
>  +        .qdev.name    = "via-ide",
>  +        .qdev.size    = sizeof(PCIIDEState),
>  +        .qdev.no_user = 1,
>  +        .init         = vt82c686b_ide_initfn,
>  +    },{
>  +        /* end of list */
>  +    }
>  +};
>  +
>  +static void via_ide_register(void)
>  +{
>  +    pci_qdev_register_many(via_ide_info);
>  +}
>  +device_init(via_ide_register);
>  +
>  -----
>  --
>
> Huacai Chen
>
>
>

Blue Swirl <blauwirbel@gmail.com> writes:

> On 5/9/10, chen huacai <zltjiangshi@gmail.com> wrote:
>> This patch add initial support of VIA IDE controller used by fulong mini pc
>>
>>  Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>>  -----
[...]
>>  diff --git a/hw/ide/via.c b/hw/ide/via.c
>>  new file mode 100644
>>  index 0000000..9adc5b5
>>  --- /dev/null
>>  +++ b/hw/ide/via.c
>>  @@ -0,0 +1,189 @@
[...]
>>  +static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
>>  +{
>>  +    BMDMAState *bm = opaque;
>>  +#ifdef DEBUG_IDE
>>  +    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
>>  +#endif
>>  +    switch(addr & 3) {
>>  +    case 2:
>>  +        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status &
>>  ~val & 0x06);
>>  +        break;
>
> Some gccs complain if there is no default case.

Are you sure?  Which version?

It warns when the switch expression's type is an enumeration, and the
switch doesn't handle all enumeration constants, and has no default
case.

I can be made to warn whenever there's no default, but we don't do that.

A quick grep finds roughly 600 switches without default case in our
code.

>>  +    }
>>  +}
[...]

>>  +    pci_register_bar((PCIDevice *)d, 4, 0x10,
>>  +                           PCI_BASE_ADDRESS_SPACE_IO, bmdma_map);
>>  +
>>  +    vmstate_register(0, &vmstate_ide_pci, d);
>
> Is this correct?
>
I think so, since ide/piix.c and ide/cmd646.c both do in this way.

On 5/11/10, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>  > On 5/9/10, chen huacai <zltjiangshi@gmail.com> wrote:
>  >> This patch add initial support of VIA IDE controller used by fulong mini pc
>  >>
>  >>  Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>  >>  -----
>  [...]
>
> >>  diff --git a/hw/ide/via.c b/hw/ide/via.c
>  >>  new file mode 100644
>  >>  index 0000000..9adc5b5
>  >>  --- /dev/null
>  >>  +++ b/hw/ide/via.c
>  >>  @@ -0,0 +1,189 @@
>
> [...]
>
> >>  +static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
>  >>  +{
>  >>  +    BMDMAState *bm = opaque;
>  >>  +#ifdef DEBUG_IDE
>  >>  +    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
>  >>  +#endif
>  >>  +    switch(addr & 3) {
>  >>  +    case 2:
>  >>  +        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status &
>  >>  ~val & 0x06);
>  >>  +        break;
>  >
>  > Some gccs complain if there is no default case.
>
>
> Are you sure?  Which version?
>
>  It warns when the switch expression's type is an enumeration, and the
>  switch doesn't handle all enumeration constants, and has no default
>  case.
>
>  I can be made to warn whenever there's no default, but we don't do that.

This problem came up with 299b520cd4092be3c53f8380b81315c33927d9d3
(fixed before commit) and 4450521668471c7685551d8c5bcc582d754e9843,
with mingw gcc and OpenBSD gccs.

$ i586-mingw32msvc-gcc -v
Using built-in specs.
Target: i586-mingw32msvc
Configured with:
/home/ron/devel/debian/mingw32/mingw32-4.2.1.dfsg/build_dir/src/gcc-4.2.1-2-dfsg/configure
-v --prefix=/usr --target=i586-mingw32msvc --enable-languages=c,c++
--enable-threads --enable-sjlj-exceptions --disable-multilib
--enable-version-specific-runtime-libs
Thread model: win32
gcc version 4.2.1-sjlj (mingw32-2)

$ gcc -v
Reading specs from /usr/lib/gcc-lib/sparc64-unknown-openbsd4.6/3.3.5/specs
Configured with:
Thread model: single
gcc version 3.3.5 (propolice)

Maybe those cases were different from this.

>
>  A quick grep finds roughly 600 switches without default case in our
>  code.
>
>  >>  +    }
>  >>  +}
>  [...]
>

Blue Swirl <blauwirbel@gmail.com> writes:

> On 5/11/10, Markus Armbruster <armbru@redhat.com> wrote:
>> Blue Swirl <blauwirbel@gmail.com> writes:
>>
>>  > On 5/9/10, chen huacai <zltjiangshi@gmail.com> wrote:
>>  >> This patch add initial support of VIA IDE controller used by fulong mini pc
>>  >>
>>  >>  Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>>  >>  -----
>>  [...]
>>
>> >>  diff --git a/hw/ide/via.c b/hw/ide/via.c
>>  >>  new file mode 100644
>>  >>  index 0000000..9adc5b5
>>  >>  --- /dev/null
>>  >>  +++ b/hw/ide/via.c
>>  >>  @@ -0,0 +1,189 @@
>>
>> [...]
>>
>> >>  +static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
>>  >>  +{
>>  >>  +    BMDMAState *bm = opaque;
>>  >>  +#ifdef DEBUG_IDE
>>  >>  +    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
>>  >>  +#endif
>>  >>  +    switch(addr & 3) {
>>  >>  +    case 2:
>>  >>  +        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status &
>>  >>  ~val & 0x06);
>>  >>  +        break;
>>  >
>>  > Some gccs complain if there is no default case.
>>
>>
>> Are you sure?  Which version?
>>
>>  It warns when the switch expression's type is an enumeration, and the
>>  switch doesn't handle all enumeration constants, and has no default
>>  case.
>>
>>  I can be made to warn whenever there's no default, but we don't do that.
>
> This problem came up with 299b520cd4092be3c53f8380b81315c33927d9d3
> (fixed before commit) and 4450521668471c7685551d8c5bcc582d754e9843,
> with mingw gcc and OpenBSD gccs.
[...]
>
> Maybe those cases were different from this.

The first commit you quoted is indeed different: the default prevents a
spurious warning: ‘context’ may be used uninitialized in this function
from some versions of gcc.  No such confusion can arise here.

In the second one, the switch expression is of enumeration type.