Aleš Nesrsta wrote: > > >> + * So we should: >> + * - allow WritebackDoneHead interrupt (WDH) by proper setting of last TD >> + * token - it is done above in transaction settings >> + * - detect setting of WDH bit in HcInterruptStatus register >> + * - compare HccaDoneHead value with address of last-1 TD. If it is not >> + * equal, check ED for halt and if not so, reset WDH bit and wait again >> + * - but it should not happen - debug it! >> Are the comments from you or any part copied from spec. We need no copy from spec as spec is available anyway and sentences copied from it may cause copyright problems >> > > It is not copy from specification, it is only inspired by OHCI > specification. I wrote it mainly for myself (and possibly others) to > explain what is the main change against previous algorithm, which is not > safe (from my point of view). It can be removed if it is not > interesting. > Ok then. > "Copied" are "official" names of OHCI registers, bits etc., i.e. > HccaDoneHead, WDH bit, HcInterruptStatus etc. - but from my point of > view it could be no problem - it is "keyword" which is necessary to > "speak with the same language". Ok, then. > And it should be aim of such > specifications, specially of such "free" specifications as USB > specifications are. Or not? > > Normally yes, but the details of license may be a problem. Many specifications has a clause which forbids modification. GPL on the other hand forbids forbiding modifications. So they are incompatible. > > Some questions and others: > > I am not so far oriented in such kind of development, so I am asking: > Will You correct my patch yourself (as maintainer of code) or should I > prepare new corrected code and related patch? And against which version > - I noted that in meantime new official version was released... > Normally we ask contributors to clean their patches themselves. But given the great importance of your changes I'll cleaned them up myself. You can find the partially cleaned version in attachment. I recommend using bzr trunk but USB code hasn't changed since few months due to lack of dev time > Did You tests on OHCI/UHCI and what was the result? > What HW are You using for testing - some real PC or qemu ? > > I have a computer with integrated UHCI and OHCI+EHCI PCI card. Testing on it showed that your patch fixed UHCI but had no effect on OHCI. with OHCI grub_ohci_detect_dev always sees status = 0. I see the similar behaviour when testing usb on grub-yeeloong-firmware (but not when chinloaded by pmon). I guess some register initialisation is missing. > It was little bit surprise for me that OHCI and UHCI drivers are not > using IRQ (and maybe also some other drivers). Is it intention of GRUB? > Nothing against such philosophy, some things are easier in this way. > > GRUB has chosen to be single thread for reducing complexity. But we have plans to add IRQs nevertheless because it will easier porting drivers and allow parallel uncompress and read. > I shortly read the UHCI specification and I was thought about my > comments of TD buffer size optimalization included in patch - such > optimization cannot be done simply because TD preparation code is common > for OHCI and UHCI and: > - UHCI needs (as far as I understand) TD buffer size length up to packet > size only (<=64 bytes). > - In contradiction, OHCI allows up to 8Kbytes TD buffer size (if buffer > is aligned to page). > So there is the question if it make sense to do any optimization for > OHCI - probably not. In fact slow data transfer and wasting of memory > caused by not optimized TD buffer is probably not so big problem. > > Keep it simple in this case. > I read also part of EHCI specification. There is question if GRUB 2 > needs support of EHCI because: > 1. According to EHCI specification, each EHCI interface should normaly > have also OHCI (or UHCI ?) interface. So, any USB 2.0 interface should > work (slowly) via GRUB2 UHCI/OHCI support. > 2. EHCI is relative new HW - so it was usually used on PCs with BIOS > which supports booting from USB - so, probably, there is not necessary > to prepare special support in GRUB for such HW. But maybe I am wrong. > > Unfortunately BIOS is far from being bugless especially in USB. We have seen e.g. 128GiB limits. On Yeeloong grub is the firmware. And this reasoning doesn't apply to PCI expansion cards. And on my laptop a bug in legacy support prevents UHCI controller from working properly w/o EHCI. > Best regards > Ales > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko