| Message ID | 1259659962-4113-2-git-send-email-bryan.wu@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Dec 01, 2009 at 05:32:42PM +0800, Bryan Wu wrote: > From: Dinh Nguyen <r00091@freescale.com> > > BugLink: https://bugs.launchpad.net/bug/431963 Missing an 's' here. Had me going for a bit there. I had a quick look at the bug, and the following is the text associated with this patch: I think I have found a fix for this issue. I have attached the patch. It is probably not the correct fix at the moment, and more analysis is needed, but for now it appears that with 9.10, there is an invalid command that is coming to the USB/SATA bridge chip. Instead of gracefully handling this command, the USB/SATA bridge chip on the Babbage board is failing on this command. This tends to imply its just a debug patch and not the final fix? Can we confirm its the final fix? -apw
Andy Whitcroft 写道: > On Tue, Dec 01, 2009 at 05:32:42PM +0800, Bryan Wu wrote: >> From: Dinh Nguyen <r00091@freescale.com> >> >> BugLink: https://bugs.launchpad.net/bug/431963 > > Missing an 's' here. Had me going for a bit there. > Sorry about that, -:). > I had a quick look at the bug, and the following is the text associated > with this patch: > > I think I have found a fix for this issue. I have attached the > patch. It is probably not the correct fix at the moment, and more > analysis is needed, but for now it appears that with 9.10, there is an > invalid command that is coming to the USB/SATA bridge chip. Instead > of gracefully handling this command, the USB/SATA bridge chip on the > Babbage board is failing on this command. > > This tends to imply its just a debug patch and not the final fix? Can > we confirm its the final fix? > This patch is not a final fix, from my point of view. It is a workaround to make the bad USB chip work. Please find the comments from the USB chip vendor: --- We found that the key issue deciding Ubuntu 9.10 boot-up or not is “how we handle this ATA PASS-THROUGH command.” Original GL830 just pass the ATA PASS-THROUGH command and the SECTOR_COUNT field of its IDENTIFY command is 0, so GL830 seems it is an invalid command. As a result, GL830 just bypass this ATA PASS-THROUGH command to HDD and return USB Host a CSW OK. The hang-out situation is because HDD is processing the IDENTIFY command, but GL830 did not response HDD. We have tried to ask GL830 response USB Host a CSW FAIL of CSW STALL to skip this ATA PASS-THROUGH command, and the Ubuntu 9.10 can successfully boot-up. The USB/SATA cable you’re using must be performance this similar solution, so it can boot system up. The root cause of this weakness (the glitch you mentioned) is because GL830 doesn’t check the integrity of ATA PASS-THROUGH command. Since there are too many exceptions, we suppose the command sent by USB Host should be a valid one. --- Although it is a workaround and might not be accept by upstream, it is important to fix this critical bug in our Karmic i.MX51 release. Thanks -Bryan
On Tue, Dec 01, 2009 at 06:11:18PM +0800, Bryan Wu wrote: > Although it is a workaround and might not be accept by upstream, it > is important to fix this critical bug in our Karmic i.MX51 release. I can see that we might need to prevent command 0x85 passing thorugh for this controller to fix this issue, but the code seems to my eye to just stop all 0x85 commands to all USB storage devices regardless of where they are connected? That seems rather wide ranging and risky? -apw
Andy Whitcroft wrote: > On Tue, Dec 01, 2009 at 06:11:18PM +0800, Bryan Wu wrote: > >> Although it is a workaround and might not be accept by upstream, it >> is important to fix this critical bug in our Karmic i.MX51 release. > > I can see that we might need to prevent command 0x85 passing thorugh for > this controller to fix this issue, but the code seems to my eye to just > stop all 0x85 commands to all USB storage devices regardless of where > they are connected? That seems rather wide ranging and risky? > > -apw > At least all USB storage devices on all controllers. So it really should be more specific to the controller on the board. -Stefan
Stefan Bader 写道: > Andy Whitcroft wrote: >> On Tue, Dec 01, 2009 at 06:11:18PM +0800, Bryan Wu wrote: >> >>> Although it is a workaround and might not be accept by upstream, it >>> is important to fix this critical bug in our Karmic i.MX51 release. >> I can see that we might need to prevent command 0x85 passing thorugh for >> this controller to fix this issue, but the code seems to my eye to just >> stop all 0x85 commands to all USB storage devices regardless of where >> they are connected? That seems rather wide ranging and risky? >> >> -apw >> > > At least all USB storage devices on all controllers. So it really should > be more specific to the controller on the board. > > -Stefan Yeah, we need to only stop passing 0x85 to GL830 chip, while keep others intact. Thanks for pointing that out, I will rework the patch and resend them out after testing. -Bryan
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 8060b85..11dd37d 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -329,8 +329,11 @@ static int usb_stor_control_thread(void * __us) /* we've got a command, let's do it! */ else { - US_DEBUG(usb_stor_show_command(us->srb)); - us->proto_handler(us->srb, us); + US_DEBUGP(usb_stor_show_command(us->srb)); +#ifdef CONFIG_MACH_MX51_BABBAGE + if (us->srb->cmnd[0] != 0x85) +#endif + us->proto_handler(us->srb, us); } /* lock access to the state */