On 17/05/11 22:24, Alan Stern wrote: > On Tue, 17 May 2011, Jan Andersson wrote: >> You are right that the comment could be shortened, but readw also >> byteswaps so the remark about readl would just be changed to readw :-) > > Not really. Changing the remark to refer to readw makes no sense, > because the whole point of that code is to test whether the 0x80 bit > ends up being set, in order to see whether or not the bytes were > swapped. > > In fact, I would have written the whole comment differently. > Something like this: > > /* > * Probe to determine the endianness of the controller. > * We know that bit 7 of the PORTSC1 register is always set > * and bit 15 is always clear. If uhci_readw() yields a value > * with bit 7 (0x80) turned on then the current little-endian > * setting is correct. Otherwise we assume the value was > * byte-swapped; hence the register interface and presumably > * also the descriptors are big-endian. > */ > >>> Apart from this minor detail, the patch series is fine with me. You >>> can resubmit this last patch if you want to fix it up, and Greg can add >>> >>> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> >>> >>> to patches 1/5, 3/5, and 5/5. >>> >> >> Thank you. I want to test if I change to readw so that I don't make a >> mistake with all the swaps that take place. I will resubmit this after >> testing tomorrow - unless you can accept the use of readl() above. > > It's entirely up to you. Since the code works okay either way, I don't > care very much. > Well, I agree that using readw() is the right thing to do. And fixing that also give the opportunity to include your version of the comment, which is better. I'll send an updated set of patches after testing. Thanks for all your comments! Best regards, Jan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
