Project

General

Profile

Actions

Bug #13817

closed

scsa2usb quirks list does not correctly use revision field

Added by Joshua M. Clulow 6 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Category:
driver - device drivers
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

The USB mass storage driver (scsa2usb) handles device-specific quirks of implementation with a table of IDs. This table includes three columns for filtering purposes:

        uint16_t        idVendor;       /* vendor ID                    */
        uint16_t        idProduct;      /* product ID                   */
        uint16_t        bcdDevice;      /* device release number in bcd */

A wildcard value (the macro X) may be specified for either idProduct or bcdDevice, which would match any value.

Unfortunately, the bcdDevice column of this table is entirely ignored at present. One or two devices have correctly specified the wildcard X for the device revision, but most entries specify 0. It is almost certainly impossible to go back now and divine what value is correct for all of our existing quirks entries, so we must instead change every entry to a wildcard match as part of fixing this logic error.

While we're there, we'll clean up the code and the comments a bit.


Related issues

Related to illumos gate - Bug #13818: Insyde BMC virtual CD-ROM confused by MODE SENSEClosedJoshua M. Clulow

Actions
Actions #1

Updated by Electric Monk 6 months ago

  • Gerrit CR set to 1505
Actions #2

Updated by Joshua M. Clulow 5 months ago

  • Related to Bug #13818: Insyde BMC virtual CD-ROM confused by MODE SENSE added
Actions #3

Updated by Joshua M. Clulow 5 months ago

Testing Notes

This change was tested along with #13818, where I had hardware that appeared on the quirks list and required a quirk attribute to function correctly.

In addition, I did a basic smoke test of a build of this change without #13818, with a device that does not appear on the quirks list:

> *scsa2usb_statep::walk softstate | ::print struct scsa2usb_state scsa2usb_dip | ::devinfo -q
fffffe2ced64abc8 usbif,class8, instance #0 (driver name: scsa2usb)
> fffffe2ced64abc8::devinfo ! grep -A1 usb-.*-id
            name='usb-revision-id' type=int items=1
                value=00000000
            name='usb-product-id' type=int items=1
                value=00002343
            name='usb-vendor-id' type=int items=1
                value=00000bc2

> *scsa2usb_statep::walk softstate | ::print struct scsa2usb_state scsa2usb_attrs
scsa2usb_attrs = 0xffff
> ~ffff=x
                0

> scsa2usb_quirks::print ! tail
        q_rev = 0xffff
        q_attr = 0x801
    },
    {
        q_vid = 0x1058
        q_pid = 0x1001
        q_rev = 0xffff
        q_attr = 0x4000
    },
]
# diskinfo
TYPE    DISK                    VID      PID              SIZE          RMV SSD
SCSI    c1t0d0                  Seagate  Portable         3726.02 GiB   no  no
...
Actions #4

Updated by Electric Monk 5 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 47b9747f90c9eb7e841fb67c110f8cc5fca20cd1

commit  47b9747f90c9eb7e841fb67c110f8cc5fca20cd1
Author: Joshua M. Clulow <josh@sysmgr.org>
Date:   2021-06-29T18:29:52.000Z

    13817 scsa2usb quirks list does not correctly use revision field
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Actions

Also available in: Atom PDF