Patch for iriver-mp3player

Patch for iriver-mp3player

[Tobias Lieber]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi

i Javier Marcet wrote in January 2004 a patch for linux-2.6.3, to make a i-River-mp3-Player mounting. It was on lkml: http://lkml.org/lkml/2004/1/6/170
i was suprised, that it was not in the kernel, and no other soulution, too. So i read his patch for getting mouted the player, and rewrote it for linux-2.6.7-rc1-bk7. I attached it. It would be nice, if it would be possible mounting the mp3-player without patching the Kernel, so i would greet it, if this or an other solution woulg com into the Kernel.

greetings

Tobias Lieber

[-- Attachment #2: patch-iriver --]
[-- Type: application/octet-stream, Size: 1465 bytes --]

--- linux-2.6.6/include/scsi/scsi_devinfo.h	2004-05-30 00:12:10.050746696 +0200
+++ linux_patched/include/scsi/scsi_devinfo.h	2004-05-30 00:19:23.898791768 +0200
@@ -24,4 +24,5 @@
 #define BLIST_REPORTLUN2	0x20000	/* try REPORT_LUNS even for SCSI-2 devs
  					   (if HBA supports more than 8 LUNs) */
 #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
+#define BLIST_NORMB             0x80000 /* Known to be not removable*/
 #endif
--- linux-2.6.6/drivers/scsi/scsi_devinfo.c	2004-05-10 04:32:27.000000000 +0200
+++ linux_patched/drivers/scsi/scsi_devinfo.c	2004-05-30 00:14:00.382973648 +0200
@@ -183,6 +183,7 @@
 	{"TOSHIBA", "CDROM", NULL, BLIST_ISROM},
 	{"TOSHIBA", "CD-ROM", NULL, BLIST_ISROM},
 	{"XYRATEX", "RS", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
+        {"iRiver", "iFP Mass Driver", NULL, BLIST_NORMB},
 	{"Zzyzx", "RocketStor 500S", NULL, BLIST_SPARSELUN},
 	{"Zzyzx", "RocketStor 2000", NULL, BLIST_SPARSELUN},
 	{ NULL, NULL, NULL, 0 },
--- linux-2.6.6/drivers/scsi/scsi_scan.c	2004-05-30 00:12:07.911071976 +0200
+++ linux_patched/drivers/scsi/scsi_scan.c	2004-05-30 00:18:30.694879992 +0200
@@ -565,7 +565,7 @@
 	 */ 
 
 	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
-	sdev->removable = (0x80 & inq_result[1]) >> 7;
+	sdev->removable = (((0x80 & inq_result[1]) >> 7) && !(*bflags &BLIST_NORMB));
 	sdev->lockable = sdev->removable;
 	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
 

Re: Patch for iriver-mp3player

[James Bottomley]

On Sat, 2004-05-29 at 18:42, Tobias Lieber wrote:
> i Javier Marcet wrote in January 2004 a patch for linux-2.6.3, to make a i-River-mp3-Player mounting. It was on lkml: http://lkml.org/lkml/2004/1/6/170
> i was suprised, that it was not in the kernel, and no other soulution, too. So i read his patch for getting mouted the player, and rewrote it for linux-2.6.7-rc1-bk7. I attached it. It would be nice, if it would be possible mounting the mp3-player without patching the Kernel, so i would greet it, if this or an other solution woulg com into the Kernel.

Well, but the question is who gets to fix this?

Since this is a USB device returning broken inquiry data which sd then
acts on, I'd rather see USB fix it.

I cc'd the USB people.  Since you already do some scrubbing on the
inquiry data from this thing (to fix up the ANSI SCSI type), can't you
just add it as an unusual dev that needs the RMB bit resetting (and, if
that's so far wrong, probably other vital pieces of inquiry data as
well)?

James

Re: Patch for iriver-mp3player

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Sun, Jun 06, 2004 at 09:48:51AM -0500, James Bottomley wrote:
> On Sat, 2004-05-29 at 18:42, Tobias Lieber wrote:
> > i Javier Marcet wrote in January 2004 a patch for linux-2.6.3, to make a i-River-mp3-Player mounting. It was on lkml: http://lkml.org/lkml/2004/1/6/170
> > i was suprised, that it was not in the kernel, and no other soulution, too. So i read his patch for getting mouted the player, and rewrote it for linux-2.6.7-rc1-bk7. I attached it. It would be nice, if it would be possible mounting the mp3-player without patching the Kernel, so i would greet it, if this or an other solution woulg com into the Kernel.
> 
> Well, but the question is who gets to fix this?
> 
> Since this is a USB device returning broken inquiry data which sd then
> acts on, I'd rather see USB fix it.
> 
> I cc'd the USB people.  Since you already do some scrubbing on the
> inquiry data from this thing (to fix up the ANSI SCSI type), can't you
> just add it as an unusual dev that needs the RMB bit resetting (and, if
> that's so far wrong, probably other vital pieces of inquiry data as
> well)?

We're actually trying to figure out the 'right way' to do this now.  Our
expectation is that there is some algorithmic way to determine if this bit
is "right" or not, as Windows seems to get it right without
white/blacklisting.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

We've made a business out of making money from annoying and stupid people.
I think you fall under that category.
					-- Chief
User Friendly, 7/11/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Patch for iriver-mp3player

[Alan Stern]

On Sun, 6 Jun 2004, Matthew Dharm wrote:

> We're actually trying to figure out the 'right way' to do this now.  Our
> expectation is that there is some algorithmic way to determine if this bit
> is "right" or not, as Windows seems to get it right without
> white/blacklisting.

The two problems are similar but not quite the same.  The Panasonic
DMC-LC40 camera reports correctly that it has removable media but then
gets confused when it receives ALLOW MEDIUM REMOVAL.  The iRiver flash MP3
player reports _incorrectly_ that it has removable media (that's what
Javier says) and then gets confused when it receives PREVENT-ALLOW MEDIUM 
REMOVAL.

It seems clear that the correct solution for the Panasonic is to clear the 
sdev->lockable flag.  Is that also the correct solution for the iRiver, or 
should we change the RMB flag in the INQUIRY response?  Clearing the 
lockable flag is a lot easier and less invasive.

Alan Stern

Re: iRiver H100 series and usb-storage issues

[Alan Stern]

I just noticed this thread on linux-kernel:

http://marc.theaimsgroup.com/?l=linux-kernel&m=108372064000188&w=2

Apparently under some circumstances the iRiver H100 returns 0 bytes 
transferred with GOOD status and residue=512 for a READ(10) command 
asking for 1 sector.  As far as I know this is legal, and it indicates the 
command should simply be retried.

But apparently the sd driver doesn't realize what has happened and assumes 
the transfer was successful.  If I'm right then the sd driver should be 
fixed, but I'm not sure what is the best thing to do.

James, can you offer some advice?

Alan Stern

Re: iRiver H100 series and usb-storage issues

[Philippe Troin]

Alan Stern <stern@rowland.harvard.edu> writes:

> I just noticed this thread on linux-kernel:
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=108372064000188&w=2
> 
> Apparently under some circumstances the iRiver H100 returns 0 bytes 
> transferred with GOOD status and residue=512 for a READ(10) command 
> asking for 1 sector.  As far as I know this is legal, and it indicates the 
> command should simply be retried.

That's correct.  However, the iRiver H1xx will also commit a USB
protocol violation by not returning a CSW (Command Status
Wrapper).  That's what screws up the transfer.
 
> But apparently the sd driver doesn't realize what has happened and assumes 
> the transfer was successful.  If I'm right then the sd driver should be 
> fixed, but I'm not sure what is the best thing to do.

I am not sure the SCSI layer can recover given that the USB layer is
confused...

Please note that:

 1. I am just repeating what I have been told by various people and
    claim no knowledge of the USB or SCSI layers.  In other words, I
    could be all wrong :-)

 2. I do not own a H1xx anymore, having returned mine.
 
Phil.

Re: iRiver H100 series and usb-storage issues

[Alan Stern]

On 7 Jun 2004, Philippe Troin wrote:

> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> > I just noticed this thread on linux-kernel:
> > 
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=108372064000188&w=2
> > 
> > Apparently under some circumstances the iRiver H100 returns 0 bytes 
> > transferred with GOOD status and residue=512 for a READ(10) command 
> > asking for 1 sector.  As far as I know this is legal, and it indicates the 
> > command should simply be retried.
> 
> That's correct.  However, the iRiver H1xx will also commit a USB
> protocol violation by not returning a CSW (Command Status
> Wrapper).  That's what screws up the transfer.

In the log you posted (at the URL above) the CSW was returned correctly.  
The same is true for the messages you referred to.  So that was not the 
source of the problem.

> Please note that:
> 
>  1. I am just repeating what I have been told by various people and
>     claim no knowledge of the USB or SCSI layers.  In other words, I
>     could be all wrong :-)
> 
>  2. I do not own a H1xx anymore, having returned mine.

Do you know of anyone who has contributed to any versions of this thread 
and still does have an H1xx?

Alan Stern

Re: iRiver H100 series and usb-storage issues

[Philippe Troin]

Alan Stern <stern@rowland.harvard.edu> writes:

> On 7 Jun 2004, Philippe Troin wrote:
> 
> > Alan Stern <stern@rowland.harvard.edu> writes:
> > 
> > > I just noticed this thread on linux-kernel:
> > > 
> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=108372064000188&w=2
> > > 
> > > Apparently under some circumstances the iRiver H100 returns 0 bytes 
> > > transferred with GOOD status and residue=512 for a READ(10) command 
> > > asking for 1 sector.  As far as I know this is legal, and it indicates the 
> > > command should simply be retried.
> > 
> > That's correct.  However, the iRiver H1xx will also commit a USB
> > protocol violation by not returning a CSW (Command Status
> > Wrapper).  That's what screws up the transfer.
> 
> In the log you posted (at the URL above) the CSW was returned correctly.  
> The same is true for the messages you referred to.  So that was not the 
> source of the problem.

Okay, quite possibly, as I was pointing out I am mostly clueless
w.r.t. USB.  But what about:

  usb-storage: usb_stor_transfer_partial(): xfer 512 bytes
  usb-storage: usb_stor_bulk_msg() returned 0 xferred 0/512
  usb-storage: Bulk data transfer result 0x1
  usb-storage: Attempting to get CSW...
               ^^^^^^^^^^^^^^^^^^^^^^^^
  usb-storage: clearing endpoint halt for pipe 0xc0011e80
  usb-storage: usb_stor_clear_halt: result=0
  usb-storage: Attempting to get CSW (2nd try)...
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  usb-storage: Bulk status result = 0
  usb-storage: Bulk status Sig 0x53425355 T 0x945 R 512 Stat 0x0
  usb-storage: -- unexpectedly short transfer
  usb-storage: Issuing auto-REQUEST_SENSE

> > Please note that:
> > 
> >  1. I am just repeating what I have been told by various people and
> >     claim no knowledge of the USB or SCSI layers.  In other words, I
> >     could be all wrong :-)
> > 
> >  2. I do not own a H1xx anymore, having returned mine.
> 
> Do you know of anyone who has contributed to any versions of this thread 
> and still does have an H1xx?

No, I did not keep track.  Nothing prevents me from "trying" another
one if you're serious about fixing the above issues.

Phil.

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[Pat LaVarre]

Philippe T:

> I was pointing out I am mostly clueless
> w.r.t. USB.  ... what about:
> 
>   usb-storage: usb_stor_transfer_partial(): xfer 512 bytes
>   usb-storage: usb_stor_bulk_msg() returned 0 xferred 0/512
>   usb-storage: Bulk data transfer result 0x1
>   usb-storage: Attempting to get CSW...
>                ^^^^^^^^^^^^^^^^^^^^^^^^
>   usb-storage: clearing endpoint halt for pipe 0xc0011e80
>   usb-storage: usb_stor_clear_halt: result=0
>   usb-storage: Attempting to get CSW (2nd try)...
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   usb-storage: Bulk status result = 0
>   usb-storage: Bulk status Sig 0x53425355 T 0x945 R 512 Stat 0x0
>   usb-storage: -- unexpectedly short transfer
>   usb-storage: Issuing auto-REQUEST_SENSE

Knowing only SCSI over USB, not Linux or USB, I'd guess ...

This is a completely ordinary trace of two popular design choices made
by this SCSI over USB device.  First, this device chose to copy less
than the expected count of bytes in.  Second, this device chose to
STALL, rather than copying the bytes in and trusting the host to notice
nonzero dCSWDataResidue.

What's ironic here is that the Linux USB layer understands entirely that
the STALL redundantly with the "CSW" "R 512" means "unexpectedly short
transfer" ... but we're told the layers above don't get it, all the
same.

The op x03 "REQUEST SENSE" here is inappropriate - it is unsolicited -
the "CSW" "Stat" was not x01 AutoSenseMe, it was x00 Passed, expressed
here as "Stat 0x0".

I'm thinking usb-storage has to remove that inappropriate "REQUEST
SENSE", else usb-storage cannot be passing correct status back up?

usb-storage cannot correctly say that it autosensed SK ASC ASCQ zero,
because usb-storage only fetched unsolicited sense, not auto sense. 
usb-storage cannot say it saw a failure without sense, because
usb-storage did think it auto sensed.

Yes?

But after all that, maybe sd still will ignore the error and miscompare?

Pat LaVarre

Re: iRiver H100 series and usb-storage issues

[Alan Stern]

On 7 Jun 2004, Philippe Troin wrote:

> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> > In the log you posted (at the URL above) the CSW was returned correctly.  
> > The same is true for the messages you referred to.  So that was not the 
> > source of the problem.
> 
> Okay, quite possibly, as I was pointing out I am mostly clueless
> w.r.t. USB.  But what about:
> 
>   usb-storage: usb_stor_transfer_partial(): xfer 512 bytes
>   usb-storage: usb_stor_bulk_msg() returned 0 xferred 0/512
>   usb-storage: Bulk data transfer result 0x1
>   usb-storage: Attempting to get CSW...
>                ^^^^^^^^^^^^^^^^^^^^^^^^
>   usb-storage: clearing endpoint halt for pipe 0xc0011e80
>   usb-storage: usb_stor_clear_halt: result=0
>   usb-storage: Attempting to get CSW (2nd try)...
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   usb-storage: Bulk status result = 0
>   usb-storage: Bulk status Sig 0x53425355 T 0x945 R 512 Stat 0x0
>   usb-storage: -- unexpectedly short transfer
>   usb-storage: Issuing auto-REQUEST_SENSE

Those are perfectly normal.  The spec allows a device to halt the 
bulk-in endpoint the first time the host tries to read the CSW.  The 
second attempted read is supposed to succeed, and indeed it did.

> > Do you know of anyone who has contributed to any versions of this thread 
> > and still does have an H1xx?
> 
> No, I did not keep track.  Nothing prevents me from "trying" another
> one if you're serious about fixing the above issues.

I am serious about fixing this.  At the moment we don't have anything new
for you to try, but maybe we'll come up with something soon.  I'm still
waiting to hear from the SCSI people about how this should be handled...

Alan Stern

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[Alan Stern]

On 7 Jun 2004, Pat LaVarre wrote:

> What's ironic here is that the Linux USB layer understands entirely that
> the STALL redundantly with the "CSW" "R 512" means "unexpectedly short
> transfer" ... but we're told the layers above don't get it, all the
> same.
> 
> The op x03 "REQUEST SENSE" here is inappropriate - it is unsolicited -
> the "CSW" "Stat" was not x01 AutoSenseMe, it was x00 Passed, expressed
> here as "Stat 0x0".
> 
> I'm thinking usb-storage has to remove that inappropriate "REQUEST
> SENSE", else usb-storage cannot be passing correct status back up?
> 
> usb-storage cannot correctly say that it autosensed SK ASC ASCQ zero,
> because usb-storage only fetched unsolicited sense, not auto sense. 
> usb-storage cannot say it saw a failure without sense, because
> usb-storage did think it auto sensed.
> 
> Yes?

The log you saw was from an older version of the kernel.  The current
version does not autosense in this situation.  Or when it does (data-out
transfer with CB transport) and the result is SK = ASC = ASCQ = 0, then it
clears the sense buffer to pretend that it didn't autosense.

> But after all that, maybe sd still will ignore the error and miscompare?

sd doesn't receive any error indication other than positive residue, and 
it ignores the residue.

Alan Stern

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[James Bottomley]

On Tue, 2004-06-08 at 09:24, Alan Stern wrote:
> sd doesn't receive any error indication other than positive residue, and 
> it ignores the residue.

Erm, but sd doesn't expect a non zero residue (and producing one is
illegal) because it sets underflow to the exact size for all read and
write commands.

Usually HBAs ignore underflow, because it simply works correctly for all
SCSI devices I've ever come across.  However, if you're going to be
getting these problems, you really should be checking the underflow.

The meaning of this field is supposed to be "return an error if you
transfer fewer than underflow bytes".  The error's not specified, but
something like DID_ERROR with SUGGEST_RETRY should be fine.

James

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[Pat LaVarre]

James B:

> underflow ... meaning ... supposed to be
> "return an error if you transfer fewer than underflow bytes".
> The error's not specified,
> but something like DID_ERROR with SUGGEST_RETRY should be fine.

Incisive, thank you.

> ... Usually HBAs ignore underflow,
> because it simply works correctly ...

Is there an easy way to test whether an HBA implements underflow
correctly?

For example, to inject an underflow into ioctl SG_IO, trivially I ask
for a one block read CDB to fill a two block buffer.

Pat LaVarre

P.S. A possibly relevant glance into the source is:

http://lxr.linux.no/search?v=2.6.5&string=underflow
search: SCSI
http://lxr.linux.no/source/drivers/scsi/sd.c?v=2.6.5#L374
...
static int sd_init_command(struct scsi_cmnd * SCpnt)
...
SCpnt->underflow = this_count << 9;
...

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[James Bottomley]

On Tue, 2004-06-08 at 11:10, Pat LaVarre wrote:
> Is there an easy way to test whether an HBA implements underflow
> correctly?

Only by looking at the source.  I think the aic7 drivers do (both old
and current).

> For example, to inject an underflow into ioctl SG_IO, trivially I ask
> for a one block read CDB to fill a two block buffer.

But, as I said, underflow is largely historical.  Modern SCSI drives
just don't produce an unexpected underflow for which they report GOOD
status (well, except for the variable length transfer commands, of
course).  Hence this feature has languished.

James

Re: [usb-storage] Re: iRiver H100 series and usb-storage issues

[Mike Anderson]

James Bottomley [James.Bottomley@steeleye.com] wrote:
> On Tue, 2004-06-08 at 11:10, Pat LaVarre wrote:
> > Is there an easy way to test whether an HBA implements underflow
> > correctly?
> 
> Only by looking at the source.  I think the aic7 drivers do (both old
> and current).

Another example of looking at the source is the qla2xxx driver
(drivers/scsi/qla2xxx/qla_isr.c near line 1088). In this driver it
will return DID_ERROR.


-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: Patch for iriver-mp3player

[Tobias Lieber]

today i compiled todays linux-cvs, an my mp3-player worked, without patching the kernel....thanks to you....good work


Tobias Lieber

On Mon, 7 Jun 2004 10:36:22 -0400 (EDT)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Sun, 6 Jun 2004, Matthew Dharm wrote:
> 
> > We're actually trying to figure out the 'right way' to do this now.  Our
> > expectation is that there is some algorithmic way to determine if this bit
> > is "right" or not, as Windows seems to get it right without
> > white/blacklisting.
> 
> The two problems are similar but not quite the same.  The Panasonic
> DMC-LC40 camera reports correctly that it has removable media but then
> gets confused when it receives ALLOW MEDIUM REMOVAL.  The iRiver flash MP3
> player reports _incorrectly_ that it has removable media (that's what
> Javier says) and then gets confused when it receives PREVENT-ALLOW MEDIUM 
> REMOVAL.
> 
> It seems clear that the correct solution for the Panasonic is to clear the 
> sdev->lockable flag.  Is that also the correct solution for the iRiver, or 
> should we change the RMB flag in the INQUIRY response?  Clearing the 
> lockable flag is a lot easier and less invasive.
> 
> Alan Stern
>