Elias Oltmanns <[email protected]> wrote:
> Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>> On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
>
>>> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
>>
>>> FEATURE as specified in ATA-7 is issued to the device and processing of
>>> the request queue is stopped thereafter until the specified timeout
>>> expires or user space asks to resume normal operation. This is supposed
>>> to prevent the heads of a hard drive from accidentally crashing onto the
>>> platter when a heavy shock is anticipated (like a falling laptop expected
>>> to hit the floor). Port resets are deferred whenever a device on that
>>> port is in the parked state.
>>>
>>> Signed-off-by: Elias Oltmanns <[email protected]>
>>
>> applied
>
> Hi Bart,
>
> may I ask you to apply yet another inter-diff? This is in order to
> address three issues:
>
> 1. Make sure that no negative value is being passed to
> jiffies_to_msecs() in ide_park_show().
> 2. Drop the superfluous variable hwif in ide_special_rq().
> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
> handling a (un)park request.
Well, #3 should have been done differently because we donn't want to
check for REQ_(UN)?PARK_HEADS more often than is necessary. Here is a
revised version of the inter-diff.
Regards,
Elias
Signed-off-by: Elias Oltmanns <[email protected]>
---
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 09d10a5..77c6eae 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,25 +672,32 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
{
- ide_hwif_t *hwif = drive->hwif;
- ide_task_t task;
- struct ide_taskfile *tf = &task.tf;
+ u8 cmd = rq->cmd[0];
+
+ if (cmd == REQ_PARK_HEADS || cmd == REQ_UNPARK_HEADS) {
+ ide_task_t task;
+ struct ide_taskfile *tf = &task.tf;
+
+ memset(&task, 0, sizeof(task));
+ if (cmd == REQ_PARK_HEADS) {
+ drive->sleep = *(unsigned long *)rq->special;
+ drive->dev_flags |= IDE_DFLAG_SLEEPING;
+ tf->command = ATA_CMD_IDLEIMMEDIATE;
+ tf->feature = 0x44;
+ tf->lbal = 0x4c;
+ tf->lbam = 0x4e;
+ tf->lbah = 0x55;
+ task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
+ } else /* cmd == REQ_UNPARK_HEADS */
+ tf->command = ATA_CMD_CHK_POWER;
+
+ task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
+ task.rq = rq;
+ drive->hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+ return do_rw_taskfile(drive, &task);
+ }
- memset(&task, 0, sizeof(task));
- switch (rq->cmd[0]) {
- case REQ_PARK_HEADS:
- drive->sleep = *(unsigned long *)rq->special;
- drive->dev_flags |= IDE_DFLAG_SLEEPING;
- tf->command = ATA_CMD_IDLEIMMEDIATE;
- tf->feature = 0x44;
- tf->lbal = 0x4c;
- tf->lbam = 0x4e;
- tf->lbah = 0x55;
- task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
- break;
- case REQ_UNPARK_HEADS:
- tf->command = ATA_CMD_CHK_POWER;
- break;
+ switch (cmd) {
case REQ_DEVSET_EXEC:
{
int err, (*setfunc)(ide_drive_t *, int) = rq->special;
@@ -710,10 +717,6 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
ide_end_request(drive, 0, 0);
return ide_stopped;
}
- task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
- task.rq = rq;
- hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
- return do_rw_taskfile(drive, &task);
}
static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 35fc3ee..02d7e35 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -61,15 +61,17 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
ide_drive_t *drive = to_ide_device(dev);
+ unsigned long now;
unsigned int msecs;
if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
return -EOPNOTSUPP;
spin_lock_irq(&ide_lock);
+ now = jiffies;
if (drive->dev_flags & IDE_DFLAG_PARKED &&
- time_after(drive->sleep, jiffies))
- msecs = jiffies_to_msecs(drive->sleep - jiffies);
+ time_after(drive->sleep, now))
+ msecs = jiffies_to_msecs(drive->sleep - now);
else
msecs = 0;
spin_unlock_irq(&ide_lock);
Elias Oltmanns <[email protected]> wrote:
> Elias Oltmanns <[email protected]> wrote:
>> Hi Bart,
>>
>> may I ask you to apply yet another inter-diff? This is in order to
>> address three issues:
>>
>> 1. Make sure that no negative value is being passed to
>> jiffies_to_msecs() in ide_park_show().
>> 2. Drop the superfluous variable hwif in ide_special_rq().
>> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
>> handling a (un)park request.
>
> Well, #3 should have been done differently because we donn't want to
> check for REQ_(UN)?PARK_HEADS more often than is necessary.
While preparing the backport to 2.6.27, it has just occurred to me that
we need to clear the IDE_DFLAG_PARKED flag in ide_disk_pre_reset()
because this flag must not be set after *any* sort of access to the
device.
So, here is yet another revised version of the inter-diff. Just don't
hurry to apply in case I have an enlightening dream tonight and want to
change something more ;-).
Regards,
Elias
Signed-off-by: Elias Oltmanns <[email protected]>
---
ide-io.c | 47 +++++++++++++++++++++++++----------------------
ide-iops.c | 1 +
ide-park.c | 6 ++++--
3 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 09d10a5..77c6eae 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,25 +672,32 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
{
- ide_hwif_t *hwif = drive->hwif;
- ide_task_t task;
- struct ide_taskfile *tf = &task.tf;
+ u8 cmd = rq->cmd[0];
+
+ if (cmd == REQ_PARK_HEADS || cmd == REQ_UNPARK_HEADS) {
+ ide_task_t task;
+ struct ide_taskfile *tf = &task.tf;
+
+ memset(&task, 0, sizeof(task));
+ if (cmd == REQ_PARK_HEADS) {
+ drive->sleep = *(unsigned long *)rq->special;
+ drive->dev_flags |= IDE_DFLAG_SLEEPING;
+ tf->command = ATA_CMD_IDLEIMMEDIATE;
+ tf->feature = 0x44;
+ tf->lbal = 0x4c;
+ tf->lbam = 0x4e;
+ tf->lbah = 0x55;
+ task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
+ } else /* cmd == REQ_UNPARK_HEADS */
+ tf->command = ATA_CMD_CHK_POWER;
+
+ task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
+ task.rq = rq;
+ drive->hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+ return do_rw_taskfile(drive, &task);
+ }
- memset(&task, 0, sizeof(task));
- switch (rq->cmd[0]) {
- case REQ_PARK_HEADS:
- drive->sleep = *(unsigned long *)rq->special;
- drive->dev_flags |= IDE_DFLAG_SLEEPING;
- tf->command = ATA_CMD_IDLEIMMEDIATE;
- tf->feature = 0x44;
- tf->lbal = 0x4c;
- tf->lbam = 0x4e;
- tf->lbah = 0x55;
- task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
- break;
- case REQ_UNPARK_HEADS:
- tf->command = ATA_CMD_CHK_POWER;
- break;
+ switch (cmd) {
case REQ_DEVSET_EXEC:
{
int err, (*setfunc)(ide_drive_t *, int) = rq->special;
@@ -710,10 +717,6 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
ide_end_request(drive, 0, 0);
return ide_stopped;
}
- task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
- task.rq = rq;
- hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
- return do_rw_taskfile(drive, &task);
}
static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 0eb6284..b762deb 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1020,6 +1020,7 @@ static void ide_disk_pre_reset(ide_drive_t *drive)
drive->special.b.recalibrate = legacy;
drive->mult_count = 0;
+ drive->dev_flags &= ~IDE_DFLAG_PARKED;
if ((drive->dev_flags & IDE_DFLAG_KEEP_SETTINGS) == 0 &&
(drive->dev_flags & IDE_DFLAG_USING_DMA) == 0)
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 35fc3ee..02d7e35 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -61,15 +61,17 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
ide_drive_t *drive = to_ide_device(dev);
+ unsigned long now;
unsigned int msecs;
if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
return -EOPNOTSUPP;
spin_lock_irq(&ide_lock);
+ now = jiffies;
if (drive->dev_flags & IDE_DFLAG_PARKED &&
- time_after(drive->sleep, jiffies))
- msecs = jiffies_to_msecs(drive->sleep - jiffies);
+ time_after(drive->sleep, now))
+ msecs = jiffies_to_msecs(drive->sleep - now);
else
msecs = 0;
spin_unlock_irq(&ide_lock);
On Sunday 05 October 2008, Elias Oltmanns wrote:
> Elias Oltmanns <[email protected]> wrote:
> > Elias Oltmanns <[email protected]> wrote:
> >> Hi Bart,
> >>
> >> may I ask you to apply yet another inter-diff? This is in order to
> >> address three issues:
> >>
> >> 1. Make sure that no negative value is being passed to
> >> jiffies_to_msecs() in ide_park_show().
> >> 2. Drop the superfluous variable hwif in ide_special_rq().
> >> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
> >> handling a (un)park request.
> >
> > Well, #3 should have been done differently because we donn't want to
> > check for REQ_(UN)?PARK_HEADS more often than is necessary.
>
> While preparing the backport to 2.6.27, it has just occurred to me that
> we need to clear the IDE_DFLAG_PARKED flag in ide_disk_pre_reset()
> because this flag must not be set after *any* sort of access to the
> device.
>
> So, here is yet another revised version of the inter-diff. Just don't
> hurry to apply in case I have an enlightening dream tonight and want to
> change something more ;-).
>
> Regards,
>
> Elias
>
> Signed-off-by: Elias Oltmanns <[email protected]>
I merged this version into the original patch.
[ Since it seems that there were no more enlightening dreams... ;) ]