Page MenuHomeFreeBSD
Differential D30919 Diff 103094 sys/dev/usb/usb_msctest.c

Changeset View

Standalone View

View Options

sys/dev/usb/usb_msctest.c

/* $FreeBSD$ */ /* $FreeBSD$ */
/*- /*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
* *
* Copyright (c) 2008,2011 Hans Petter Selasky. All rights reserved. * Copyright (c) 2008-2022 Hans Petter Selasky.
* Copyright (c) 2021-2022 Idwer Vollering.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
* are met: * are met:
* 1. Redistributions of source code must retain the above copyright * 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer. * notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright * 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the * notice, this list of conditions and the following disclaimer in the
Show All 10 Lines
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE. * SUCH DAMAGE.
*/ */
/* /*
* The following file contains code that will detect USB autoinstall * The following file contains code that will detect USB autoinstall
* disks. * disks.
*
* TODO: Potentially we could add code to automatically detect USB
* mass storage quirks for not supported SCSI commands!
*/ */
#ifdef USB_GLOBAL_INCLUDE_FILE #ifdef USB_GLOBAL_INCLUDE_FILE
#include USB_GLOBAL_INCLUDE_FILE #include USB_GLOBAL_INCLUDE_FILE
#else #else
#include <sys/stdint.h> #include <sys/stdint.h>
#include <sys/stddef.h> #include <sys/stddef.h>
#include <sys/param.h> #include <sys/param.h>
Show All 21 Lines
#include <dev/usb/usb_busdma.h> #include <dev/usb/usb_busdma.h>
#include <dev/usb/usb_process.h> #include <dev/usb/usb_process.h>
#include <dev/usb/usb_transfer.h> #include <dev/usb/usb_transfer.h>
#include <dev/usb/usb_msctest.h> #include <dev/usb/usb_msctest.h>
#include <dev/usb/usb_debug.h> #include <dev/usb/usb_debug.h>
#include <dev/usb/usb_device.h> #include <dev/usb/usb_device.h>
#include <dev/usb/usb_request.h> #include <dev/usb/usb_request.h>
#include <dev/usb/usb_util.h> #include <dev/usb/usb_util.h>
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

This looks ugly, it's here to include usb_quirk_str[]

vidwer_fbsdbugs_gmail.com: This looks ugly, it's here to include usb_quirk_str[]
hselaskyAuthorUnsubmitted
Done
Inline Actions

Please export the structure via the usb_quirk.h, like an extern declaration.

hselasky: Please export the structure via the usb_quirk.h, like an extern declaration.
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Please export the structure via the usb_quirk.h, like an extern declaration.

..and drop the 'static' keyword?

https://stackoverflow.com/questions/25288555/how-to-correctly-write-declarations-of-extern-arrays-in-cs-header-files

vidwer_fbsdbugs_gmail.com: > Please export the structure via the usb_quirk.h, like an extern declaration. ..and drop the…
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Please export the structure via the usb_quirk.h, like an extern declaration.

vidwer_fbsdbugs_gmail.com: > Please export the structure via the usb_quirk.h, like an extern declaration.
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Please export the structure via the usb_quirk.h, like an extern declaration.

Yes, this needs to be marked as 'external', otherwise the printed quirk (e.g usb_quirk_str[UQ_MSC_NO_TEST_UNIT_READY] in the printf() below) will be (null).

vidwer_fbsdbugs_gmail.com: > Please export the structure via the usb_quirk.h, like an extern declaration. Yes, this needs…
#include <dev/usb/quirk/usb_quirk.h> #include <dev/usb/quirk/usb_quirk.h>
#endif /* USB_GLOBAL_INCLUDE_FILE */ #endif /* USB_GLOBAL_INCLUDE_FILE */
enum { enum {
ST_COMMAND, ST_COMMAND,
ST_DATA_RD, ST_DATA_RD,
ST_DATA_RD_CS, ST_DATA_RD_CS,
ST_DATA_WR, ST_DATA_WR,
Show All 11 Lines
#define SCSI_MAX_LEN MAX(SCSI_FIXED_BLOCK_SIZE, USB_MSCTEST_BULK_SIZE) #define SCSI_MAX_LEN MAX(SCSI_FIXED_BLOCK_SIZE, USB_MSCTEST_BULK_SIZE)
#define SCSI_INQ_LEN 0x24 #define SCSI_INQ_LEN 0x24
#define SCSI_SENSE_LEN 0xFF #define SCSI_SENSE_LEN 0xFF
#define SCSI_FIXED_BLOCK_SIZE 512 /* bytes */ #define SCSI_FIXED_BLOCK_SIZE 512 /* bytes */
static uint8_t scsi_test_unit_ready[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; static uint8_t scsi_test_unit_ready[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_inquiry[] = { 0x12, 0x00, 0x00, 0x00, SCSI_INQ_LEN, 0x00 }; static uint8_t scsi_inquiry[] = { 0x12, 0x00, 0x00, 0x00, SCSI_INQ_LEN, 0x00 };
static uint8_t scsi_rezero_init[] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; static uint8_t scsi_rezero_init[] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_start_stop_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x02, 0x00 }; static uint8_t scsi_start_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x01, 0x00 };
static uint8_t scsi_stop_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x02, 0x00 };
static uint8_t scsi_ztestor_eject[] = { 0x85, 0x01, 0x01, 0x01, 0x18, 0x01, static uint8_t scsi_ztestor_eject[] = { 0x85, 0x01, 0x01, 0x01, 0x18, 0x01,
0x01, 0x01, 0x01, 0x01, 0x00, 0x00 }; 0x01, 0x01, 0x01, 0x01, 0x00, 0x00 };
static uint8_t scsi_cmotech_eject[] = { 0xff, 0x52, 0x44, 0x45, 0x56, 0x43, static uint8_t scsi_cmotech_eject[] = { 0xff, 0x52, 0x44, 0x45, 0x56, 0x43,
0x48, 0x47 }; 0x48, 0x47 };
static uint8_t scsi_huawei_eject[] = { 0x11, 0x06, 0x00, 0x00, 0x00, 0x00, static uint8_t scsi_huawei_eject[] = { 0x11, 0x06, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00 }; 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_huawei_eject2[] = { 0x11, 0x06, 0x20, 0x00, 0x00, 0x01, static uint8_t scsi_huawei_eject2[] = { 0x11, 0x06, 0x20, 0x00, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00 }; 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_tct_eject[] = { 0x06, 0xf5, 0x04, 0x02, 0x52, 0x70 }; static uint8_t scsi_tct_eject[] = { 0x06, 0xf5, 0x04, 0x02, 0x52, 0x70 };
static uint8_t scsi_sync_cache[] = { 0x35, 0x00, 0x00, 0x00, 0x00, 0x00, static uint8_t scsi_sync_cache[] = { 0x35, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00 }; 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_request_sense[] = { 0x03, 0x00, 0x00, 0x00, 0x12, 0x00, static uint8_t scsi_request_sense[] = { 0x03, 0x00, 0x00, 0x00, 0x12, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_read_capacity[] = { 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, static uint8_t scsi_read_capacity[] = { 0x25, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00 }; 0x00, 0x00, 0x00, 0x00 };
static uint8_t scsi_prevent_removal[] = { 0x1e, 0, 0, 0, 1, 0 }; static uint8_t scsi_prevent_removal[] = { 0x1e, 0, 0, 0, 1, 0 };
static uint8_t scsi_allow_removal[] = { 0x1e, 0, 0, 0, 0, 0 }; static uint8_t scsi_allow_removal[] = { 0x1e, 0, 0, 0, 0, 0 };
#ifndef USB_MSCTEST_BULK_SIZE #ifndef USB_MSCTEST_BULK_SIZE
hselaskyAuthorUnsubmitted
Done
Inline Actions

Is this comment correct? Isn't the first byte part of a SCSI command?

hselasky: Is this comment correct? Isn't the first byte part of a SCSI command?
#define USB_MSCTEST_BULK_SIZE 64 /* dummy */ #define USB_MSCTEST_BULK_SIZE 64 /* dummy */
#endif #endif
hselaskyAuthorUnsubmitted
Done
Inline Actions

Does this code compile if you use "static const" here?

hselasky: Does this code compile if you use "static const" here?
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Does this code compile if you use "static const" here?

(..)/sys/dev/usb/usb_msctest.c:849:7: error: passing 'const uint8_t (*)[6]' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]

&scsi_quirk_no_test_unit_ready, sizeof(scsi_quirk_no_test_unit_ready),
vidwer_fbsdbugs_gmail.com: > Does this code compile if you use "static const" here? (..)/sys/dev/usb/usb_msctest.c:849:7…
hselaskyAuthorUnsubmitted
Done
Inline Actions

I see.

hselasky: I see.
hselaskyAuthorUnsubmitted
Done
Inline Actions

Can you explain what SCSI commands the entries above represent. Looks invalid to me!

hselasky: Can you explain what SCSI commands the entries above represent. Looks invalid to me!
#define ERR_CSW_FAILED -1 #define ERR_CSW_FAILED -1
hselaskyAuthorUnsubmitted
Done
Inline Actions

Ditto.

hselasky: Ditto.
/* Command Block Wrapper */ /* Command Block Wrapper */
struct bbb_cbw { struct bbb_cbw {
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

For UQ_MSC_FORCE_PROTO_SCSI, the byte sequence probably will look like this:

static uint8_t scsi_quirk_force_proto_scsi[] = { 0x2e, 0, 0, 0, 0, 0 };

vidwer_fbsdbugs_gmail.com: For UQ_MSC_FORCE_PROTO_SCSI, the byte sequence probably will look like this: static uint8_t…
uDWord dCBWSignature; uDWord dCBWSignature;
#define CBWSIGNATURE 0x43425355 #define CBWSIGNATURE 0x43425355
uDWord dCBWTag; uDWord dCBWTag;
uDWord dCBWDataTransferLength; uDWord dCBWDataTransferLength;
uByte bCBWFlags; uByte bCBWFlags;
#define CBWFLAGS_OUT 0x00 #define CBWFLAGS_OUT 0x00
#define CBWFLAGS_IN 0x80 #define CBWFLAGS_IN 0x80
uByte bCBWLUN; uByte bCBWLUN;
▲ Show 20 Lines • Show All 617 Lines • ▼ Show 20 Linesusb_msc_get_max_lun(struct usb_device *udev, uint8_t iface_index)
err = usbd_do_request(udev, NULL, &req, &buf); err = usbd_do_request(udev, NULL, &req, &buf);
if (err) if (err)
buf = 0; buf = 0;
return (buf); return (buf);
} }
#define USB_ADD_QUIRK(udev, any, which) do { \
if (usb_get_manufacturer(udev) != NULL && usb_get_product(udev) != NULL) { \
hselaskyAuthorUnsubmitted
Done
Inline Actions

You should use a pointer to uaa, and not copy the structure itself.

hselasky: You should use a pointer to uaa, and not copy the structure itself.
DPRINTFN(0, #which " set for USB mass storage device %s %s (0x%04x:0x%04x)\n", \
usb_get_manufacturer(udev), \
usb_get_product(udev), \
UGETW(udev->ddesc.idVendor), \
UGETW(udev->ddesc.idProduct)); \
} else { \
DPRINTFN(0, #which " set for USB mass storage device, 0x%04x:0x%04x\n", \
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

It should read DPRINTFN(), then it will compile.

vidwer_fbsdbugs_gmail.com: It should read DPRINTFN(), then it will compile.
vidwer_fbsdbugs_gmail.comUnsubmitted
Not Done
Inline Actions

The parameter text 'Applying ...' seems to get lost in both the if and else case. In other words: I don't see per-device quirk details in dmesg.

vidwer_fbsdbugs_gmail.com: The parameter text 'Applying ...' seems to get lost in both the if and else case. In other…
hselaskyAuthorUnsubmitted
Done
Inline Actions

Did you add:

options USB_DEBUG

to your kernel configuration file?

hselasky: Did you add: options USB_DEBUG to your kernel configuration file?
hselaskyAuthorUnsubmitted
Done
Inline Actions

And the code is compiled with:

options USB_DEBUG

???

hselasky: And the code is compiled with: options USB_DEBUG ???
vidwer_fbsdbugs_gmail.comUnsubmitted
Not Done
Inline Actions

USB_DEBUG is set/present in sys/amd64/conf/GENERIC

vidwer_fbsdbugs_gmail.com: USB_DEBUG is set/present in sys/amd64/conf/GENERIC
UGETW(udev->ddesc.idVendor), \
UGETW(udev->ddesc.idProduct)); \
} \
usbd_add_dynamic_quirk(udev, which); \
any = 1; \
} while (0)
usb_error_t usb_error_t
usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index) usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index,
const struct usb_attach_arg *uaa)
{ {
struct bbb_transfer *sc; struct bbb_transfer *sc;
uint8_t timeout; uint8_t timeout;
uint8_t is_no_direct; uint8_t is_no_direct;
uint8_t sid_type; uint8_t sid_type;
uint8_t any_quirk;
int err; int err;
sc = bbb_attach(udev, iface_index, UICLASS_MASS); sc = bbb_attach(udev, iface_index, UICLASS_MASS);
if (sc == NULL) if (sc == NULL)
return (0); return (0);
any_quirk = 0;
vidwer_fbsdbugs_gmail.comUnsubmitted
Not Done
Inline Actions

whitespace

vidwer_fbsdbugs_gmail.com: whitespace
/* /*
* Some devices need a delay after that the configuration * Some devices need a delay after that the configuration
* value is set to function properly: * value is set to function properly:
*/ */
usb_pause_mtx(NULL, hz); usb_pause_mtx(NULL, hz);
if (usb_msc_get_max_lun(udev, iface_index) == 0) { if (usb_test_quirk(uaa, UQ_MSC_NO_GETMAXLUN) == 0 &&
usb_msc_get_max_lun(udev, iface_index) == 0) {
DPRINTF("Device has only got one LUN.\n"); DPRINTF("Device has only got one LUN.\n");
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_GETMAXLUN); USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_GETMAXLUN);
} }
is_no_direct = 1; is_no_direct = 1;
for (timeout = 4; timeout != 0; timeout--) { for (timeout = 4; timeout != 0; timeout--) {
err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, err = bbb_command_start(sc, DIR_IN, 0, sc->buffer,
SCSI_INQ_LEN, &scsi_inquiry, sizeof(scsi_inquiry), SCSI_INQ_LEN, &scsi_inquiry, sizeof(scsi_inquiry),
USB_MS_HZ); USB_MS_HZ);
Show All 10 Lines for (timeout = 4; timeout != 0; timeout--) {
usb_pause_mtx(NULL, hz); usb_pause_mtx(NULL, hz);
} }
if (is_no_direct) { if (is_no_direct) {
DPRINTF("Device is not direct access.\n"); DPRINTF("Device is not direct access.\n");
goto done; goto done;
} }
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, if (usb_test_quirk(uaa, UQ_MSC_NO_TEST_UNIT_READY) == 0) {
err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0,
&scsi_test_unit_ready, sizeof(scsi_test_unit_ready), &scsi_test_unit_ready, sizeof(scsi_test_unit_ready),
USB_MS_HZ); USB_MS_HZ);
if (err != 0) { if (err != 0) {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
DPRINTF("Test unit ready failed\n"); USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_TEST_UNIT_READY);
} }
}
err = bbb_command_start(sc, DIR_OUT, 0, NULL, 0, if (usb_test_quirk(uaa, UQ_MSC_NO_PREVENT_ALLOW) == 0) {
hselaskyAuthorUnsubmitted
Done
Inline Actions

When err is zero, it means the command was successful, and then you apply the quirk?
That will end up applying the TEST UNIT READY quirk for _all_ USB mass storage devices?

Please explain the logic here. I don't get it!

--HPS

hselasky: When err is zero, it means the command was successful, and then you apply the quirk? That will…
err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0,
&scsi_prevent_removal, sizeof(scsi_prevent_removal), &scsi_prevent_removal, sizeof(scsi_prevent_removal),
USB_MS_HZ); USB_MS_HZ);
impUnsubmitted
Done
Inline Actions

The question here is whether or not sending this command, at all, hangs the drive and if so, do we have a quirk that prevents that.

imp: The question here is whether or not sending this command, at all, hangs the drive and if so, do…
hselaskyAuthorUnsubmitted
Done
Inline Actions

Maybe if the quirks is already set, don't query for it? That would avoid hanging devices.

Also, a list of USB mass storage devices and detected quirks would be nice!

hselasky: Maybe if the quirks is already set, don't query for it? That would avoid hanging devices. Also…
impUnsubmitted
Done
Inline Actions

Maybe if the quirks is already set, don't query for it? That would avoid hanging devices.

That's what I'm thinking. There's no need to probe fore something that's already quirked out. And that would be the best way to 'fail safe' with this bug. It's no worse than today, but future entries can be avoided...

imp: > Maybe if the quirks is already set, don't query for it? That would avoid hanging devices.
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Maybe if the quirks is already set, don't query for it? That would avoid hanging devices.

Right. It seems quirks are primarily set at sc_quirks in umass_softc.

Also, a list of USB mass storage devices and detected quirks would be nice!

So print vendor and product from scsi_inquiry_data.

vidwer_fbsdbugs_gmail.com: > Maybe if the quirks is already set, don't query for it? That would avoid hanging devices.
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

I'm not sure how to add and pass umass_probe_proto or umass_softc (for quirks or sc_quirks) in usb_msctest.c

vidwer_fbsdbugs_gmail.com: I'm not sure how to add and pass umass_probe_proto or umass_softc (for quirks or sc_quirks) in…
hselaskyAuthorUnsubmitted
Done
Inline Actions

Should be DIR_NONE when the datalength is zero.

hselasky: Should be DIR_NONE when the datalength is zero.
if (err == 0) { if (err == 0) {
err = bbb_command_start(sc, DIR_OUT, 0, NULL, 0, err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0,
&scsi_allow_removal, sizeof(scsi_allow_removal), &scsi_allow_removal, sizeof(scsi_allow_removal),
hselaskyAuthorUnsubmitted
Done
Inline Actions

For the final version the printf() should be replaced with debug prints:

DPRINTF()'s

Controlled by hw.usb.debug

hselasky: For the final version the printf() should be replaced with debug prints: DPRINTF()'s…
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

The additional text is now falling off, even with hw.usb.debug=0.

vidwer_fbsdbugs_gmail.com: The additional text is now falling off, even with hw.usb.debug=0.
USB_MS_HZ); USB_MS_HZ);
} }
hselaskyAuthorUnsubmitted
Done
Inline Actions

Please use usb_get_manufacturer() and usb_get_product().

udev->manufacturer and udev->product may be NULL.

hselasky: Please use usb_get_manufacturer() and usb_get_product(). udev->manufacturer and udev->product…
if (err != 0) { if (err != 0) {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
impUnsubmitted
Not Done
Inline Actions

Same comment as above

imp: Same comment as above
goto error; goto error;
DPRINTF("Device doesn't handle prevent and allow removal\n"); USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_PREVENT_ALLOW);
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_PREVENT_ALLOW);
} }
}
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

The amount of broken hardware I have or can borrow is finite. Future contributions would the use of the 'error' label to be present in the last check before the 'retry_sync_cache' label.

vidwer_fbsdbugs_gmail.com: The amount of broken hardware I have or can borrow is finite. Future contributions would the…
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

The amount of broken hardware I have or can borrow is finite. Future contributions would the use of the 'error' label to be present in the last check before the 'retry_sync_cache' label.

"Future contributions would require the use of the 'error' label to be present in the last check before the 'retry_sync_cache' label."

vidwer_fbsdbugs_gmail.com: > The amount of broken hardware I have or can borrow is finite. Future contributions would the…
timeout = 1; timeout = 1;
retry_sync_cache: retry_sync_cache:
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0,
&scsi_sync_cache, sizeof(scsi_sync_cache), &scsi_sync_cache, sizeof(scsi_sync_cache),
USB_MS_HZ); USB_MS_HZ);
if (err != 0) { if (err != 0) {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
hselaskyAuthorUnsubmitted
Done
Inline Actions

Ditto.

hselasky: Ditto.
DPRINTF("Device doesn't handle synchronize cache\n"); USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE);
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_SYNC_CACHE);
} else { } else {
/* /*
* Certain Kingston memory sticks fail the first * Certain Kingston memory sticks fail the first
* read capacity after a synchronize cache command * read capacity after a synchronize cache command
* has been issued. Disable the synchronize cache * has been issued. Disable the synchronize cache
* command for such devices. * command for such devices.
*/ */
err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, 8, err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, 8,
&scsi_read_capacity, sizeof(scsi_read_capacity), &scsi_read_capacity, sizeof(scsi_read_capacity),
USB_MS_HZ); USB_MS_HZ);
if (err != 0) { if (err != 0) {
hselaskyAuthorUnsubmitted
Done
Inline Actions

You should also handle timeouts here, not only CSW_FAILED.

hselasky: You should also handle timeouts here, not only CSW_FAILED.
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Handle timeouts (with USB_ERR_TIMEOUT) just in this place, or in both places?

vidwer_fbsdbugs_gmail.com: Handle timeouts (with USB_ERR_TIMEOUT) just in this place, or in both places?
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, 8, err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, 8,
&scsi_read_capacity, sizeof(scsi_read_capacity), &scsi_read_capacity, sizeof(scsi_read_capacity),
USB_MS_HZ); USB_MS_HZ);
if (err == 0) { if (err == 0) {
if (timeout--) if (timeout--)
goto retry_sync_cache; goto retry_sync_cache;
DPRINTF("Device most likely doesn't " USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE);
"handle synchronize cache\n");
usbd_add_dynamic_quirk(udev,
UQ_MSC_NO_SYNC_CACHE);
} else { } else {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
} }
} }
} }
if (usb_test_quirk(uaa, UQ_MSC_NO_START_STOP) == 0) {
err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0,
&scsi_start_unit, sizeof(scsi_start_unit),
USB_MS_HZ);
if (err != 0) {
if (err != ERR_CSW_FAILED)
goto error;
USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_START_STOP);
}
}
/* clear sense status of any failed commands on the device */ /* clear sense status of any failed commands on the device */
err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, err = bbb_command_start(sc, DIR_IN, 0, sc->buffer,
SCSI_INQ_LEN, &scsi_inquiry, sizeof(scsi_inquiry), SCSI_INQ_LEN, &scsi_inquiry, sizeof(scsi_inquiry),
USB_MS_HZ); USB_MS_HZ);
DPRINTF("Inquiry = %d\n", err); DPRINTF("Inquiry = %d\n", err);
if (err != 0) { if (err != 0) {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
} }
err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, err = bbb_command_start(sc, DIR_IN, 0, sc->buffer,
SCSI_SENSE_LEN, &scsi_request_sense, SCSI_SENSE_LEN, &scsi_request_sense,
sizeof(scsi_request_sense), USB_MS_HZ); sizeof(scsi_request_sense), USB_MS_HZ);
DPRINTF("Request sense = %d\n", err); DPRINTF("Request sense = %d\n", err);
if (err != 0) { if (err != 0) {
if (err != ERR_CSW_FAILED) if (err != ERR_CSW_FAILED)
goto error; goto error;
} }
goto done;
error:
/* Apply most quirks */
USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE);
USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_PREVENT_ALLOW);
USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_TEST_UNIT_READY);
USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_START_STOP);
done: done:
bbb_detach(sc); bbb_detach(sc);
return (0);
error: if (any_quirk) {
hselaskyAuthorUnsubmitted
Done
Inline Actions

Error means we are no longer able to communicate and need to reset the device.

hselasky: Error means we are no longer able to communicate and need to reset the device.
bbb_detach(sc); /* Unconfigure device, to clear software data toggle. */
usbd_set_config_index(udev, USB_UNCONFIG_INDEX);
DPRINTF("Device did not respond, enabling all quirks\n"); /* Need to re-enumerate the device to clear its state. */
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_SYNC_CACHE);
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_PREVENT_ALLOW);
usbd_add_dynamic_quirk(udev, UQ_MSC_NO_TEST_UNIT_READY);
/* Need to re-enumerate the device */
usbd_req_re_enumerate(udev, NULL); usbd_req_re_enumerate(udev, NULL);
return (USB_ERR_STALLED); return (USB_ERR_STALLED);
} }
vidwer_fbsdbugs_gmail.comUnsubmitted
Done
Inline Actions

Is this line now obsolete?

vidwer_fbsdbugs_gmail.com: Is this line now obsolete?
hselaskyAuthorUnsubmitted
Done
Inline Actions

No we still need this.

hselasky: No we still need this.
/* No quirks were added, continue as usual. */
return (0);
}
usb_error_t usb_error_t
usb_msc_eject(struct usb_device *udev, uint8_t iface_index, int method) usb_msc_eject(struct usb_device *udev, uint8_t iface_index, int method)
{ {
struct bbb_transfer *sc; struct bbb_transfer *sc;
usb_error_t err; usb_error_t err;
sc = bbb_attach(udev, iface_index, UICLASS_MASS); sc = bbb_attach(udev, iface_index, UICLASS_MASS);
if (sc == NULL) if (sc == NULL)
return (USB_ERR_INVAL); return (USB_ERR_INVAL);
switch (method) { switch (method) {
case MSC_EJECT_STOPUNIT: case MSC_EJECT_STOPUNIT:
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
&scsi_test_unit_ready, sizeof(scsi_test_unit_ready), &scsi_test_unit_ready, sizeof(scsi_test_unit_ready),
USB_MS_HZ); USB_MS_HZ);
DPRINTF("Test unit ready status: %s\n", usbd_errstr(err)); DPRINTF("Test unit ready status: %s\n", usbd_errstr(err));
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
&scsi_start_stop_unit, sizeof(scsi_start_stop_unit), &scsi_stop_unit, sizeof(scsi_stop_unit),
USB_MS_HZ); USB_MS_HZ);
break; break;
case MSC_EJECT_REZERO: case MSC_EJECT_REZERO:
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
&scsi_rezero_init, sizeof(scsi_rezero_init), &scsi_rezero_init, sizeof(scsi_rezero_init),
USB_MS_HZ); USB_MS_HZ);
break; break;
case MSC_EJECT_ZTESTOR: case MSC_EJECT_ZTESTOR:
▲ Show 20 Lines • Show All 152 Lines • Show Last 20 Lines