Using get_last_written() in sr.c causes problems with some usb devices (bz.kernel.org 9668)

Using get_last_written() in sr.c causes problems with some usb devices (bz.kernel.org 9668)

[Hans de Goede]

Hi Tejun & List,

I've been debugging a problem where Linux would not talk to the *emulated*
cdrom drive on a usb picture frame.

This picture frame comes with internal flash, and presents itself as a
cdrom device, which contains windows software to store photo's into the
internal flash (through the use of proprietary scsi commands).

I've written userspace software (part of libgphoto2) to be able to access
the internal flash under Linux. But sr.c does things to the frame
it does not like which keeps causing usb resets.

After about a day of debugging I've tracked the problem down to the
get_last_written() call done from get_sectorsize() in sr.c, which was
added by this patch:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5915136d4d3954867cced8a2297bddd16caf36a1

Specifically the READ_DISC_INFORMATION scsi command get_last_written()
sends is the problem. If I remove the get_last_written() call from
get_sectorsize() in sr.c, or if I patch the usb-storage driver to
filter out (fail as invalid request) READ_DISC_INFORMATION scsi
commands. The problem with the frame's cdrom emulation goes away,
one can see the windows software and more importantly the continues
usb resets stop.

Notice that after a single READ_DISC_INFORMATION scsi command, the
device is fu-barred, and the usb resets do not really help, the
device continues talking to us, but reports bogus values for
various things (sector size amongst others).

Note the cdrom emulation of the device
indicates it not liking the READ_DISC_INFORMATION by responding
with a Unit Attention sense code, with additional sense values:
3A 00 (medium not present), which is all fsck-ed up.

So the above is the analysis of the problem, now the question is
how to fix this...

The first thing to notice here is that according to MMC3
READ_DISC_INFORMATION is not necessarily implemented by CD-ROM
and DVD-ROM drives, so we may want to check the drive is some sort
of recordable before calling it, not that that will help
in this case as the picture frame claims to be a CD-R <sigh>.

The only thing I can come up with to fix this is a device specific
quirk. Either in sr.c or in ubs-storage. Which is not a very
pretty solution, hence this mail in the hope someone has
a clever idea.

Regards,

Hans

Re: Using get_last_written() in sr.c causes problems with some usb devices (bz.kernel.org 9668)

[Tejun Heo]

Hello,

On 04/27/2010 11:43 AM, Hans de Goede wrote:
> The only thing I can come up with to fix this is a device specific
> quirk. Either in sr.c or in ubs-storage. Which is not a very
> pretty solution, hence this mail in the hope someone has
> a clever idea.

For cases like this, I think quirking is the cleanest solution.
Trying to modify generic behavior to accomodate special cases
sometimes create unexpected subtle issues in other devices and as the
fix itself is buried in the general seemingly unrelated logic, it's
relatively easy to lose the workaround later during an unrelated
update.

Is the device uniquely identifiable?

Thanks.

-- 
tejun

Re: Using get_last_written() in sr.c causes problems with some usb devices (bz.kernel.org 9668)

[Hans de Goede]

Hi,

On 04/27/2010 11:50 AM, Tejun Heo wrote:
> Hello,
>
> On 04/27/2010 11:43 AM, Hans de Goede wrote:
>> The only thing I can come up with to fix this is a device specific
>> quirk. Either in sr.c or in ubs-storage. Which is not a very
>> pretty solution, hence this mail in the hope someone has
>> a clever idea.
>
> For cases like this, I think quirking is the cleanest solution.
> Trying to modify generic behavior to accomodate special cases
> sometimes create unexpected subtle issues in other devices and as the
> fix itself is buried in the general seemingly unrelated logic, it's
> relatively easy to lose the workaround later during an unrelated
> update.
>
> Is the device uniquely identifiable?
>

Yes it can be uniquely identified from its USB-ID, which
using a usb-storage unusual devs entry for this with a new flag
is the best solution.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html