Chameleon > Patches

[PATCH] defer USB controller reset until done loading everything

(1/2) > >>

r007:
As detailed in my support forum post resetting the USB when you still want to load data from it might not be a very bright idea.

This attached patch defers USB resets until setup of fake efi is through.

Issues:
- It's limited to max 10 Controllers of each type (UHCI, EHCI) due to lack of proper dynamically allocated queue
- It lacks a proper dynamically allocated queue ;)

Tell me what you think.

meklort:
Well, my first comment is that it's probably a bad idea to have this line:

--- Code: ---memcpy(&gUSBEnqueuedEHCI[gUSBEnqueuedEHCICount++], pci_dev, sizeof(pci_dt_t));
--- End code ---

gUSBEnqueuedEHCI is hardcoded to a size of  MAX_USB_DEVICES * sizeof(pci_dt_t), so if you ever have a machine with more than MAX_USB_DEVICES controllers, you're writing to memory that you shouldn't be. The same goes for the other pci_dt_t array.

If you really wanted to, you could change it to

--- Code: ---if(gUSBEnqueuedEHCICount < MAX_USB_DEVICES ) memcpy(&gUSBEnqueuedEHCI[gUSBEnqueuedEHCICount++], pci_dev, sizeof(pci_dt_t));
--- End code ---


I also have an implementation in my branch (http://forge.voodooprojects.org/p/chameleon/source/tree/HEAD/branches/meklort/i386/libsaio/usb.c) for the usb code, and inside of pci_setup.c as well. There is one main bug that I still haven't fixed with it (running the reset on the correct controller, but that's just two lines to fix)

r007:
Yup, I realize that that is extremely bad code (hey, it took me about an hour to fix what I was looking for...)

All in all I like your code better... (Sure enough, moving the commented-out switch statement from pci_setup.c to usb.c will do for calling the appropriate code for the controller.)

Only one question: shouldn't you be free()ing that array as well after usb_loop() is done?

Any chance this yours gets merged upstream anytime soon?

zef:

--- Quote from: r007 on July 17, 2010, 10:48:38 PM ---Any chance this yours gets merged upstream anytime soon?

--- End quote ---

I'm inspecting meklort's changes, and I'd like to merge his md0 ramdisk + usb changes back to the trunk.

meklort:

--- Quote from: r007 on July 17, 2010, 10:48:38 PM ---Only one question: shouldn't you be free()ing that array as well after usb_loop() is done?
--- End quote ---

There isn't any reason to, the kernel with just reuse it when it's started up (I believe, correct me if I'm wrong).

Navigation

[0] Message Index

[#] Next page