* Hack to fix not working spindown over Firewire @ 2008-04-29 23:26 Tino Keitel 2008-04-30 13:31 ` Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Tino Keitel @ 2008-04-29 23:26 UTC (permalink / raw) To: linux-kernel, linux-scsi [-- Attachment #1: Type: text/plain, Size: 683 bytes --] Hi folks, I like the manage_start_stop feature of the SD driver to spin down the hard disks (SATA and USB) during suspend. However, it didn't work with my Firewire hard disk. After some research I found out that I can spin down the disk with sg_start --pc=2, and spin it up with sg_start --pc=1. I adopted this into sd_start_stop_device() with the vendor name of the device hardcoded (see the attached patch) and now my Firewire hard disk spins down on suspend and spins up on resume. Is there any chance to get this behaviour without such ugly changes to the kernel? I had to make it conditional by checking the device as otherwise the SATA disk reports an error. Regards, Tino [-- Attachment #2: start_stop_unit_set_power_condition_v2.diff --] [-- Type: text/x-diff, Size: 592 bytes --] diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 3cea17d..4d7dbd6 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1788,8 +1788,16 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) struct scsi_device *sdp = sdkp->device; int res; - if (start) + if (start) { cmd[4] |= 1; /* START */ + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { + cmd[4] |= (1 << 4); /* power condition */ + } + } else { + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { + cmd[4] |= (2 << 4); /* power condition */ + } + } if (!scsi_device_online(sdp)) return -ENODEV; ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-04-29 23:26 Hack to fix not working spindown over Firewire Tino Keitel @ 2008-04-30 13:31 ` Stefan Richter 2008-04-30 13:36 ` Stefan Richter 2008-05-07 18:00 ` Hack " Tino Keitel 0 siblings, 2 replies; 21+ messages in thread From: Stefan Richter @ 2008-04-30 13:31 UTC (permalink / raw) To: linux-kernel, linux-scsi Tino Keitel wrote: > I like the manage_start_stop feature of the SD driver to spin down the > hard disks (SATA and USB) during suspend. However, it didn't work with > my Firewire hard disk. > > After some research I found out that I can spin down the disk with > sg_start --pc=2, and spin it up with sg_start --pc=1. I adopted this > into sd_start_stop_device() with the vendor name of the device > hardcoded (see the attached patch) and now my Firewire hard disk spins > down on suspend and spins up on resume. > > Is there any chance to get this behaviour without such ugly changes to > the kernel? I had to make it conditional by checking the device as > otherwise the SATA disk reports an error. [...] > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1788,8 +1788,16 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) > struct scsi_device *sdp = sdkp->device; > int res; > > - if (start) > + if (start) { > cmd[4] |= 1; /* START */ > + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { > + cmd[4] |= (1 << 4); /* power condition */ > + } > + } else { > + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { > + cmd[4] |= (2 << 4); /* power condition */ > + } > + } > > if (!scsi_device_online(sdp)) > return -ENODEV; For LKML: There was previous discussion on linux1394-user and LSML. http://marc.info/?t=120720546800001 The enclosure is a Raidsonic ICY Box. Responsible for the spindown behavior is the IDE-to-FireWire bridge controller which is a Prolific PL-3507, and its firmware which is different from the USB firmware. Also, there are different firmwares in the wild for PL-3507. The sdp->vendor string is assembled by the firmware from data gathered from the IDE device. It is therefore not suitable to detect affected devices. The traditional way to handle things like this is: - Implement this as a workaround in sd. - Make this workaround conditional on a quirks flag in struct scsi_device. - Switch this flag on in a place where it can be reliably detected that this device needs it. There is an interface to do this in userspace. But in this particular case, the best place is the sbp2 driver (ond firewire-sbp2 driver), because it can detect PL-3507 firmwares by means of firmware identifiers. Tino, to find the firmware marker you can for example: # echo 0x1000 > /sys/module/sbp2/parameters/workarounds Then plug in the disk and # dmesg | grep sbp2 On the other hand, perhaps we can enable the workaround unconditional for all firewire disks (switch in the flag in sbp2 and firewire-sbp2 without looking at the device) or for all RBC disks (switch on the flag in sd.c if sdev->type == TYPE_RBC). RBC clause 5.4.1 states that support of the following POWER CONDITION values in the START STOP UNIT cdb is mandatory: 0 = No change in power condition 1 = Place device in Active condition 2 = Place device in Idle condition 3 = Place device in Standby condition 5 = Place device in Sleep condition There is also optional support for 7 = Device Control, which is uninteresting for our purposes. According to the description of Active, Idle, Standby, Sleep in RBC, we do most certainly want code 1 on resume, and rather 3 than 2 on suspend. 5 instead of 3 or 2 might even be better from the POV of energy consumption but a "device reset may be required before access to the device is allowed" which I'd rather not deal with. Tino, does the scsi stack log the device as "Direct-Access-RBC" after it was plugged in? Note to self: Test sg_start --pc={1,3} with all the various SBP-2 bridges I have access to... -- Stefan Richter -=====-==--- -=-- ====- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-04-30 13:31 ` Stefan Richter @ 2008-04-30 13:36 ` Stefan Richter 2008-05-09 20:16 ` Clean patch " Tino Keitel 2008-05-07 18:00 ` Hack " Tino Keitel 1 sibling, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-04-30 13:36 UTC (permalink / raw) To: linux-kernel, linux-scsi I wrote: > Tino Keitel wrote: >> @@ -1788,8 +1788,16 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) >> struct scsi_device *sdp = sdkp->device; >> int res; >> >> - if (start) >> + if (start) { >> cmd[4] |= 1; /* START */ >> + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { >> + cmd[4] |= (1 << 4); /* power condition */ >> + } >> + } else { >> + if(!strncmp(sdp->vendor, "WDC WD32", 8)) { >> + cmd[4] |= (2 << 4); /* power condition */ >> + } >> + } >> >> if (!scsi_device_online(sdp)) >> return -ENODEV; > [...] > The traditional way to handle things like this is: > > - Implement this as a workaround in sd. > > - Make this workaround conditional on a quirks flag in > struct scsi_device. > > - Switch this flag on in a place where it can be reliably > detected that this device needs it. There is an interface > to do this in userspace. But in this particular case, the > best place is the sbp2 driver (ond firewire-sbp2 driver), > because it can detect PL-3507 firmwares by means of firmware > identifiers. [...] > On the other hand, perhaps we can enable the workaround unconditional > for all firewire disks (switch in the flag in sbp2 and firewire-sbp2 > without looking at the device) or for all RBC disks (switch on the flag > in sd.c if sdev->type == TYPE_RBC). PS: If it works for all RBC devices (it should according to the spec...) && your ICY Box exposes itself as RBC device (it should do so), then we don't need any changes to struct scsi_device and to sbp2/ firewire-sbp2. -- Stefan Richter -=====-==--- -=-- ====- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Clean patch to fix not working spindown over Firewire 2008-04-30 13:36 ` Stefan Richter @ 2008-05-09 20:16 ` Tino Keitel 2008-05-09 21:13 ` Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Tino Keitel @ 2008-05-09 20:16 UTC (permalink / raw) To: Stefan Richter; +Cc: linux-kernel, linux-scsi [-- Attachment #1: Type: text/plain, Size: 373 bytes --] On Wed, Apr 30, 2008 at 15:36:36 +0200, Stefan Richter wrote: [...] > PS: > If it works for all RBC devices (it should according to the spec...) && > your ICY Box exposes itself as RBC device (it should do so), then we > don't need any changes to struct scsi_device and to sbp2/ firewire-sbp2. Do you mean something like in the attached (untested) patch? Regards, Tino [-- Attachment #2: start_stop_unit_set_power_condition_v3.diff --] [-- Type: text/x-diff, Size: 566 bytes --] diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 01cefbb..6b927f6 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1787,8 +1787,13 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) struct scsi_device *sdp = sdkp->device; int res; - if (start) + if (start) { cmd[4] |= 1; /* START */ + /* active power condition */ + cmd[4] |= (sdp->type == TYPE_RBC) ? (1 << 4) : 0; + } else + /* standby power condition */ + cmd[4] |= (sdp->type == TYPE_RBC) ? (3 << 4) : 0 ; if (!scsi_device_online(sdp)) return -ENODEV; ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: Clean patch to fix not working spindown over Firewire 2008-05-09 20:16 ` Clean patch " Tino Keitel @ 2008-05-09 21:13 ` Stefan Richter 2008-05-09 22:02 ` Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-09 21:13 UTC (permalink / raw) To: Tino Keitel; +Cc: linux-kernel, linux-scsi On 9 May, Tino Keitel wrote: > On Wed, Apr 30, 2008 at 15:36:36 +0200, Stefan Richter wrote: >> If it works for all RBC devices (it should according to the spec...) && >> your ICY Box exposes itself as RBC device (it should do so), then we >> don't need any changes to struct scsi_device and to sbp2/ firewire-sbp2. > > Do you mean something like in the attached (untested) patch? > > Regards, > Tino > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 01cefbb..6b927f6 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1787,8 +1787,13 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) > struct scsi_device *sdp = sdkp->device; > int res; > > - if (start) > + if (start) { > cmd[4] |= 1; /* START */ > + /* active power condition */ > + cmd[4] |= (sdp->type == TYPE_RBC) ? (1 << 4) : 0; > + } else > + /* standby power condition */ > + cmd[4] |= (sdp->type == TYPE_RBC) ? (3 << 4) : 0 ; > > if (!scsi_device_online(sdp)) > return -ENODEV; Yes, that's what I meant. Would you test it on your disks and, if it works, repost to LSML with your Signed-off-by line? (See linux/Documentation/SubmittingPatches.) Or write it as if (start) cmd[4] |= 1; /* START */ + + if (sdp->type == TYPE_RBC) + cmd[4] |= start ? 1 << 4 : 3 << 4; /* POWER CONDITION */ Alas the test for TYPE_RBC is not ideal. As I wrote in my other mail, there are some SBP-2 disks which pose as TYPE_DISK. It's not a big problem though because almost all of them also work with POWER CONDITION = 0. The only exception is DViCO Momobay CX-1 which goes belly-up POWER CONDITION = 0 and START = 0. Maybe I should simply overwrite the CX-1's sdp->type by TYPE_RBC. I suspect that this is what it implements anyway. The firmwares of the later Momobay models FX-3A and IIRC CX-2 do so. -- Stefan Richter -=====-==--- -=-= -=--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Clean patch to fix not working spindown over Firewire 2008-05-09 21:13 ` Stefan Richter @ 2008-05-09 22:02 ` Stefan Richter 0 siblings, 0 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-09 22:02 UTC (permalink / raw) To: Tino Keitel; +Cc: linux-kernel, linux-scsi > On 9 May, Tino Keitel wrote: >> On Wed, Apr 30, 2008 at 15:36:36 +0200, Stefan Richter wrote: >>> If it works for all RBC devices (it should according to the spec...) && >>> your ICY Box exposes itself as RBC device (it should do so), then we >>> don't need any changes to struct scsi_device and to sbp2/ firewire-sbp2. >> >> Do you mean something like in the attached (untested) patch? >> >> Regards, >> Tino >> >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 01cefbb..6b927f6 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -1787,8 +1787,13 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) >> struct scsi_device *sdp = sdkp->device; >> int res; >> >> - if (start) >> + if (start) { >> cmd[4] |= 1; /* START */ >> + /* active power condition */ >> + cmd[4] |= (sdp->type == TYPE_RBC) ? (1 << 4) : 0; >> + } else >> + /* standby power condition */ >> + cmd[4] |= (sdp->type == TYPE_RBC) ? (3 << 4) : 0 ; >> PS: The power conditions should be supported by many more devices besides RBC devices. Only SAT (and hence libata) and Linux' "File-backed USB Storage Gadget" (drivers/usb/gadget/file_storage.c) have objections against power condition. But it's less intrusive if we enable it only for RBC for now because we now have a good idea about how well it is supported by them. -- Stefan Richter -=====-==--- -=-= -=--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-04-30 13:31 ` Stefan Richter 2008-04-30 13:36 ` Stefan Richter @ 2008-05-07 18:00 ` Tino Keitel 2008-05-09 18:32 ` Stefan Richter 1 sibling, 1 reply; 21+ messages in thread From: Tino Keitel @ 2008-05-07 18:00 UTC (permalink / raw) To: linux-kernel, linux-scsi On Wed, Apr 30, 2008 at 15:31:41 +0200, Stefan Richter wrote: [...] > Tino, to find the firmware marker you can for example: > # echo 0x1000 > /sys/module/sbp2/parameters/workarounds > Then plug in the disk and > # dmesg | grep sbp2 I used firewire_sbp2. I hope that's ok. firewire_sbp2: Please notify linux1394-devel@lists.sourceforge.net if you need the workarounds parameter for fw1.0 firewire_sbp2: Workarounds for fw1.0: 0x100 (firmware_revision 0x012804, model_id 0x000001) firewire_sbp2: fw1.0: logged in to LUN 0000 (0 retries) [...] > uninteresting for our purposes. According to the description of Active, > Idle, Standby, Sleep in RBC, we do most certainly want code 1 on resume, > and rather 3 than 2 on suspend. 5 instead of 3 or 2 might even be better FYI, sg_start --pc=3 also spins down the drive here. > from the POV of energy consumption but a "device reset may be required > before access to the device is allowed" which I'd rather not deal with. > > Tino, does the scsi stack log the device as "Direct-Access-RBC" after it > was plugged in? Yes: scsi 7:0:0:0: Direct-Access-RBC WDC WD32 00JB-00KFA0 PQ: 0 ANSI: 4 > > Note to self: Test sg_start --pc={1,3} with all the various SBP-2 > bridges I have access to... Thanks for your effort. Regards, Tino ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-05-07 18:00 ` Hack " Tino Keitel @ 2008-05-09 18:32 ` Stefan Richter 2008-05-10 15:31 ` Stefan Richter 2008-05-10 22:53 ` Hack to fix not working spindown over Firewire Stefan Richter 0 siblings, 2 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-09 18:32 UTC (permalink / raw) To: linux1394-devel; +Cc: linux-kernel, linux-scsi, Tino Keitel On 7 May, Tino Keitel wrote at LKML and LSML: [PL-3507 in Raidsonic Icy Box needs power condition 2 or 3 in the START STOP UNIT command in order to spin down] > On Wed, Apr 30, 2008 at 15:31:41 +0200, Stefan Richter wrote: >> According to the description of Active, >> Idle, Standby, Sleep in RBC, we do most certainly want code 1 on resume, >> and rather 3 than 2 on suspend. 5 instead of 3 or 2 might even be better > > FYI, sg_start --pc=3 also spins down the drive here. > >> from the POV of energy consumption but a "device reset may be required >> before access to the device is allowed" which I'd rather not deal with. >> >> Tino, does the scsi stack log the device as "Direct-Access-RBC" after it >> was plugged in? > > Yes: > > scsi 7:0:0:0: Direct-Access-RBC WDC WD32 00JB-00KFA0 PQ: 0 ANSI: 4 I tested a few FireWire disks now. 1) FireWire to IDE or to SATA chipset 2) start bit is honored: "sg_start --stop" stops the motor and "sg_start" restarts it 3) power condition field is honored: "sg_start --pc=3" stops the motor and "sg_start --pc=1 restarts it 4) peripheral device type claimed by the firmware 5) vendor and model of the enclosure 1) 2) 3) 4) 5) bridge start pc PDT enclosure -------------------------------------------------------------- INIC-2430 yes yes SBC AVLAB 2.5" Drive Kit INIC-2430 yes yes SBC IOI FWB-IDE01AB, 1 HDD INIC-2430 yes yes SBC IOI FWB-IDE01AB, 2 HDDs, JBOD OXFW911 yes yes RBC MacPower Icecube OXUF922 yes yes RBC MacPower Icecube 800+ OXFW912 yes yes RBC MacPower Igloo OXFW912 yes yes RBC VulcanTech DualDrive 2nd gen. OXUF924DSB yes yes SBC MacPower Taurus, 1 HDD OXUF924DSB no no SBC MacPower Taurus, 2 HDDs, JBOD PL-3507 no no RBC MacPower Prefect II TSB42AA9 no yes SBC DViCO MomoBay CX-1 TSB42AA9A yes yes RBC DViCO MomoBay FX-3A General remarks: Several disks don't require an "sg_start" or "sg_start --pc=1" to spin up again; they do so on next access. Some even ignore "sg_start --pc=1" and do not start the motor until the next access (e.g. read access). I did not test systematically whether "sg_start" or "sg_start --pc=1" are required at all. (Proper support of "sg_start --pc=1" is beneficial in so far as the high-latency operation which spin-up is happens when it is expected, not sometime later.) The firmwares of CX-1, FX-3A, AVLAB, and Prefect II implement auto-spin-down after some time of inactivity (different from firmware to firmware) and auto-spin-up on subsequent access. Remarks on individual enclosures/ firmwares: MacPower Taurus --------------- I could not test RAID0 and RAID1 due to lack of spare disks. MacPower Prefect II ------------------- This is a 5.25" enclosure, i.e. primarily meant for CD/DVD-ROM/R/W. I tested it with a HDD here. The firmware is from MacPower, not from Prolific. Power conditions 2 or 5 don't spin the disk down either. MomoBay CX-1 ------------ "sg_start --stop" stops the motor, but "sg_start" fails with unit attention. Subsequent read commands after "sg_start --stop" time out. I.e. "sg_start --stop" causes CX-1 to go offline. It needs to be power-cycled after that. MomoBay FX-3A ------------- "sg_start --stop" works. "sg_start" ends with unit attention, but starts the motor. Subsequent read commands succeed. Conclusions so far: - Some PL-3507 based devices, notably those with a later firmware from Prolific, spin down if power condition 2 or 3 is set. Others don't. - The old and in several ways quirky DViCO CX-1 must not be exposed to START STOP UNIT with start flag off. - There is alas no correlation between the PDT and support for either variant of spin down/ spin up. I hope to test another PL-3507 device and an Apple Mac in target mode soon. I will then post a patch for the sbp2/scsi/sd stack based on Tino's patch and the current findings which switches all SBP-2 HDDs to using the power condition field in sd_start_stop_device(). I also have two devices which probably contain old SYM13FWxxx bridges. Alas I can't test them with HDDs because they are sealed CD-RW enclosures which cannot be opened without damage. -- Stefan Richter -=====-==--- -=-= -=--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-05-09 18:32 ` Stefan Richter @ 2008-05-10 15:31 ` Stefan Richter 2008-05-10 22:32 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Stefan Richter 2008-05-10 22:53 ` Hack to fix not working spindown over Firewire Stefan Richter 1 sibling, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-10 15:31 UTC (permalink / raw) To: linux1394-devel; +Cc: Tino Keitel, linux-kernel, linux-scsi I wrote: > 1) FireWire to IDE or to SATA chipset > 2) start bit is honored: > "sg_start --stop" stops the motor and "sg_start" restarts it > 3) power condition field is honored: > "sg_start --pc=3" stops the motor and "sg_start --pc=1 restarts it > 4) peripheral device type claimed by the firmware > 5) vendor and model of the enclosure > > > 1) 2) 3) 4) 5) > bridge start pc PDT enclosure > -------------------------------------------------------------- > INIC-2430 yes yes SBC AVLAB 2.5" Drive Kit > INIC-2430 yes yes SBC IOI FWB-IDE01AB, 1 HDD > INIC-2430 yes yes SBC IOI FWB-IDE01AB, 2 HDDs, JBOD > > OXFW911 yes yes RBC MacPower Icecube > OXUF922 yes yes RBC MacPower Icecube 800+ > OXFW912 yes yes RBC MacPower Igloo > OXFW912 yes yes RBC VulcanTech DualDrive 2nd gen. > OXUF924DSB yes yes SBC MacPower Taurus, 1 HDD > OXUF924DSB no no SBC MacPower Taurus, 2 HDDs, JBOD > > PL-3507 no no RBC MacPower Prefect II ¹ PL-3507 no yes RBC Mapower MAP-KC21C1 ² > TSB42AA9 no yes SBC DViCO MomoBay CX-1 > TSB42AA9A yes yes RBC DViCO MomoBay FX-3A EFI firmware no no RBC Apple Mac mini x86 in target mode ³ ¹) As mentioned, with firmware from MacPower. ²) Same firmware_revision as with Tino's disk, i.e. apparently firmware from Prolific. Same findings as Tino's: "sg_start --stop" is accepted but does not stop the motor. "sg_start --pc=3" or "=2" stop the motor. "sg_start --pc=1" or a read command restarts the motor. "sg_start" does not restart the motor. ³) "sg_start --stop" is accepted but does not stop the motor. "sg_start --pc=3" or "=1" fails with "invalid field in cdb". This is of course against the spec, because RBC includes power condition. Moreover, when OS X shuts down, it apparently sends START STOP UNIT without power condition to FireWire disks. Therefore we also should rather _not_ use power condition per default. I will send a patch which uses the power condition field only for PL-3507 and DViCO CX-1. -- Stefan Richter -=====-==--- -=-= -=-=- http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-10 15:31 ` Stefan Richter @ 2008-05-10 22:32 ` Stefan Richter 2008-05-10 22:34 ` [PATCH 1/5] scsi: sd: optionally set power condition in START STOP UNIT Stefan Richter 2008-05-16 6:23 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Tino Keitel 0 siblings, 2 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:32 UTC (permalink / raw) To: linux1394-devel, James Bottomley; +Cc: Tino Keitel, linux-kernel, linux-scsi This series contains: Patch 1/5...3/5: Some SBP-2 disks don't work as expected if the power condition field in the START STOP UNIT command is not set. They either don't spin down on start=0, or they become unresponsive after it. These patches selectively adds power condition values for known affected devices. We can't do so across the board because certain other SBP-2 devices have been found to not support power condition (even though it is mandatory as per RBC). These three patches should IMO go into mainline during the current -rc phase. Patch 4/5...5/5: Enable scsi_device.manage_start_stop for all SBP-2 devices (except if logged in non-exclusively). This lets FireWire disks spin down on suspend, driver unbinding, or system shutdown. These two patches are post 2.6.26 material. James, how do we handle patch 1/5, assumed that this one is OK? Do you route it through scsi-rc-fixes (or scsi-misc if really necessary), or do you make an exception and let it slip through linux1394-2.6.git? The patch applies to plain 2.6.25 as well as to current scsi-misc without conflicts. drivers/firewire/fw-sbp2.c | 25 +++++++++++++++++++++++-- drivers/ieee1394/sbp2.c | 22 ++++++++++++++++++++-- drivers/ieee1394/sbp2.h | 1 + drivers/scsi/sd.c | 5 +++++ include/scsi/scsi_device.h | 1 + 5 files changed, 50 insertions(+), 4 deletions(-) Stefan Richter (5): scsi: sd: optionally set power condition in START STOP UNIT firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares ieee1394: sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares firewire: fw-sbp2: spin disks down on suspend and shutdown ieee1394: sbp2: spin disks down on suspend and shutdown -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] scsi: sd: optionally set power condition in START STOP UNIT 2008-05-10 22:32 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Stefan Richter @ 2008-05-10 22:34 ` Stefan Richter 2008-05-10 22:35 ` [PATCH 2/5] firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares Stefan Richter 2008-05-16 6:23 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Tino Keitel 1 sibling, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:34 UTC (permalink / raw) To: linux1394-devel, James Bottomley; +Cc: Tino Keitel, linux-kernel, linux-scsi Adds a new scsi_device flag, start_stop_pwr_cond: If enabled, the sd driver will not send plain START STOP UNIT commands but ones with the power condition field set to 3 (standby) or 1 (active) respectively. Some FireWire disk firmwares do not stop the motor if power condition is zero. Or worse, the become unresponsive after a START STOP UNIT with power condition = 0 and start = 0. http://lkml.org/lkml/2008/4/29/704 This patch only adds the necessary code to sd_mod but doesn't activate it. Follow-up patches to the FireWire drivers will add detection of affected devices and enable the code for them. I did not add power condition values to scsi_error.c::scsi_eh_try_stu() for now. The three firmwares which suffer from above mentioned problems do not need START STOP UNIT in the error handler, and they are not adversely affected by START STOP UNIT with power condition = 0 and start = 1 (like scsi_eh_try_stu() sends it if scsi_device.allow_restart is enabled). Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> --- drivers/scsi/sd.c | 5 +++++ include/scsi/scsi_device.h | 1 + 2 files changed, 6 insertions(+) Index: linux/drivers/scsi/sd.c =================================================================== --- linux.orig/drivers/scsi/sd.c +++ linux/drivers/scsi/sd.c @@ -1115,6 +1115,8 @@ sd_spinup_disk(struct scsi_disk *sdkp) cmd[1] = 1; /* Return immediately */ memset((void *) &cmd[2], 0, 8); cmd[4] = 1; /* Start spin cycle */ + if (sdkp->device->start_stop_pwr_cond) + cmd[4] |= 1 << 4; scsi_execute_req(sdkp->device, cmd, DMA_NONE, NULL, 0, &sshdr, SD_TIMEOUT, SD_MAX_RETRIES); @@ -1781,6 +1783,9 @@ static int sd_start_stop_device(struct s if (start) cmd[4] |= 1; /* START */ + if (sdp->start_stop_pwr_cond) + cmd[4] |= start ? 1 << 4 : 3 << 4; + if (!scsi_device_online(sdp)) return -ENODEV; Index: linux/include/scsi/scsi_device.h =================================================================== --- linux.orig/include/scsi/scsi_device.h +++ linux/include/scsi/scsi_device.h @@ -134,6 +134,7 @@ struct scsi_device { unsigned no_start_on_add:1; /* do not issue start on add */ unsigned allow_restart:1; /* issue START_UNIT in error handler */ unsigned manage_start_stop:1; /* Let HLD (sd) manage start/stop */ + unsigned start_stop_pwr_cond:1; /* Set power cond. in START_STOP_UNIT */ unsigned no_uld_attach:1; /* disable connecting to upper level drivers */ unsigned select_no_atn:1; unsigned fix_capacity:1; /* READ_CAPACITY is too high by 1 */ -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares 2008-05-10 22:34 ` [PATCH 1/5] scsi: sd: optionally set power condition in START STOP UNIT Stefan Richter @ 2008-05-10 22:35 ` Stefan Richter 2008-05-10 22:35 ` [PATCH 3/5] ieee1394: sbp2: " Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:35 UTC (permalink / raw) To: linux1394-devel, James Bottomley; +Cc: Tino Keitel, linux-kernel, linux-scsi Reported by Tino Keitel: PL-3507 with firmware from Prolific does not spin down the disk on START STOP UNIT with power condition = 0 and start = 0. It does however work with power condition = 2 or 3. Also found while investigating this: DViCO Momobay CX-1 and FX-3A (TI TSB42AA9/A based) become unresponsive after START STOP UNIT with power condition = 0 and start = 0. They stay responsive if power condition is set when stopping the motor. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> --- drivers/firewire/fw-sbp2.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) Index: linux/drivers/firewire/fw-sbp2.c =================================================================== --- linux.orig/drivers/firewire/fw-sbp2.c +++ linux/drivers/firewire/fw-sbp2.c @@ -86,6 +86,11 @@ MODULE_PARM_DESC(exclusive_login, "Exclu * - delay inquiry * Wait extra SBP2_INQUIRY_DELAY seconds after login before SCSI inquiry. * + * - power condition + * Set the power condition field in the START STOP UNIT commands sent by + * sd_mod on suspend, resume, and shutdown (if manage_start_stop is on). + * Some disks need this to spin down or to resume properly. + * * - override internal blacklist * Instead of adding to the built-in blacklist, use only the workarounds * specified in the module load parameter. @@ -97,6 +102,7 @@ MODULE_PARM_DESC(exclusive_login, "Exclu #define SBP2_WORKAROUND_FIX_CAPACITY 0x8 #define SBP2_WORKAROUND_DELAY_INQUIRY 0x10 #define SBP2_INQUIRY_DELAY 12 +#define SBP2_WORKAROUND_POWER_CONDITION 0x20 #define SBP2_WORKAROUND_OVERRIDE 0x100 static int sbp2_param_workarounds; @@ -107,6 +113,8 @@ MODULE_PARM_DESC(workarounds, "Work arou ", skip mode page 8 = " __stringify(SBP2_WORKAROUND_MODE_SENSE_8) ", fix capacity = " __stringify(SBP2_WORKAROUND_FIX_CAPACITY) ", delay inquiry = " __stringify(SBP2_WORKAROUND_DELAY_INQUIRY) + ", set power condition in start stop unit = " + __stringify(SBP2_WORKAROUND_POWER_CONDITION) ", override internal blacklist = " __stringify(SBP2_WORKAROUND_OVERRIDE) ", or a combination)"); @@ -310,18 +318,25 @@ static const struct { .firmware_revision = 0x002800, .model = 0x001010, .workarounds = SBP2_WORKAROUND_INQUIRY_36 | - SBP2_WORKAROUND_MODE_SENSE_8, + SBP2_WORKAROUND_MODE_SENSE_8 | + SBP2_WORKAROUND_POWER_CONDITION, }, /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { .firmware_revision = 0x002800, .model = 0x000000, - .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY, + .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY | + SBP2_WORKAROUND_POWER_CONDITION, }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, .model = ~0, .workarounds = SBP2_WORKAROUND_INQUIRY_36, }, + /* PL-3507 bridge with Prolific firmware */ { + .firmware_revision = 0x012800, + .model = ~0, + .workarounds = SBP2_WORKAROUND_POWER_CONDITION, + }, /* Symbios bridge */ { .firmware_revision = 0xa0b800, .model = ~0, @@ -1539,6 +1554,9 @@ static int sbp2_scsi_slave_configure(str if (lu->tgt->workarounds & SBP2_WORKAROUND_FIX_CAPACITY) sdev->fix_capacity = 1; + if (lu->tgt->workarounds & SBP2_WORKAROUND_POWER_CONDITION) + sdev->start_stop_pwr_cond = 1; + if (lu->tgt->workarounds & SBP2_WORKAROUND_128K_MAX_TRANS) blk_queue_max_sectors(sdev->request_queue, 128 * 1024 / 512); -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] ieee1394: sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares 2008-05-10 22:35 ` [PATCH 2/5] firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares Stefan Richter @ 2008-05-10 22:35 ` Stefan Richter 2008-05-10 22:36 ` [PATCH 4/5] firewire: fw-sbp2: spin disks down on suspend and shutdown Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:35 UTC (permalink / raw) To: linux1394-devel, James Bottomley; +Cc: Tino Keitel, linux-kernel, linux-scsi Reported by Tino Keitel: PL-3507 with firmware from Prolific does not spin down the disk on START STOP UNIT with power condition = 0 and start = 0. It does however work with power condition = 2 or 3. Also found while investigating this: DViCO Momobay CX-1 and FX-3A (TI TSB42AA9/A based) become unresponsive after START STOP UNIT with power condition = 0 and start = 0. They stay responsive if power condition is set when stopping the motor. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> --- drivers/ieee1394/sbp2.c | 20 ++++++++++++++++++-- drivers/ieee1394/sbp2.h | 1 + 2 files changed, 19 insertions(+), 2 deletions(-) Index: linux/drivers/ieee1394/sbp2.c =================================================================== --- linux.orig/drivers/ieee1394/sbp2.c +++ linux/drivers/ieee1394/sbp2.c @@ -186,6 +186,11 @@ MODULE_PARM_DESC(exclusive_login, "Exclu * - delay inquiry * Wait extra SBP2_INQUIRY_DELAY seconds after login before SCSI inquiry. * + * - power condition + * Set the power condition field in the START STOP UNIT commands sent by + * sd_mod on suspend, resume, and shutdown (if manage_start_stop is on). + * Some disks need this to spin down or to resume properly. + * * - override internal blacklist * Instead of adding to the built-in blacklist, use only the workarounds * specified in the module load parameter. @@ -199,6 +204,8 @@ MODULE_PARM_DESC(workarounds, "Work arou ", skip mode page 8 = " __stringify(SBP2_WORKAROUND_MODE_SENSE_8) ", fix capacity = " __stringify(SBP2_WORKAROUND_FIX_CAPACITY) ", delay inquiry = " __stringify(SBP2_WORKAROUND_DELAY_INQUIRY) + ", set power condition in start stop unit = " + __stringify(SBP2_WORKAROUND_POWER_CONDITION) ", override internal blacklist = " __stringify(SBP2_WORKAROUND_OVERRIDE) ", or a combination)"); @@ -359,18 +366,25 @@ static const struct { .firmware_revision = 0x002800, .model_id = 0x001010, .workarounds = SBP2_WORKAROUND_INQUIRY_36 | - SBP2_WORKAROUND_MODE_SENSE_8, + SBP2_WORKAROUND_MODE_SENSE_8 | + SBP2_WORKAROUND_POWER_CONDITION, }, /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { .firmware_revision = 0x002800, .model_id = 0x000000, - .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY, + .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY | + SBP2_WORKAROUND_POWER_CONDITION, }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, .model_id = SBP2_ROM_VALUE_WILDCARD, .workarounds = SBP2_WORKAROUND_INQUIRY_36, }, + /* PL-3507 bridge with Prolific firmware */ { + .firmware_revision = 0x012800, + .model_id = SBP2_ROM_VALUE_WILDCARD, + .workarounds = SBP2_WORKAROUND_POWER_CONDITION, + }, /* Symbios bridge */ { .firmware_revision = 0xa0b800, .model_id = SBP2_ROM_VALUE_WILDCARD, @@ -2002,6 +2016,8 @@ static int sbp2scsi_slave_configure(stru sdev->skip_ms_page_8 = 1; if (lu->workarounds & SBP2_WORKAROUND_FIX_CAPACITY) sdev->fix_capacity = 1; + if (lu->workarounds & SBP2_WORKAROUND_POWER_CONDITION) + sdev->start_stop_pwr_cond = 1; if (lu->workarounds & SBP2_WORKAROUND_128K_MAX_TRANS) blk_queue_max_sectors(sdev->request_queue, 128 * 1024 / 512); return 0; Index: linux/drivers/ieee1394/sbp2.h =================================================================== --- linux.orig/drivers/ieee1394/sbp2.h +++ linux/drivers/ieee1394/sbp2.h @@ -345,6 +345,7 @@ enum sbp2lu_state_types { #define SBP2_WORKAROUND_FIX_CAPACITY 0x8 #define SBP2_WORKAROUND_DELAY_INQUIRY 0x10 #define SBP2_INQUIRY_DELAY 12 +#define SBP2_WORKAROUND_POWER_CONDITION 0x20 #define SBP2_WORKAROUND_OVERRIDE 0x100 #endif /* SBP2_H */ -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] firewire: fw-sbp2: spin disks down on suspend and shutdown 2008-05-10 22:35 ` [PATCH 3/5] ieee1394: sbp2: " Stefan Richter @ 2008-05-10 22:36 ` Stefan Richter 2008-05-10 22:37 ` [PATCH 5/5] ieee1394: sbp2: " Stefan Richter 0 siblings, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:36 UTC (permalink / raw) To: linux1394-devel, James Bottomley; +Cc: Tino Keitel, linux-kernel, linux-scsi This instructs sd_mod to send START STOP UNIT on suspend and resume, and on driver unbinding or unloading (including when the system is shut down). We don't do this though if multiple initiators may log in to the target. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> --- drivers/firewire/fw-sbp2.c | 3 +++ 1 file changed, 3 insertions(+) Index: linux/drivers/firewire/fw-sbp2.c =================================================================== --- linux.orig/drivers/firewire/fw-sbp2.c +++ linux/drivers/firewire/fw-sbp2.c @@ -1544,6 +1544,9 @@ static int sbp2_scsi_slave_configure(str sdev->use_10_for_rw = 1; + if (sbp2_param_exclusive_login) + sdev->manage_start_stop = 1; + if (sdev->type == TYPE_ROM) sdev->use_10_for_ms = 1; -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] ieee1394: sbp2: spin disks down on suspend and shutdown 2008-05-10 22:36 ` [PATCH 4/5] firewire: fw-sbp2: spin disks down on suspend and shutdown Stefan Richter @ 2008-05-10 22:37 ` Stefan Richter 0 siblings, 0 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:37 UTC (permalink / raw) To: linux1394-devel; +Cc: James Bottomley, Tino Keitel, linux-kernel, linux-scsi This instructs sd_mod to send START STOP UNIT on suspend and resume, and on driver unbinding or unloading (including when the system is shut down). We don't do this though if multiple initiators may log in to the target. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> --- drivers/ieee1394/sbp2.c | 2 ++ 1 file changed, 2 insertions(+) Index: linux/drivers/ieee1394/sbp2.c =================================================================== --- linux.orig/drivers/ieee1394/sbp2.c +++ linux/drivers/ieee1394/sbp2.c @@ -2009,6 +2009,8 @@ static int sbp2scsi_slave_configure(stru sdev->use_10_for_rw = 1; + if (sbp2_exclusive_login) + sdev->manage_start_stop = 1; if (sdev->type == TYPE_ROM) sdev->use_10_for_ms = 1; if (sdev->type == TYPE_DISK && -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-10 22:32 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Stefan Richter 2008-05-10 22:34 ` [PATCH 1/5] scsi: sd: optionally set power condition in START STOP UNIT Stefan Richter @ 2008-05-16 6:23 ` Tino Keitel 2008-05-16 17:43 ` Stefan Richter 1 sibling, 1 reply; 21+ messages in thread From: Tino Keitel @ 2008-05-16 6:23 UTC (permalink / raw) To: Stefan Richter; +Cc: James Bottomley, linux1394-devel, linux-kernel, linux-scsi Hi, I applied all patches, and tried it on my system with a SATA, USB and Firewire disk. All disks spun down at suspend, and spun up again at resume. Regards, Tino ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-16 6:23 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Tino Keitel @ 2008-05-16 17:43 ` Stefan Richter 2008-05-18 17:35 ` Tino Keitel 0 siblings, 1 reply; 21+ messages in thread From: Stefan Richter @ 2008-05-16 17:43 UTC (permalink / raw) To: Tino Keitel; +Cc: linux1394-devel, James Bottomley, linux-kernel, linux-scsi Tino Keitel wrote: > I applied all patches, and tried it on my system with a SATA, USB and > Firewire disk. All disks spun down at suspend, and spun up again at > resume. So can I add "Tested-by: Tino Keitel <tino.keitel@gmx.de>" to the changelog of patches 1/5, 2/5, 4/5 (the patches affecting sd and fw-sbp2)? Thanks for testing, and for finding in the first place that we need the power conditions field. -- Stefan Richter -=====-==--- -=-= =---- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-16 17:43 ` Stefan Richter @ 2008-05-18 17:35 ` Tino Keitel 2008-05-18 20:32 ` Stefan Richter 2008-05-19 17:18 ` Stefan Richter 0 siblings, 2 replies; 21+ messages in thread From: Tino Keitel @ 2008-05-18 17:35 UTC (permalink / raw) To: Stefan Richter; +Cc: James Bottomley, linux1394-devel, linux-kernel, linux-scsi On Fri, May 16, 2008 at 19:43:43 +0200, Stefan Richter wrote: > Tino Keitel wrote: > > I applied all patches, and tried it on my system with a SATA, USB and > > Firewire disk. All disks spun down at suspend, and spun up again at > > resume. > > So can I add "Tested-by: Tino Keitel <tino.keitel@gmx.de>" to the > changelog of patches 1/5, 2/5, 4/5 (the patches affecting sd and > fw-sbp2)? Yes. Now I need to figure out how to get the Firewire disk to spin down using HAL. It looks like HAL just uses /usr/bin/eject, which just sends a START STOP UNIT command without power conditions set, so the kernel can't do anything here. Regards, Tino ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-18 17:35 ` Tino Keitel @ 2008-05-18 20:32 ` Stefan Richter 2008-05-19 17:18 ` Stefan Richter 1 sibling, 0 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-18 20:32 UTC (permalink / raw) To: linux-scsi; +Cc: linux1394-devel, James Bottomley, linux-kernel Tino Keitel wrote: > Now I need to figure out how to get the Firewire disk to spin down > using HAL. It looks like HAL just uses /usr/bin/eject, which just sends > a START STOP UNIT command without power conditions set, so the kernel > can't do anything here. What if you teach hal to unbind sd from the respective scsi_device? "echo -n 123:0:0:0 > /sys/bus/scsi/drivers/sd/unbind" is a manual way to do it. And the "bind" file does the reverse. Or replace /usr/bin/eject by a shell script of yours which calls sg_start when you need it to... -- Stefan Richter -=====-==--- -=-= =--=- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks 2008-05-18 17:35 ` Tino Keitel 2008-05-18 20:32 ` Stefan Richter @ 2008-05-19 17:18 ` Stefan Richter 1 sibling, 0 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-19 17:18 UTC (permalink / raw) To: Tino Keitel, James Bottomley; +Cc: linux1394-devel, linux-kernel, linux-scsi >> Tino Keitel wrote: >>> I applied all patches, and tried it on my system with a SATA, USB and >>> Firewire disk. All disks spun down at suspend, and spun up again at >>> resume. OK, I now added your Tested-by. I also added a short comment to the change in sd_start_stop_device(), as quoted below, and enqueued this stuff in git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git sbp2-spindown I also made these changes visible in the branch of linux1394-2.6.git which is pulled into -next. Shortlog and diffstat is Stefan Richter (5): scsi: sd: optionally set power condition in START STOP UNIT firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares ieee1394: sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares firewire: fw-sbp2: spin disks down on suspend and shutdown ieee1394: sbp2: spin disks down on suspend and shutdown drivers/firewire/fw-sbp2.c | 25 +++++++++++++++++++++++-- drivers/ieee1394/sbp2.c | 22 ++++++++++++++++++++-- drivers/ieee1394/sbp2.h | 1 + drivers/scsi/sd.c | 5 +++++ include/scsi/scsi_device.h | 1 + 5 files changed, 50 insertions(+), 4 deletions(-) James, do you agree to the scsi_device.h + sd.c patch? And will you pick it up eventually or should I keep it? I would submit this and the depending 4 FireWire patches after 2.6.26 was released --- although the first 3 patches could also go to Linus before 2.6.26. Here is the updated first patch: Date: Sun, 11 May 2008 00:34:07 +0200 (CEST) From: Stefan Richter <stefanr@s5r6.in-berlin.de> Subject: scsi: sd: optionally set power condition in START STOP UNIT Adds a new scsi_device flag, start_stop_pwr_cond: If enabled, the sd driver will not send plain START STOP UNIT commands but ones with the power condition field set to 3 (standby) or 1 (active) respectively. Some FireWire disk firmwares do not stop the motor if power condition is zero. Or worse, they become unresponsive after a START STOP UNIT with power condition = 0 and start = 0. http://lkml.org/lkml/2008/4/29/704 This patch only adds the necessary code to sd_mod but doesn't activate it. Follow-up patches to the FireWire drivers will add detection of affected devices and enable the code for them. I did not add power condition values to scsi_error.c::scsi_eh_try_stu() for now. The three firmwares which suffer from above mentioned problems do not need START STOP UNIT in the error handler, and they are not adversely affected by START STOP UNIT with power condition = 0 and start = 1 (like scsi_eh_try_stu() sends it if scsi_device.allow_restart is enabled). Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Tested-by: Tino Keitel <tino.keitel@gmx.de> --- drivers/scsi/sd.c | 5 +++++ include/scsi/scsi_device.h | 1 + 2 files changed, 6 insertions(+) Index: linux/drivers/scsi/sd.c =================================================================== --- linux.orig/drivers/scsi/sd.c +++ linux/drivers/scsi/sd.c @@ -1115,6 +1115,8 @@ sd_spinup_disk(struct scsi_disk *sdkp) cmd[1] = 1; /* Return immediately */ memset((void *) &cmd[2], 0, 8); cmd[4] = 1; /* Start spin cycle */ + if (sdkp->device->start_stop_pwr_cond) + cmd[4] |= 1 << 4; scsi_execute_req(sdkp->device, cmd, DMA_NONE, NULL, 0, &sshdr, SD_TIMEOUT, SD_MAX_RETRIES); @@ -1781,6 +1783,9 @@ static int sd_start_stop_device(struct s if (start) cmd[4] |= 1; /* START */ + if (sdp->start_stop_pwr_cond) + cmd[4] |= start ? 1 << 4 : 3 << 4; /* Active or Standby */ + if (!scsi_device_online(sdp)) return -ENODEV; Index: linux/include/scsi/scsi_device.h =================================================================== --- linux.orig/include/scsi/scsi_device.h +++ linux/include/scsi/scsi_device.h @@ -134,6 +134,7 @@ struct scsi_device { unsigned no_start_on_add:1; /* do not issue start on add */ unsigned allow_restart:1; /* issue START_UNIT in error handler */ unsigned manage_start_stop:1; /* Let HLD (sd) manage start/stop */ + unsigned start_stop_pwr_cond:1; /* Set power cond. in START_STOP_UNIT */ unsigned no_uld_attach:1; /* disable connecting to upper level drivers */ unsigned select_no_atn:1; unsigned fix_capacity:1; /* READ_CAPACITY is too high by 1 */ -- Stefan Richter -=====-==--- -=-= =--== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Hack to fix not working spindown over Firewire 2008-05-09 18:32 ` Stefan Richter 2008-05-10 15:31 ` Stefan Richter @ 2008-05-10 22:53 ` Stefan Richter 1 sibling, 0 replies; 21+ messages in thread From: Stefan Richter @ 2008-05-10 22:53 UTC (permalink / raw) To: linux1394-devel; +Cc: Tino Keitel, linux-kernel, linux-scsi I wrote: > 1) FireWire to IDE or to SATA chipset > 2) start bit is honored: > "sg_start --stop" stops the motor and "sg_start" restarts it > 3) power condition field is honored: > "sg_start --pc=3" stops the motor and "sg_start --pc=1 restarts it > 4) peripheral device type claimed by the firmware > 5) vendor and model of the enclosure > > > 1) 2) 3) 4) 5) > bridge start pc PDT enclosure > -------------------------------------------------------------- ... > TSB42AA9 no yes SBC DViCO MomoBay CX-1 > TSB42AA9A yes yes RBC DViCO MomoBay FX-3A Correction: While the FX-3A stops the motor if START STOP UNIT with an equivalent to "sg_start --stop", it does not work correctly anymore after it. Certain commands after that will hang the firmware up; notably those sent when sd_mod is probing the disk. I.e. FX-3A fails not exactly as CX-1, but similarly. -- Stefan Richter -=====-==--- -=-= -=-== http://arcgraph.de/sr/ ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2008-05-19 17:19 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-29 23:26 Hack to fix not working spindown over Firewire Tino Keitel 2008-04-30 13:31 ` Stefan Richter 2008-04-30 13:36 ` Stefan Richter 2008-05-09 20:16 ` Clean patch " Tino Keitel 2008-05-09 21:13 ` Stefan Richter 2008-05-09 22:02 ` Stefan Richter 2008-05-07 18:00 ` Hack " Tino Keitel 2008-05-09 18:32 ` Stefan Richter 2008-05-10 15:31 ` Stefan Richter 2008-05-10 22:32 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Stefan Richter 2008-05-10 22:34 ` [PATCH 1/5] scsi: sd: optionally set power condition in START STOP UNIT Stefan Richter 2008-05-10 22:35 ` [PATCH 2/5] firewire: fw-sbp2: fix spindown for PL-3507 and TSB42AA9 firmwares Stefan Richter 2008-05-10 22:35 ` [PATCH 3/5] ieee1394: sbp2: " Stefan Richter 2008-05-10 22:36 ` [PATCH 4/5] firewire: fw-sbp2: spin disks down on suspend and shutdown Stefan Richter 2008-05-10 22:37 ` [PATCH 5/5] ieee1394: sbp2: " Stefan Richter 2008-05-16 6:23 ` [PATCH 0/5] SCSI and FireWire: fix/add START STOP UNIT for SBP-2 disks Tino Keitel 2008-05-16 17:43 ` Stefan Richter 2008-05-18 17:35 ` Tino Keitel 2008-05-18 20:32 ` Stefan Richter 2008-05-19 17:18 ` Stefan Richter 2008-05-10 22:53 ` Hack to fix not working spindown over Firewire Stefan Richter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox