Re: [Qemu-devel] sh4 r2d system emulation - warning: could not add USB device keyboard

[Aurelien Jarno]

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 <address@hidden> 
> 
> 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
address@hidden                 http://www.aurel32.net