Hi, To Szymon (& Vladimir too...): ... > In such case EHCI should handover port to a companion 1.1 controller. > (see section 4.2 of EHCI specification)
Yes, driver does it in such way - but I tested it first alone, without any other USB driver, and in such case companion controllers of course did not work. I wrote it maybe little bit not clearly in my e-mail... I did two kind of tests: 1. Loaded only EHCI driver (paragraph beginning with "I shortly tested main functions..." 2. Loaded EHCI driver and UHCI driver (for companion controllers; paragraph beginning with "I tried also test EHCI and UHCI drivers together..." As You can see, in second kind of test is full/low speed working even if it is connected to root port (EHCI/UHCI shared). The only one thing, which is not working, is low speed HID device connected via USB 2.0 hub (because in both cases the USB 2.0 hub is handled by EHCI). As I wrote, I suspect some bulk->interrupt transaction conflict inside USB 2.0 hub when it is done via Split Transaction from EHCI. I did not study USB 2.0 and EHCI specifications too deeply to be sure... There could be little bit another but related possible issue: Driver is currently very simple and it does not test how many ports of companion controllers are used and if some of them is free or not -> I don't know what can happen in case when there is no free port of companion controller - I cannot test it on my HW... Vladimir wrote it has HW which has no companion controllers, so he can test it... To Vladimir: Ufff, You sent a lot of comments... :-) Note, it is first working/tested version... My main problem is the time, so I wrote it piece by piece through approx. half of year, often I forgot what I did before... - so the code looks like it looks... I will rewrite some part according to Your comments but don't expect it too early... Answers/questions to most of Your comments: ... macro->inline func. - I like macros... :-) But You are right. ... run indent - Why not. But what options of indent are preferred for GRUB source ? I can set in the same way also my favorite editor (Geany) to prevent necessity of such source code additional processing. ... __attribute__((packed)) - I remember, we probably discussed the same thing on OHCI/UHCI in past. In these drivers are such attributes still used for HW structures. I think it is more safe - we probably cannot assume that compiler will align structure members to 32 bits. ... only 32 bits - The driver is very simple, similarly as UHCI driver. EHCI can support memory higher than 4G but I currently don't care about it, I fill all related registers with zero... I cannot test it (my computer is little bit older and cannot use more than 4GB RAM...) and, generally, I don't have experiences with 64 bits... Similarly, to be simple, driver currently does not use DMA allocations (as UHCI driver) - there is too much work with it... :-) - some thing should be done little bit different because in GRUB is missing some function which can convert DMA address to virtual address and vice versa. I made some helping functions in OHCI driver but I am not sure if these functions are safe because of possible not continuous DMA memory - or does the function grub_memalign_dma32 ensure that whole allocated memory is continuous in both DMA and virtual spaces in every case ? (It is maybe stupid question but I don't want to study whole GRUB code related to memory management, additionally, I don't know details about memory management of non-x86 architectures...). Under term "full 64-bit compatible" I understand that any register, QH/TD structure and any buffer data can be above 4G - maybe it is wrong from my side. So, to be fully 64-bit compatible, if memory for framelist, QH and TD is allocated above 4G, CTRLDSSEGMENT register should be properly set - and, additionally, framelist, QH and TD should be in the same 4G memory area and should not cross any 4G boundary. Of course, CTRLDSSEGMENT register is set to zero in current driver version and similarly also higher dwords of buffer address are set to zero inside QH/TD structures - but it is also 32-bit simplification because it assumes that memory of transfer buffer is below 4G in every case. ... "No need to specifically exclude those. Just zero-pad address." - Hm, I think it could be not so easy because (base_h !=0) means that EHCI I/O memory registers are mapped above 4G... Maybe stupid question - does it work access to mapped I/O in this case properly in GRUB ? ... "e->iobase should have volatile attribute" - I have related but probably stupid question - why does work evaluation of QH/TD values in runtime properly even if related variables are not declared as "volatile" ? (It is the same in UHCI/OHCI drivers.) AFAIK "volatile" should be used for variables which can be modified "outside" of compiler code, i.e. by HW/DMA. So, I cannot understand why for example e->iobase should be "volatile" (even if this variable cannot be changed by HW/DMA - but, of course, values of memory area related to this pointer can be changed by HW - maybe this is the reason (?)) and structures QH/TD need not to be "volatile" even if their values can be changed by HW/DMA... ??? ... "Arithmetics with PCI address aren't guaranteed to be available or to behave in a sane way." - Hm, yes, I change it to usual construction - read whole DWORD, change only one bit and write it back. I will also add the "hard way" of ownership as You propose. Best regards Ales _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
