From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LvTK1-0004gn-Q4 for qemu-devel@nongnu.org; Sun, 19 Apr 2009 05:28:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LvTK0-0004gX-HG for qemu-devel@nongnu.org; Sun, 19 Apr 2009 05:28:21 -0400 Received: from [199.232.76.173] (port=40560 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LvTK0-0004gJ-8M for qemu-devel@nongnu.org; Sun, 19 Apr 2009 05:28:20 -0400 Received: from hall.aurel32.net ([88.191.82.174]:56337) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LvTJz-00012B-Hb for qemu-devel@nongnu.org; Sun, 19 Apr 2009 05:28:20 -0400 Received: from volta.aurel32.net ([2002:52e8:2fb:1:21e:8cff:feb0:693b]) by hall.aurel32.net with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1LvTJy-0004Qs-2V for qemu-devel@nongnu.org; Sun, 19 Apr 2009 11:28:18 +0200 Received: from aurel32 by volta.aurel32.net with local (Exim 4.69) (envelope-from ) id 1LvTJx-0003pb-9n for qemu-devel@nongnu.org; Sun, 19 Apr 2009 11:28:17 +0200 Date: Sun, 19 Apr 2009 11:28:17 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] sh4 r2d system emulation - warning: could not add USB device keyboard Message-ID: <20090419092817.GC5483@volta.aurel32.net> References: <49EA9F88.9000302@juno.dti.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <49EA9F88.9000302@juno.dti.ne.jp> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Sun, Apr 19, 2009 at 12:50:32PM +0900, Shin-ichiro KAWASAKI wrote: > Hi, Aurelien, the patch I send one hour ago was broken by my mailer. > I send it again. Sorry for messy posts. Thanks, applied. > Aurelien Jarno wrote: > >> > > On Sat, Apr 18, 2009 at 12:30:01PM +0900, Shin-ichiro KAWASAKI wrote: > >>>> >> >> Hi Kristoffer, Thank you for your feed back. > >>>> >> >> > >>>> >> >> Kristoffer Ericson wrote: > >>>>>> >>> >>> Hi, > >>>>>> >>> >>> > >>>>>> >>> >>> Using kernel from www.assembla.. (6th of april) and getting this at bootup : > >>>>>> >>> >>> Warning: could not add USB device keyboard > >>>>>> >>> >>> > >>>>>> >>> >>> starting with : > >>>>>> >>> >>> qemu-system-sh4 -M r2d -kernel r2d_nokernelarg_zImage -hda sh4-disk.img -m 512M -append "root=/dev/sda1" -serial vc -serial stdio -usb -usbdevice keyboard > >>>>>> >>> >>> inside boot window I see, > >>>>>> >>> >>> can't start sm501-usb..... > >>>>>> >>> >>> startup error -75 > >>>> >> >> SM501's usb support patch has not yet been merged into the qemu trunk. > >>>> >> >> http://lists.gnu.org/archive/html/qemu-devel/2009-03/msg00112.html > >>>> >> >> > >>>> >> >> Here's the new patch which can be applied current svn trunk, rev 7169. > >> > > > >> > > It looks good, I just have a few minor comments. See below. > >> > > > (...) > >>>> >> >> > >>>> >> >> -static inline int ohci_read_ed(uint32_t addr, struct ohci_ed *ed) > >>>> >> >> +static inline int ohci_read_ed(OHCIState *ohci, > >>>> >> >> + uint32_t addr, struct ohci_ed *ed) > >>>> >> >> { > >>>> >> >> - return get_dwords(addr, (uint32_t *)ed, sizeof(*ed) >> 2); > >>>> >> >> + return get_dwords(addr + ohci->localmem_base, > >>>> >> >> + (uint32_t *)ed, sizeof(*ed) >> 2); > >>>> >> >> } > >> > > > >> > > I think the address translation should be done as close as possible to > >> > > the call to cpu_physical_memory_rw(). Could you please move it to > >> > > get_dwords() instead, even if it means passing a pointer to the OHCIState struct there? > >> > > > >> > > Please also use spaces for indentations of ohci_read_ed's arguments. > >> > > > >> > > That's apply to the similar functions below (get_words(), put_dwords(), > >> > > put_words()). > >> > > > > Thank you for reviewing! Here's the revised one. > > # TABs are left in 'hw/sm501.c'. I'll send the patch to eliminate them > # after a while. > > Regards, > Shin-ichiro KAWASAKI > > > # Comments on the patch again, for convenience. > > Adds SM501 usb host emulation feature. > It makes usb keyboard available for sh4/r2d system emulation. > > The changes for "hw/usb-ohci.c" are as follows. > - 'localmem_base' is introduced as OHCIState struct member. > SM501 has a local memory, and it is used to pass and receive data with > OHCI driver. OHCI driver accesses it with SH4 physical memory address, > and SM501 accesses it with SM501 local address. 'localmem_base' holds > where the SM501 local memory is mapped into SH4 physical address space. > - Memory access functions modified to adjust address with 'localmem_base'. > The functions are, ohci_read_*(), ohci_put_*(), and ohci_copy_*(). > - ohci_read_hcca() and ohci_put_hcca() are introduced for more consistent > implementation. > > For other source files, it does, > - introduces usb_ohci_init_sm501(). > - adds irq argument for SM501 initialization, to emulate USB interrupts. > > > Signed-off-by: Shin-ichiro KAWASAKI > > Index: trunk/hw/r2d.c > =================================================================== > --- trunk/hw/r2d.c (revision 7173) > +++ trunk/hw/r2d.c (working copy) > @@ -222,7 +222,7 @@ > irq = r2d_fpga_init(0x04000000, sh7750_irl(s)); > pci = sh_pci_register_bus(r2d_pci_set_irq, r2d_pci_map_irq, irq, 0, 4); > > - sm501_init(0x10000000, SM501_VRAM_SIZE, serial_hds[2]); > + sm501_init(0x10000000, SM501_VRAM_SIZE, irq[SM501], serial_hds[2]); > > /* onboard CF (True IDE mode, Master only). */ > if ((i = drive_get_index(IF_IDE, 0, 0)) != -1) > Index: trunk/hw/usb-ohci.c > =================================================================== > --- trunk/hw/usb-ohci.c (revision 7173) > +++ trunk/hw/usb-ohci.c (working copy) > @@ -32,6 +32,7 @@ > #include "usb.h" > #include "pci.h" > #include "pxa.h" > +#include "devices.h" > > //#define DEBUG_OHCI > /* Dump packet contents. */ > @@ -60,7 +61,8 @@ > > enum ohci_type { > OHCI_TYPE_PCI, > - OHCI_TYPE_PXA > + OHCI_TYPE_PXA, > + OHCI_TYPE_SM501, > }; > > typedef struct { > @@ -108,6 +110,9 @@ > uint32_t hreset; > uint32_t htest; > > + /* SM501 local memory offset */ > + target_phys_addr_t localmem_base; > + > /* Active packets. */ > uint32_t old_ctl; > USBPacket usb_packet; > @@ -425,10 +430,13 @@ > } > > /* Get an array of dwords from main memory */ > -static inline int get_dwords(uint32_t addr, uint32_t *buf, int num) > +static inline int get_dwords(OHCIState *ohci, > + uint32_t addr, uint32_t *buf, int num) > { > int i; > > + addr += ohci->localmem_base; > + > for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { > cpu_physical_memory_rw(addr, (uint8_t *)buf, sizeof(*buf), 0); > *buf = le32_to_cpu(*buf); > @@ -438,10 +446,13 @@ > } > > /* Put an array of dwords in to main memory */ > -static inline int put_dwords(uint32_t addr, uint32_t *buf, int num) > +static inline int put_dwords(OHCIState *ohci, > + uint32_t addr, uint32_t *buf, int num) > { > int i; > > + addr += ohci->localmem_base; > + > for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { > uint32_t tmp = cpu_to_le32(*buf); > cpu_physical_memory_rw(addr, (uint8_t *)&tmp, sizeof(tmp), 1); > @@ -451,10 +462,13 @@ > } > > /* Get an array of words from main memory */ > -static inline int get_words(uint32_t addr, uint16_t *buf, int num) > +static inline int get_words(OHCIState *ohci, > + uint32_t addr, uint16_t *buf, int num) > { > int i; > > + addr += ohci->localmem_base; > + > for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { > cpu_physical_memory_rw(addr, (uint8_t *)buf, sizeof(*buf), 0); > *buf = le16_to_cpu(*buf); > @@ -464,10 +478,13 @@ > } > > /* Put an array of words in to main memory */ > -static inline int put_words(uint32_t addr, uint16_t *buf, int num) > +static inline int put_words(OHCIState *ohci, > + uint32_t addr, uint16_t *buf, int num) > { > int i; > > + addr += ohci->localmem_base; > + > for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { > uint16_t tmp = cpu_to_le16(*buf); > cpu_physical_memory_rw(addr, (uint8_t *)&tmp, sizeof(tmp), 1); > @@ -476,40 +493,63 @@ > return 1; > } > > -static inline int ohci_read_ed(uint32_t addr, struct ohci_ed *ed) > +static inline int ohci_read_ed(OHCIState *ohci, > + uint32_t addr, struct ohci_ed *ed) > { > - return get_dwords(addr, (uint32_t *)ed, sizeof(*ed) >> 2); > + return get_dwords(ohci, addr, (uint32_t *)ed, sizeof(*ed) >> 2); > } > > -static inline int ohci_read_td(uint32_t addr, struct ohci_td *td) > +static inline int ohci_read_td(OHCIState *ohci, > + uint32_t addr, struct ohci_td *td) > { > - return get_dwords(addr, (uint32_t *)td, sizeof(*td) >> 2); > + return get_dwords(ohci, addr, (uint32_t *)td, sizeof(*td) >> 2); > } > > -static inline int ohci_read_iso_td(uint32_t addr, struct ohci_iso_td *td) > +static inline int ohci_read_iso_td(OHCIState *ohci, > + uint32_t addr, struct ohci_iso_td *td) > { > - return (get_dwords(addr, (uint32_t *)td, 4) && > - get_words(addr + 16, td->offset, 8)); > + return (get_dwords(ohci, addr, (uint32_t *)td, 4) && > + get_words(ohci, addr + 16, td->offset, 8)); > } > > -static inline int ohci_put_ed(uint32_t addr, struct ohci_ed *ed) > +static inline int ohci_read_hcca(OHCIState *ohci, > + uint32_t addr, struct ohci_hcca *hcca) > { > - return put_dwords(addr, (uint32_t *)ed, sizeof(*ed) >> 2); > + cpu_physical_memory_rw(addr + ohci->localmem_base, > + (uint8_t *)hcca, sizeof(*hcca), 0); > + return 1; > } > > -static inline int ohci_put_td(uint32_t addr, struct ohci_td *td) > +static inline int ohci_put_ed(OHCIState *ohci, > + uint32_t addr, struct ohci_ed *ed) > { > - return put_dwords(addr, (uint32_t *)td, sizeof(*td) >> 2); > + return put_dwords(ohci, addr, (uint32_t *)ed, sizeof(*ed) >> 2); > } > > -static inline int ohci_put_iso_td(uint32_t addr, struct ohci_iso_td *td) > +static inline int ohci_put_td(OHCIState *ohci, > + uint32_t addr, struct ohci_td *td) > { > - return (put_dwords(addr, (uint32_t *)td, 4) && > - put_words(addr + 16, td->offset, 8)); > + return put_dwords(ohci, addr, (uint32_t *)td, sizeof(*td) >> 2); > } > > +static inline int ohci_put_iso_td(OHCIState *ohci, > + uint32_t addr, struct ohci_iso_td *td) > +{ > + return (put_dwords(ohci, addr, (uint32_t *)td, 4) && > + put_words(ohci, addr + 16, td->offset, 8)); > +} > + > +static inline int ohci_put_hcca(OHCIState *ohci, > + uint32_t addr, struct ohci_hcca *hcca) > +{ > + cpu_physical_memory_rw(addr + ohci->localmem_base, > + (uint8_t *)hcca, sizeof(*hcca), 1); > + return 1; > +} > + > /* Read/Write the contents of a TD from/to main memory. */ > -static void ohci_copy_td(struct ohci_td *td, uint8_t *buf, int len, int write) > +static void ohci_copy_td(OHCIState *ohci, struct ohci_td *td, > + uint8_t *buf, int len, int write) > { > uint32_t ptr; > uint32_t n; > @@ -518,16 +558,17 @@ > n = 0x1000 - (ptr & 0xfff); > if (n > len) > n = len; > - cpu_physical_memory_rw(ptr, buf, n, write); > + cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, n, write); > if (n == len) > return; > ptr = td->be & ~0xfffu; > buf += n; > - cpu_physical_memory_rw(ptr, buf, len - n, write); > + cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, len - n, write); > } > > /* Read/Write the contents of an ISO TD from/to main memory. */ > -static void ohci_copy_iso_td(uint32_t start_addr, uint32_t end_addr, > +static void ohci_copy_iso_td(OHCIState *ohci, > + uint32_t start_addr, uint32_t end_addr, > uint8_t *buf, int len, int write) > { > uint32_t ptr; > @@ -537,12 +578,12 @@ > n = 0x1000 - (ptr & 0xfff); > if (n > len) > n = len; > - cpu_physical_memory_rw(ptr, buf, n, write); > + cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, n, write); > if (n == len) > return; > ptr = end_addr & ~0xfffu; > buf += n; > - cpu_physical_memory_rw(ptr, buf, len - n, write); > + cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, len - n, write); > } > > static void ohci_process_lists(OHCIState *ohci, int completion); > @@ -579,7 +620,7 @@ > > addr = ed->head & OHCI_DPTR_MASK; > > - if (!ohci_read_iso_td(addr, &iso_td)) { > + if (!ohci_read_iso_td(ohci, addr, &iso_td)) { > printf("usb-ohci: ISO_TD read error at %x\n", addr); > return 0; > } > @@ -621,7 +662,7 @@ > i = OHCI_BM(iso_td.flags, TD_DI); > if (i < ohci->done_count) > ohci->done_count = i; > - ohci_put_iso_td(addr, &iso_td); > + ohci_put_iso_td(ohci, addr, &iso_td); > return 0; > } > > @@ -696,7 +737,7 @@ > } > > if (len && dir != OHCI_TD_DIR_IN) { > - ohci_copy_iso_td(start_addr, end_addr, ohci->usb_buf, len, 0); > + ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len, 0); > } > > if (completion) { > @@ -732,7 +773,7 @@ > /* Writeback */ > if (dir == OHCI_TD_DIR_IN && ret >= 0 && ret <= len) { > /* IN transfer succeeded */ > - ohci_copy_iso_td(start_addr, end_addr, ohci->usb_buf, ret, 1); > + ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret, 1); > OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC, > OHCI_CC_NOERROR); > OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_SIZE, ret); > @@ -788,7 +829,7 @@ > if (i < ohci->done_count) > ohci->done_count = i; > } > - ohci_put_iso_td(addr, &iso_td); > + ohci_put_iso_td(ohci, addr, &iso_td); > return 1; > } > > @@ -818,7 +859,7 @@ > #endif > return 1; > } > - if (!ohci_read_td(addr, &td)) { > + if (!ohci_read_td(ohci, addr, &td)) { > fprintf(stderr, "usb-ohci: TD read error at %x\n", addr); > return 0; > } > @@ -859,7 +900,7 @@ > } > > if (len && dir != OHCI_TD_DIR_IN && !completion) { > - ohci_copy_td(&td, ohci->usb_buf, len, 0); > + ohci_copy_td(ohci, &td, ohci->usb_buf, len, 0); > } > } > > @@ -918,7 +959,7 @@ > } > if (ret >= 0) { > if (dir == OHCI_TD_DIR_IN) { > - ohci_copy_td(&td, ohci->usb_buf, ret, 1); > + ohci_copy_td(ohci, &td, ohci->usb_buf, ret, 1); > #ifdef DEBUG_PACKET > dprintf(" data:"); > for (i = 0; i < ret; i++) > @@ -987,7 +1028,7 @@ > i = OHCI_BM(td.flags, TD_DI); > if (i < ohci->done_count) > ohci->done_count = i; > - ohci_put_td(addr, &td); > + ohci_put_td(ohci, addr, &td); > return OHCI_BM(td.flags, TD_CC) != OHCI_CC_NOERROR; > } > > @@ -1005,7 +1046,7 @@ > return 0; > > for (cur = head; cur; cur = next_ed) { > - if (!ohci_read_ed(cur, &ed)) { > + if (!ohci_read_ed(ohci, cur, &ed)) { > fprintf(stderr, "usb-ohci: ED read error at %x\n", cur); > return 0; > } > @@ -1046,7 +1087,7 @@ > } > } > > - ohci_put_ed(cur, &ed); > + ohci_put_ed(ohci, cur, &ed); > } > > return active; > @@ -1087,7 +1128,7 @@ > OHCIState *ohci = opaque; > struct ohci_hcca hcca; > > - cpu_physical_memory_rw(ohci->hcca, (uint8_t *)&hcca, sizeof(hcca), 0); > + ohci_read_hcca(ohci, ohci->hcca, &hcca); > > /* Process all the lists at the end of the frame */ > if (ohci->ctl & OHCI_CTL_PLE) { > @@ -1131,7 +1172,7 @@ > ohci_sof(ohci); > > /* Writeback HCCA */ > - cpu_physical_memory_rw(ohci->hcca, (uint8_t *)&hcca, sizeof(hcca), 1); > + ohci_put_hcca(ohci, ohci->hcca, &hcca); > } > > /* Start sending SOF tokens across the USB bus, lists are processed in > @@ -1620,7 +1661,8 @@ > }; > > static void usb_ohci_init(OHCIState *ohci, int num_ports, int devfn, > - qemu_irq irq, enum ohci_type type, const char *name) > + qemu_irq irq, enum ohci_type type, > + const char *name, uint32_t localmem_base) > { > int i; > > @@ -1641,6 +1683,7 @@ > } > > ohci->mem = cpu_register_io_memory(0, ohci_readfn, ohci_writefn, ohci); > + ohci->localmem_base = localmem_base; > ohci->name = name; > > ohci->irq = irq; > @@ -1687,7 +1730,7 @@ > ohci->pci_dev.config[0x3d] = 0x01; /* interrupt pin 1 */ > > usb_ohci_init(&ohci->state, num_ports, devfn, ohci->pci_dev.irq[0], > - OHCI_TYPE_PCI, ohci->pci_dev.name); > + OHCI_TYPE_PCI, ohci->pci_dev.name, 0); > > pci_register_io_region((struct PCIDevice *)ohci, 0, 256, > PCI_ADDRESS_SPACE_MEM, ohci_mapfunc); > @@ -1699,7 +1742,19 @@ > OHCIState *ohci = (OHCIState *)qemu_mallocz(sizeof(OHCIState)); > > usb_ohci_init(ohci, num_ports, devfn, irq, > - OHCI_TYPE_PXA, "OHCI USB"); > + OHCI_TYPE_PXA, "OHCI USB", 0); > > cpu_register_physical_memory(base, 0x1000, ohci->mem); > } > + > +void usb_ohci_init_sm501(uint32_t mmio_base, uint32_t localmem_base, > + int num_ports, int devfn, qemu_irq irq) > +{ > + OHCIState *ohci = (OHCIState *)qemu_mallocz(sizeof(OHCIState)); > + > + usb_ohci_init(ohci, num_ports, devfn, irq, > + OHCI_TYPE_SM501, "OHCI USB", localmem_base); > + > + cpu_register_physical_memory(mmio_base, 0x1000, ohci->mem); > +} > + > Index: trunk/hw/sm501.c > =================================================================== > --- trunk/hw/sm501.c (revision 7173) > +++ trunk/hw/sm501.c (working copy) > @@ -1055,7 +1055,8 @@ > sm501_draw_crt(s); > } > > -void sm501_init(uint32_t base, uint32_t local_mem_bytes, CharDriverState *chr) > +void sm501_init(uint32_t base, uint32_t local_mem_bytes, qemu_irq irq, > + CharDriverState *chr) > { > SM501State * s; > int sm501_system_config_index; > @@ -1089,6 +1090,10 @@ > cpu_register_physical_memory(base + MMIO_BASE_OFFSET + SM501_DC, > 0x1000, sm501_disp_ctrl_index); > > + /* bridge to usb host emulation module */ > + usb_ohci_init_sm501(base + MMIO_BASE_OFFSET + SM501_USB_HOST, base, > + 2, -1, irq); > + > /* bridge to serial emulation module */ > if (chr) > serial_mm_init(base + MMIO_BASE_OFFSET + SM501_UART0, 2, > Index: trunk/hw/devices.h > =================================================================== > --- trunk/hw/devices.h (revision 7173) > +++ trunk/hw/devices.h (working copy) > @@ -74,5 +74,10 @@ > qemu_irq tc6393xb_l3v_get(struct tc6393xb_s *s); > > /* sm501.c */ > -void sm501_init(uint32_t base, uint32_t local_mem_bytes, CharDriverState *chr); > +void sm501_init(uint32_t base, uint32_t local_mem_bytes, qemu_irq irq, > + CharDriverState *chr); > + > +/* usb-ohci.c */ > +void usb_ohci_init_sm501(uint32_t mmio_base, uint32_t localmem_base, > + int num_ports, int devfn, qemu_irq irq); > #endif > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net