[PATCH 1/4] Minor Cleanup

2009-06-08 Thread oliver . henshaw
Changelog:
* bus/usb/uhci.c: Remove un-needed doubled lines.
* bus/usb/ohci.c: Likewise. Change interf to grub_uint32_t. Remove 
whitespace inside comment.
---
 bus/usb/ohci.c |7 ++-
 bus/usb/uhci.c |2 --
 2 files changed, 2 insertions(+), 7 deletions(-)

Index: grub2/bus/usb/ohci.c
===
--- grub2.orig/bus/usb/ohci.c
+++ grub2/bus/usb/ohci.c
@@ -118,7 +118,7 @@ grub_ohci_pci_iter (int bus, int device,
 {
   grub_uint32_t class;
   grub_uint32_t subclass;
-  int interf;
+  grub_uint32_t interf;
   grub_uint32_t base;
   grub_pci_address_t addr;
   struct grub_ohci *o;
@@ -127,8 +127,6 @@ grub_ohci_pci_iter (int bus, int device,
 
   addr = grub_pci_make_address (bus, device, func, 2);
   class = grub_pci_read (addr);
-  addr = grub_pci_make_address (bus, device, func, 2);
-  class = grub_pci_read (addr);
 
   interf = class & 0xFF;
   subclass = (class >> 16) & 0xFF;
@@ -171,8 +169,7 @@ grub_ohci_pci_iter (int bus, int device,
   frame_interval = grub_ohci_readreg32 (o, GRUB_OHCI_REG_FRAME_INTERVAL);
 
   /* Suspend the OHCI by issuing a reset.  */
-  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, 1); /* XXX: Magic.
- */
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, 1); /* XXX: Magic.  */
   grub_millisleep (1);
   grub_dprintf ("ohci", "OHCI reset\n");
 
Index: grub2/bus/usb/uhci.c
===
--- grub2.orig/bus/usb/uhci.c
+++ grub2/bus/usb/uhci.c
@@ -151,8 +151,6 @@ grub_uhci_pci_iter (int bus, int device,
 
   addr = grub_pci_make_address (bus, device, func, 2);
   class = grub_pci_read (addr);
-  addr = grub_pci_make_address (bus, device, func, 2);
-  class = grub_pci_read (addr);
 
   subclass = (class >> 16) & 0xFF;
   class >>= 24;



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 4/4] Define fields in terms of the Class Code register

2009-06-08 Thread oliver . henshaw
Changelog:
* bus/usb/ohci.c: Define the Class, Subclass and Programming Interface 
fields in 
  terms of the 3 byte Class Code register.
* bus/usb/uhci.c: Likewise.
---
 bus/usb/ohci.c |9 +
 bus/usb/uhci.c |9 +
 2 files changed, 10 insertions(+), 8 deletions(-)

Index: grub2/bus/usb/ohci.c
===
--- grub2.orig/bus/usb/ohci.c
+++ grub2/bus/usb/ohci.c
@@ -116,6 +116,7 @@ static int NESTED_FUNC_ATTR
 grub_ohci_pci_iter (int bus, int device, int func,
grub_pci_id_t pciid __attribute__((unused)))
 {
+  grub_uint32_t class_code;
   grub_uint32_t class;
   grub_uint32_t subclass;
   grub_uint32_t interf;
@@ -126,11 +127,11 @@ grub_ohci_pci_iter (int bus, int device,
   grub_uint32_t frame_interval;
 
   addr = grub_pci_make_address (bus, device, func, 2);
-  class = grub_pci_read (addr);
+  class_code = grub_pci_read (addr) >> 8;
 
-  interf = (class >> 8) & 0xFF;
-  subclass = (class >> 16) & 0xFF;
-  class >>= 24;
+  interf = class_code & 0xFF;
+  subclass = (class_code >> 8) & 0xFF;
+  class = class_code >> 16;
 
   /* If this is not an OHCI controller, just return.  */
   if (class != 0x0c || subclass != 0x03 || interf != 0x10)
Index: grub2/bus/usb/uhci.c
===
--- grub2.orig/bus/usb/uhci.c
+++ grub2/bus/usb/uhci.c
@@ -141,6 +141,7 @@ static int NESTED_FUNC_ATTR
 grub_uhci_pci_iter (int bus, int device, int func,
grub_pci_id_t pciid __attribute__((unused)))
 {
+  grub_uint32_t class_code;
   grub_uint32_t class;
   grub_uint32_t subclass;
   grub_uint32_t interf;
@@ -151,11 +152,11 @@ grub_uhci_pci_iter (int bus, int device,
   int i;
 
   addr = grub_pci_make_address (bus, device, func, 2);
-  class = grub_pci_read (addr);
+  class_code = grub_pci_read (addr) >> 8;
 
-  interf = (class >> 8) & 0xFF;
-  subclass = (class >> 16) & 0xFF;
-  class >>= 24;
+  interf = class_code & 0xFF;
+  subclass = (class_code >> 8) & 0xFF;
+  class = class_code >> 16;
 
   /* If this is not an UHCI controller, just return.  */
   if (class != 0x0c || subclass != 0x03 || interf != 0x00)



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 3/4] Check usb programming interface

2009-06-08 Thread oliver . henshaw
Changelog:
* bus/usb/ohci.c: Check programming interface is ohci. Add grub_dprintf 
for symmetry 
  with bus/usb/uhci.c.
* bus/usb/uhci.c: Check programming interface is uhci. Add interf 
variable for 
  Programming Interface. Print interface with 
class/subclass.
---
 bus/usb/ohci.c |5 -
 bus/usb/uhci.c |8 +---
 2 files changed, 9 insertions(+), 4 deletions(-)

Index: grub2/bus/usb/ohci.c
===
--- grub2.orig/bus/usb/ohci.c
+++ grub2/bus/usb/ohci.c
@@ -133,7 +133,7 @@ grub_ohci_pci_iter (int bus, int device,
   class >>= 24;
 
   /* If this is not an OHCI controller, just return.  */
-  if (class != 0x0c || subclass != 0x03)
+  if (class != 0x0c || subclass != 0x03 || interf != 0x10)
 return 0;
 
   /* Determine IO base address.  */
@@ -159,6 +159,9 @@ grub_ohci_pci_iter (int bus, int device,
   /* Reserve memory for the HCCA.  */
   o->hcca = (struct grub_ohci_hcca *) grub_memalign (256, 256);
 
+  grub_dprintf ("ohci", "class=0x%02x 0x%02x interface 0x%02x base=0x%x\n",
+   class, subclass, interf, o->iobase);
+
   /* Check if the OHCI revision is actually 1.0 as supported.  */
   revision = grub_ohci_readreg32 (o, GRUB_OHCI_REG_REVISION);
   grub_dprintf ("ohci", "OHCI revision=0x%02x\n", revision & 0xFF);
Index: grub2/bus/usb/uhci.c
===
--- grub2.orig/bus/usb/uhci.c
+++ grub2/bus/usb/uhci.c
@@ -143,6 +143,7 @@ grub_uhci_pci_iter (int bus, int device,
 {
   grub_uint32_t class;
   grub_uint32_t subclass;
+  grub_uint32_t interf;
   grub_uint32_t base;
   grub_uint32_t fp;
   grub_pci_address_t addr;
@@ -152,11 +153,12 @@ grub_uhci_pci_iter (int bus, int device,
   addr = grub_pci_make_address (bus, device, func, 2);
   class = grub_pci_read (addr);
 
+  interf = (class >> 8) & 0xFF;
   subclass = (class >> 16) & 0xFF;
   class >>= 24;
 
   /* If this is not an UHCI controller, just return.  */
-  if (class != 0x0c || subclass != 0x03)
+  if (class != 0x0c || subclass != 0x03 || interf != 0x00)
 return 0;
 
   /* Determine IO base address.  */
@@ -177,8 +179,8 @@ grub_uhci_pci_iter (int bus, int device,
   u->framelist = 0;
   u->qh = 0;
   u->td = 0;
-  grub_dprintf ("uhci", "class=0x%02x 0x%02x base=0x%x\n",
-   class, subclass, u->iobase);
+  grub_dprintf ("uhci", "class=0x%02x 0x%02x interface 0x%02x base=0x%x\n",
+   class, subclass, interf, u->iobase);
 
   /* Reserve a page for the frame list.  */
   u->framelist = grub_memalign (4096, 4096);



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 2/4] Fix inteface definition for ohci

2009-06-08 Thread oliver . henshaw
Changelog:
* bus/usb/ohci.c: Set interf with correct field.
---
 bus/usb/ohci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: grub2/bus/usb/ohci.c
===
--- grub2.orig/bus/usb/ohci.c
+++ grub2/bus/usb/ohci.c
@@ -128,7 +128,7 @@ grub_ohci_pci_iter (int bus, int device,
   addr = grub_pci_make_address (bus, device, func, 2);
   class = grub_pci_read (addr);
 
-  interf = class & 0xFF;
+  interf = (class >> 8) & 0xFF;
   subclass = (class >> 16) & 0xFF;
   class >>= 24;
 



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 0/4] Re: Grub needs to check the programming interface for usb controllers

2009-06-08 Thread oliver . henshaw
Here's a second try. I used quilt to manage the patch series but mailed them by 
hand instead of exporting to a mailbox, and didn't realise that they weren't 
named as *.patch (otherwise I think the content type would have been fine). I 
should probably move on to git, but I was a little intimidated by the idea of 
re-writing history - manipulating a patch series seemed more natural to me. But 
onwards and upwards.

I've removed all changes to trailing whitespace, as requested. There is still a 
whitespace change in side a comment, something I noticed with syntax 
highlighting.


Thanks,
Oliver


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: insmod ohci fails to register usb ports

2009-06-07 Thread Oliver Henshaw
2009/6/7 Oliver Henshaw :
> Even with the fixes mentioned previously, grub fails with an OHCI
> controller (NEC ohci/ehci chipset on a PCI card). It seems to successfully 
> initialise the
> controller, but fails to enumerate the ports. The same output is seen with a 
> mouse
> attached as with no devices attached.

I forgot to mention what I've tried so far.
* I read the Host Controller initialisation part of the OHCI spec and
checked whether the controller is initially under SMI or BIOS control
- but it's not.
* I then took a look at the freebsd[1] usb controller drivers and
tried out the quirks and workarounds they use - but they didn't make
any difference.

What I haven't done is put any effort into figuring out what more
needs to be done to make sure all queue-related registers are
correctly set. It doesn't look like everything suggested in the OHCI
spec is implemented. Also, it looks like the freebsd driver does still
more than the spec recommends.


Oliver

[1] For some reason, the usb drivers in FreeBSD 7 are still
BSD+Advertising license. Luckily, the drivers in svn head are not.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: insmod uhci causes computer to reset

2009-06-07 Thread Oliver Henshaw
2009/6/7 Pavel Roskin :
> Maybe the crash is in usb_keyboard?
I don't think that can be it. AFAICS usb_keyboard would only be called
if I manually loaded the model. And I've never done that.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


insmod ohci fails to register usb ports

2009-06-07 Thread Oliver Henshaw
Even with the fixes mentioned previously, grub fails with an OHCI
controller (NEC ohci/ehci

chipset on a PCI card). It seems to successfully initialise the
controller, but fails to

enumerate the ports. The same output is seen with a mouse attached as
with no devices

attached.



grub> insmod ohci

../bus/usb/ohci.c:180: class=0x0c 0x03 interface 0x10 base=0xe2102000

../bus/usb/ohci.c:184: OHCI revision=0x10

../bus/usb/ohci.c:195: OHCI reset

../bus/usb/ohci.c:213: OHCI HCCA

../bus/usb/ohci.c:219: OHCI enable: 0x02

../bus/usb/ohci.c:180: class=0x0c 0x03 interface 0x10 base=0xe2103000

../bus/usb/ohci.c:184: OHCI revision=0x10

../bus/usb/ohci.c:195: OHCI reset

../bus/usb/ohci.c:213: OHCI HCCA

../bus/usb/ohci.c:219: OHCI enable: 0x02

../bus/usb/ohci.c:622: root hub ports=2

../bus/usb/ohci.c:604: detect_dev status=0x00

../bus/usb/ohci.c:604: detect_dev status=0x00

../bus/usb/ohci.c:622: root hub ports=3

../bus/usb/ohci.c:604: detect_dev status=0x00

../bus/usb/ohci.c:604: detect_dev status=0x00

../bus/usb/ohci.c:604: detect_dev status=0x00


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


insmod uhci causes computer to reset

2009-06-07 Thread Oliver Henshaw
Even with the fixes described previously, grub fails with an UHCI
controller (VIA uhci
chipset on the motherboard). When no devices are attached, module
loading completes
successfully. When a mouse is attached, module loading completes but
listing the device with
'usb' causes the machine to reset. If I try without 'set debug=uhci'
then 'insmod uhci' causes
the computer to hang.





With no devices:



grub> insmod uhci

../bus/usb/uhci.c:188: class=0x0c 0x03 interface 0x00 base=0xe400

../bus/usb/uhci.c:278: UHCI initialized

../bus/usb/uhci.c:188: class=0x0c 0x03 interface 0x00 base=0xe800

../bus/usb/uhci.c:278: UHCI initialized

../bus/usb/uhci.c:636: detect=0x48a port=0

../bus/usb/uhci.c:636: detect=0x58a port=1

../bus/usb/uhci.c:636: detect=0x48a port=0

../bus/usb/uhci.c:636: detect=0x48a port=1

../bus/usb/uhci.c:668: registered


grub>


With a mouse attached:



grub> insmod uhci

../bus/usb/uhci.c:188: class=0x0c 0x03 interface 0x00 base=0xe400

../bus/usb/uhci.c:278: UHCI initialized

../bus/usb/uhci.c:188: class=0x0c 0x03 interface 0x00 base=0xe800

../bus/usb/uhci.c:278: UHCI initialized

../bus/usb/uhci.c:636: detect=0x48a port=0

../bus/usb/uhci.c:636: detect=0x5ab port=1

../bus/usb/uhci.c:582: enable=1 port=1

../bus/usb/uhci.c:593: detect=0x5ab

../bus/usb/uhci.c:602: reset completed

../bus/usb/uhci.c:608: waiting for the port to be enabled

../bus/usb/uhci.c:613: >3detect=0x5a7

../bus/usb/usbtrans.c:43: control: reqtype=0x80 req=0x06 val=0x100 idx=0x00 size

=18

../bus/usb/uhci.c:408: transaction: endp=0, type=2, addr=0, toggle=0, size=8 dat

a=0x67ad0 td=0x2403e000

../bus/usb/uhci.c:408: transaction: endp=0, type=0, addr=0, toggle=1, size=18 da

ta=0x2403d760 td=0x2403e020

../bus/usb/uhci.c:408: transaction: endp=0, type=1, addr=0, toggle=1, size=0 dat

a=0x0 td=0x2403e040

../bus/usb/uhci.c:481: setup transaction 0

../bus/usb/uhci.c:487: initiate transaction

../bus/usb/uhci.c:498: >t status=0x440007 data=0x67ad0 td=0x2403e000

../bus/usb/uhci.c:504: t status=0x440007

../bus/usb/uhci.c:509: >>t status=0x440007

../bus/usb/uhci.c:548: transaction failed

../bus/usb/usbtrans.c:43: control: reqtype=0x00 req=0x05 val=0x01 idx=0x00 size=
0

../bus/usb/uhci.c:408: transaction: endp=0, type=2, addr=0, toggle=0, size=8 dat
a=0x67b10 td=0x2403e040

../bus/usb/uhci.c:408: transaction: endp=0, type=0, addr=0, toggle=1, size=0 dat
a=0x0 td=0x2403e020

../bus/usb/uhci.c:481: setup transaction 0

../bus/usb/uhci.c:487: initiate transaction

../bus/usb/uhci.c:498: >t status=0x440007 data=0x67b10 td=0x2403e040

../bus/usb/uhci.c:504: t status=0x440007

../bus/usb/uhci.c:509: >>t status=0x440007

../bus/usb/uhci.c:548: transaction failed

../bus/usb/uhci.c:636: detect=0x48a port=0

../bus/usb/uhci.c:636: detect=0x48a port=1

../bus/usb/uhci.c:668: registered

grub> usb

USB devices:



USB version 0.0, VendorID: 0x00, ProductID: 0x00, #conf: 0

Low speed device

Interface #0: #Endpoints: 22   ../bus/usb/usbtrans.c:43: control: reqtype=0x80 r
eq=0x06 val=0x3c3 idx=0x409 size=1


(and then the computer resets)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Link usb controller struct only when initialised

2009-06-07 Thread Oliver Henshaw
When controller initialisation is aborted in grub_uhci_pci_iter
(grub_ohci_pci_iter) control
jumps to "fail:", where any allocated memory is freed. However, the
struct grub_uhci *u
(struct grub_ohci *o) for that controller remains linked to the list
of UHCI (OHCI)
controllers. This causes problems later, when grub iterates over
controllers to initialise
their ports.

The solution is to link only when the usb controller is successfully
initialised, and just
before returning from the function.

This patch is tested with real hardware and with qemu and the rescue image.


Thanks,
Oliver
ChangeLog:

* bus/usb/ohci.c: Link struct only after initialising controller.
* bus/usb/uhci.c: Likewise.

Index: bus/usb/ohci.c
===
--- bus/usb/ohci.c	(revision 2216)
+++ bus/usb/ohci.c	(working copy)
@@ -153,9 +153,6 @@ grub_ohci_pci_iter (int bus, int device, int func,
   if (! o)
 return 1;
 
-  /* Link in the OHCI.  */
-  o->next = ohci;
-  ohci = o;
   o->iobase = (grub_uint32_t *) base;
 
   /* Reserve memory for the HCCA.  */
@@ -189,6 +186,10 @@ grub_ohci_pci_iter (int bus, int device, int func,
   grub_dprintf ("ohci", "OHCI enable: 0x%02x\n",
 		(grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL) >> 6) & 3);
  
+  /* link to ohci now that initialisation is successful. */
+  o->next = ohci;
+  ohci = o;
+
   return 0;
 
  fail:
Index: bus/usb/uhci.c
===
--- bus/usb/uhci.c	(revision 2216)
+++ bus/usb/uhci.c	(working copy)
@@ -173,8 +173,6 @@ grub_uhci_pci_iter (int bus, int device, int func,
   if (! u)
 return 1;
 
-  u->next = uhci;
-  uhci = u;
   u->iobase = base & GRUB_UHCI_IOMASK;
   u->framelist = 0;
   u->qh = 0;
@@ -287,6 +285,10 @@ grub_uhci_pci_iter (int bus, int device, int func,
   }
 #endif
 
+  /* link to uhci now that initialisation is successful. */
+  u->next = uhci;
+  uhci = u;
+
   return 0;
 
  fail:
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 4/4] Define fields in terms of the Class Code register

2009-06-07 Thread Oliver Henshaw



usb-change-to-class_code
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 3/4] Check usb programming interface

2009-06-07 Thread Oliver Henshaw



usb-check-interface
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 2/4] Fix inteface definition for ohci

2009-06-07 Thread Oliver Henshaw



usb-fix-interface
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 1/4] bus/usb minor cleanups

2009-06-07 Thread Oliver Henshaw



usb-minor-cleanup
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Grub needs to check the programming interface for usb controllers

2009-06-07 Thread Oliver Henshaw
Grub needs to check that the usb programming interface is of the
correct type. Otherwise, hilarity ensues when e.g. bus/usb/ohci.c
attempts to initialise an uhci controller. This can easily be
reproduced by typing 'insmod ohci' in the grub rescue disk in qemu,
with usb enabled.

Additionally, the usb interface (which was defined but not used in
bus/usb/ohci.c) was set to the wrong value.

I've split this into a patch series bookended by more cosmetic
patches. The first patch consists of minor fixes: removing some
doubled lines (which look like a pasto, but I could be wrong),
changing an int to a grub_uint32_t (to match the types of all other
similar variables) and I've added some whitespace fixes. The final
patch is purely cosmetic: it defines the pci Class, Subclass and
Programming Interface fields in terms of the Class Code register, so
that the code better reflects the spec.

This last patch may seem minor, but the (previously unused) usb
interface was set to the wrong value in bus/usb/ohci.c up to now -
this means that the code now better reflects the structure of the
Class Code register, and better documents the specification.

The patch series has been tested on on real hardware, and each patch
has been tested with qemu and a grub rescue image. Both ohci and uhci
cases have been tested (qemu has a virtual ohci controller, but the
option to enable it must be patched in).


Thanks,
Oliver


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel