| Message ID | 20091216115756.GA23682@shadowen.org |
|---|---|
| State | Accepted |
| Headers | show |
Andy Whitcroft wrote: > On Wed, Dec 16, 2009 at 05:21:12PM +0800, Bryan Wu wrote: >> From: Dinh Nguyen <r00091@freescale.com> >> >> BugLink: http://bugs.launchpad.net/bugs/431963 >> >> Original patch was from Dinh Nguyen. That one find the root cause >> of this SATA drive failure issue. The USB2SATA chip GL830 can not >> accept the ATA PASS THROUGH command. >> >> This patch will skip the ATA PASS THROUGH command only for GL830 >> USB device. So it will not effect other USB devices. >> >> Signed-off-by: Dinh Nguyen <r00091@freescale.com> >> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >> --- >> drivers/usb/storage/usb.c | 17 +++++++++++++++-- >> 1 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c >> index 8060b85..d538511 100644 >> --- a/drivers/usb/storage/usb.c >> +++ b/drivers/usb/storage/usb.c >> @@ -329,8 +329,21 @@ 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)); > > This changes ... > >> - us->proto_handler(us->srb, us); >> +#ifdef CONFIG_MACH_MX51_BABBAGE >> + u16 vid = >> + le16_to_cpu(us->pusb_dev->descriptor.idVendor); >> + u16 pid = >> + le16_to_cpu(us->pusb_dev->descriptor.idProduct); >> +#endif >> + US_DEBUGP(usb_stor_show_command(us->srb)); > > ... to this, which is wrong as the routine there is a void, so there is > nothing to print. I think this will translate into a compile error when > CONFIG_USB_STORAGE_DEBUG is enabled. > > It also seems wrong to be looking up and translating these ids for every > command when we only care for 0x85 commands. > >> +#ifdef CONFIG_MACH_MX51_BABBAGE >> + if ((us->srb->cmnd[0] == 0x85) && >> + (vid == 0x05e3) && >> + (pid == 0x0718)) >> + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); >> + else >> +#endif >> + us->proto_handler(us->srb, us); >> } >> >> /* lock access to the state */ > > So I propose to apply the patch as below. > > Comments? > > -apw > > commit 95860a73f4c1ebff91cbf03783027c15600361d7 > Author: Dinh Nguyen <r00091@freescale.com> > Date: Wed Dec 16 17:21:12 2009 +0800 > > Ubuntu: SAUCE: Workaround for SATA drive failure on Ubuntu installation > > BugLink: http://bugs.launchpad.net/bugs/431963 > > Original patch was from Dinh Nguyen. That one find the root cause > of this SATA drive failure issue. The USB2SATA chip GL830 can not > accept the ATA PASS THROUGH command. > > This patch will skip the ATA PASS THROUGH command only for GL830 > USB device. So it will not effect other USB devices. > > [apw@canonical.com: fixed debug and optimised.] > Signed-off-by: Dinh Nguyen <r00091@freescale.com> > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Andy Whitcroft <apw@canonical.com> > > diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c > index 8060b85..961bb1a 100644 > --- a/drivers/usb/storage/usb.c > +++ b/drivers/usb/storage/usb.c > @@ -330,6 +330,15 @@ 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)); > +#ifdef CONFIG_MACH_MX51_BABBAGE > + if ((us->srb->cmnd[0] == 0x85) && > + (le16_to_cpu(us->pusb_dev->descriptor.idVendor) > + == 0x05e3) && > + (le16_to_cpu(us->pusb_dev->descriptor.idProduct) > + == 0x0718)) > + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); > + else > +#endif > us->proto_handler(us->srb, us); > } > > Doh, a good catch on the additional P. And with the the checking of vendor id and product id in place, even I seem to notice it is done, which I totally failed to do this morning. So, ok, works better for me. -Stefan
Andy Whitcroft wrote: > On Wed, Dec 16, 2009 at 05:21:12PM +0800, Bryan Wu wrote: > >> From: Dinh Nguyen <r00091@freescale.com> >> >> BugLink: http://bugs.launchpad.net/bugs/431963 >> >> Original patch was from Dinh Nguyen. That one find the root cause >> of this SATA drive failure issue. The USB2SATA chip GL830 can not >> accept the ATA PASS THROUGH command. >> >> This patch will skip the ATA PASS THROUGH command only for GL830 >> USB device. So it will not effect other USB devices. >> >> Signed-off-by: Dinh Nguyen <r00091@freescale.com> >> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >> --- >> drivers/usb/storage/usb.c | 17 +++++++++++++++-- >> 1 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c >> index 8060b85..d538511 100644 >> --- a/drivers/usb/storage/usb.c >> +++ b/drivers/usb/storage/usb.c >> @@ -329,8 +329,21 @@ 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)); >> > > This changes ... > > >> - us->proto_handler(us->srb, us); >> +#ifdef CONFIG_MACH_MX51_BABBAGE >> + u16 vid = >> + le16_to_cpu(us->pusb_dev->descriptor.idVendor); >> + u16 pid = >> + le16_to_cpu(us->pusb_dev->descriptor.idProduct); >> +#endif >> + US_DEBUGP(usb_stor_show_command(us->srb)); >> > > ... to this, which is wrong as the routine there is a void, so there is > nothing to print. I think this will translate into a compile error when > CONFIG_USB_STORAGE_DEBUG is enabled. > > It also seems wrong to be looking up and translating these ids for every > command when we only care for 0x85 commands. > > >> +#ifdef CONFIG_MACH_MX51_BABBAGE >> + if ((us->srb->cmnd[0] == 0x85) && >> + (vid == 0x05e3) && >> + (pid == 0x0718)) >> + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); >> + else >> +#endif >> + us->proto_handler(us->srb, us); >> } >> >> /* lock access to the state */ >> > > So I propose to apply the patch as below. > > Comments? > > -apw > > commit 95860a73f4c1ebff91cbf03783027c15600361d7 > Author: Dinh Nguyen <r00091@freescale.com> > Date: Wed Dec 16 17:21:12 2009 +0800 > > Ubuntu: SAUCE: Workaround for SATA drive failure on Ubuntu installation > > BugLink: http://bugs.launchpad.net/bugs/431963 > > Original patch was from Dinh Nguyen. That one find the root cause > of this SATA drive failure issue. The USB2SATA chip GL830 can not > accept the ATA PASS THROUGH command. > > This patch will skip the ATA PASS THROUGH command only for GL830 > USB device. So it will not effect other USB devices. > > [apw@canonical.com: fixed debug and optimised.] > Signed-off-by: Dinh Nguyen <r00091@freescale.com> > Signed-off-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Andy Whitcroft <apw@canonical.com> > > diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c > index 8060b85..961bb1a 100644 > --- a/drivers/usb/storage/usb.c > +++ b/drivers/usb/storage/usb.c > @@ -330,6 +330,15 @@ 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)); > +#ifdef CONFIG_MACH_MX51_BABBAGE > + if ((us->srb->cmnd[0] == 0x85) && > + (le16_to_cpu(us->pusb_dev->descriptor.idVendor) > + == 0x05e3) && > + (le16_to_cpu(us->pusb_dev->descriptor.idProduct) > + == 0x0718)) > + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); > + else > +#endif > us->proto_handler(us->srb, us); > } > > Thanks a lot, Andy. I missed these 2 key points you mentioned here. Shall I repost the patch again or you will apply it. Cheers, -Bryan
On Wed, Dec 16, 2009 at 11:49:08PM +0800, Bryan Wu wrote: > Thanks a lot, Andy. I missed these 2 key points you mentioned here. > Shall I repost the patch again > or you will apply it. I'll apply it as you are happy. -apw
Bryan Wu wrote: > Andy Whitcroft wrote: >> On Wed, Dec 16, 2009 at 05:21:12PM +0800, Bryan Wu wrote: >> >>> From: Dinh Nguyen <r00091@freescale.com> >>> >>> BugLink: http://bugs.launchpad.net/bugs/431963 >>> >>> Original patch was from Dinh Nguyen. That one find the root cause >>> of this SATA drive failure issue. The USB2SATA chip GL830 can not >>> accept the ATA PASS THROUGH command. >>> >>> This patch will skip the ATA PASS THROUGH command only for GL830 >>> USB device. So it will not effect other USB devices. >>> >>> Signed-off-by: Dinh Nguyen <r00091@freescale.com> >>> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >>> --- >>> drivers/usb/storage/usb.c | 17 +++++++++++++++-- >>> 1 files changed, 15 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c >>> index 8060b85..d538511 100644 >>> --- a/drivers/usb/storage/usb.c >>> +++ b/drivers/usb/storage/usb.c >>> @@ -329,8 +329,21 @@ 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)); >>> >> This changes ... >> >> >>> - us->proto_handler(us->srb, us); >>> +#ifdef CONFIG_MACH_MX51_BABBAGE >>> + u16 vid = >>> + le16_to_cpu(us->pusb_dev->descriptor.idVendor); >>> + u16 pid = >>> + le16_to_cpu(us->pusb_dev->descriptor.idProduct); >>> +#endif >>> + US_DEBUGP(usb_stor_show_command(us->srb)); >>> >> ... to this, which is wrong as the routine there is a void, so there is >> nothing to print. I think this will translate into a compile error when >> CONFIG_USB_STORAGE_DEBUG is enabled. >> >> It also seems wrong to be looking up and translating these ids for every >> command when we only care for 0x85 commands. >> >> >>> +#ifdef CONFIG_MACH_MX51_BABBAGE >>> + if ((us->srb->cmnd[0] == 0x85) && >>> + (vid == 0x05e3) && >>> + (pid == 0x0718)) >>> + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); >>> + else >>> +#endif >>> + us->proto_handler(us->srb, us); >>> } >>> >>> /* lock access to the state */ >>> >> So I propose to apply the patch as below. >> >> Comments? >> >> -apw >> >> commit 95860a73f4c1ebff91cbf03783027c15600361d7 >> Author: Dinh Nguyen <r00091@freescale.com> >> Date: Wed Dec 16 17:21:12 2009 +0800 >> >> Ubuntu: SAUCE: Workaround for SATA drive failure on Ubuntu installation >> >> BugLink: http://bugs.launchpad.net/bugs/431963 >> >> Original patch was from Dinh Nguyen. That one find the root cause >> of this SATA drive failure issue. The USB2SATA chip GL830 can not >> accept the ATA PASS THROUGH command. >> >> This patch will skip the ATA PASS THROUGH command only for GL830 >> USB device. So it will not effect other USB devices. >> >> [apw@canonical.com: fixed debug and optimised.] >> Signed-off-by: Dinh Nguyen <r00091@freescale.com> >> Signed-off-by: Bryan Wu <bryan.wu@canonical.com> >> Signed-off-by: Andy Whitcroft <apw@canonical.com> >> >> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c >> index 8060b85..961bb1a 100644 >> --- a/drivers/usb/storage/usb.c >> +++ b/drivers/usb/storage/usb.c >> @@ -330,6 +330,15 @@ 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)); >> +#ifdef CONFIG_MACH_MX51_BABBAGE >> + if ((us->srb->cmnd[0] == 0x85) && >> + (le16_to_cpu(us->pusb_dev->descriptor.idVendor) >> + == 0x05e3) && >> + (le16_to_cpu(us->pusb_dev->descriptor.idProduct) >> + == 0x0718)) >> + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); >> + else >> +#endif >> us->proto_handler(us->srb, us); >> } >> >> > Thanks a lot, Andy. I missed these 2 key points you mentioned here. > Shall I repost the patch again > or you will apply it. > > Cheers, > -Bryan > Also applied to Karmic and prepared the new package. I'll upload it when I can (Launchpad is writable again). -Stefan
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 8060b85..961bb1a 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -330,6 +330,15 @@ 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)); +#ifdef CONFIG_MACH_MX51_BABBAGE + if ((us->srb->cmnd[0] == 0x85) && + (le16_to_cpu(us->pusb_dev->descriptor.idVendor) + == 0x05e3) && + (le16_to_cpu(us->pusb_dev->descriptor.idProduct) + == 0x0718)) + US_DEBUGP("Skip ATA PASS-THROUGH command\n"); + else +#endif us->proto_handler(us->srb, us); }