storage: Widen bcdDevice range for SanDisk SDDR-31 quirk

storage: Widen bcdDevice range for SanDisk SDDR-31 quirk

[Mark Knibbs]

On Thu, 7 Jun 2018, Alan Stern wrote:
> On Thu, 7 Jun 2018, Mark Knibbs wrote:
> > The SanDisk SDDR-31 needs the US_FL_FIX_CAPACITY quirk. Previously that
> > was only applied for bcdDevice 0x0009 but later firmware needs it too.
> >
> > The later firmware has bcdDevice 0x0022. On the assumption that any even
> > later firmware (if it exists) has the same problem I have widened the
> > bcdDevice range to 0x0000 to 0x00FF. (That would allow a patched firmware
> > which doesn't have the bug to use e.g. bcdDevice 0x0122.)
>
> That's not a very convincing argument. Either assume that all future devices
> will need the quirk, or don't make any assumptions and just bump the range up
> to 0x0022. In any case, there's no good reason for selecting 0x00FF as a
> cut-off point. We don't have any control over what bcdDevice values the vendor
> assigns to its firmwares. If they do go to the trouble of fixing the READ
> CAPACITY bug, what makes you think they would change the bcdDevice value to 
> something above 0x0100? Finally, 0x00FF is not a valid bcdDevice value. These
> are binary-coded decimal numbers, ranging up to 0x9999. (I know we have a bunch
> of entries set to 0xFFFF; they aren't valid either but at least they make a
> clear point.) So, I would accept a patch where the upper limit was set to any
> of 0x0022, 0x9999, or 0xFFFF. But 0x00FF just seems weird.

The SDDR-31 is long-discontinued and no longer supported by SanDisk. Given that
both known firmware versions have the READ CAPACITY bug, it's very unlikely any
fixed official firmware exists. But if it does, its bcdDevice value would
probably be 0x0099 or less.

I chose 0x00FF as upper value rather than 0x9999/0xFFFF because it should be
feasible to modify the 0x0022 firmware to fix that bug. 0x00FF would allow
the modified firmware to report e.g. 0x0122, so the quirk would not be
misapplied to it. (And 0x00FF instead of 0x0099 because devices which
report non-BCD values for bcdDevice exist.)

I'll re-send the patch with upper value 0x0022 then.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

storage: Widen bcdDevice range for SanDisk SDDR-31 quirk

[Alan Stern]

On Thu, 7 Jun 2018, Mark Knibbs wrote:

> The SanDisk SDDR-31 needs the US_FL_FIX_CAPACITY quirk. Previously that
> was only applied for bcdDevice 0x0009 but later firmware needs it too.
> 
> The later firmware has bcdDevice 0x0022. On the assumption that any even
> later firmware (if it exists) has the same problem I have widened the
> bcdDevice range to 0x0000 to 0x00FF. (That would allow a patched firmware
> which doesn't have the bug to use e.g. bcdDevice 0x0122.)

That's not a very convincing argument.  Either assume that all future 
devices will need the quirk, or don't make any assumptions and just 
bump the range up to 0x0022.

In any case, there's no good reason for selecting 0x00FF as a cut-off
point.  We don't have any control over what bcdDevice values the vendor
assigns to its firmwares.  If they do go to the trouble of fixing the
READ CAPACITY bug, what makes you think they would change the bcdDevice
value to something above 0x0100?

Finally, 0x00FF is not a valid bcdDevice value.  These are binary-coded
decimal numbers, ranging up to 0x9999.  (I know we have a bunch of
entries set to 0xFFFF; they aren't valid either but at least they
make a clear point.)

So, I would accept a patch where the upper limit was set to any of 
0x0022, 0x9999, or 0xFFFF.  But 0x00FF just seems weird.

Alan Stern

> Signed-off-by: Mark Knibbs <mark_k@iname.com>
> ---
> diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
> index 747d3a9..dfcceaf 100644
> --- a/drivers/usb/storage/unusual_devs.h
> +++ b/drivers/usb/storage/unusual_devs.h
> @@ -1044,7 +1044,7 @@ UNUSUAL_DEV(  0x0781, 0x0001, 0x0200, 0x0200,
>                 USB_SC_SCSI, USB_PR_CB, NULL,
>                 US_FL_SINGLE_LUN ),
>  
> -UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009,
> +UNUSUAL_DEV(  0x0781, 0x0002, 0x0000, 0x00ff,
>                 "SanDisk Corporation",
>                 "ImageMate CompactFlash USB",
>                 USB_SC_DEVICE, USB_PR_DEVICE, NULL,
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

storage: Widen bcdDevice range for SanDisk SDDR-31 quirk

[Mark Knibbs]

The SanDisk SDDR-31 needs the US_FL_FIX_CAPACITY quirk. Previously that
was only applied for bcdDevice 0x0009 but later firmware needs it too.

The later firmware has bcdDevice 0x0022. On the assumption that any even
later firmware (if it exists) has the same problem I have widened the
bcdDevice range to 0x0000 to 0x00FF. (That would allow a patched firmware
which doesn't have the bug to use e.g. bcdDevice 0x0122.)

Signed-off-by: Mark Knibbs <mark_k@iname.com>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index 747d3a9..dfcceaf 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1044,7 +1044,7 @@ UNUSUAL_DEV(  0x0781, 0x0001, 0x0200, 0x0200,
                USB_SC_SCSI, USB_PR_CB, NULL,
                US_FL_SINGLE_LUN ),
 
-UNUSUAL_DEV(  0x0781, 0x0002, 0x0009, 0x0009,
+UNUSUAL_DEV(  0x0781, 0x0002, 0x0000, 0x00ff,
                "SanDisk Corporation",
                "ImageMate CompactFlash USB",
                USB_SC_DEVICE, USB_PR_DEVICE, NULL,