Impact: fix infinite retry loop
After a command failed, ide-tape and floppy inserts REQUEST_SENSE in
front of the failed command and according to the result, sets
pc->retries, flags and errors. After REQUEST_SENSE is complete, the
failed command is again at the front of the queue and if the verdict
was to terminate the request, the issue functions tries to complete it
directly by calling drive->pc_callback() and returning ide_stopped.
However, drive->pc_callback() doesn't complete a request. It only
prepares for completion of the request. As a result, this creates an
infinite loop where the failed request is retried perpetually.
Fix it by actually ending the request by calling ide_complete_rq().
Signed-off-by: Tejun Heo <tj@kernel.org>
Impact: cleanup rq->data usage
ide-pm uses rq->data to carry pointer to struct request_pm_state
through request queue and rq->special is used to carray pointer to
local struct ide_cmd, which isn't necessary. Use rq->special for
request_pm_state instead and use local ide_cmd in
ide_start_power_step().
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Impact: unify request data buffer handling
rq->data is used mostly to pass kernel buffer through request queue
without using bio. There are only a couple of places which still do
this in kernel and converting to bio isn't difficult.
This patch converts ide-cd and atapi to use bio instead of rq->data
for request sense and internal pc commands. With previous change to
unify sense request handling, this is relatively easily achieved by
adding blk_rq_map_kern() during sense_rq prep and PC issue.
If blk_rq_map_kern() fails for sense, the error is deferred till sense
issue and aborts the failed command which triggered the sense. Note
that this is a slim possibility as sense prep is done on each command
issue, so for the above condition to actually trigger, all preps since
the last sense issue till the issue of the request which would require
a sense should fail.
* do_request functions might sleep now. This should be okay as ide
request_fn - do_ide_request() - is invoked only from make_request
and plug work. Make sure this is the case by adding might_sleep()
to do_ide_request().
* Functions which access the read sense data before the sense request
is complete now should access bio_data(sense_rq->bio) as the sense
buffer might have been copied during blk_rq_map_kern().
* ide-tape updated to map sg.
* cdrom_do_block_pc() now doesn't have to deal with REQ_TYPE_ATA_PC
special case. Simplified.
* tp_ops->output/input_data path dropped from ide_pc_intr().
Signed-off-by: Tejun Heo <tj@kernel.org>
Since we're issuing REQ_TYPE_SENSE now we need to allow those types of
rqs in the ->do_request callbacks. As a future improvement, sense_len
assignment might be unified across all ATAPI devices. Borislav to
check with specs and test.
As a result, get rid of ide_queue_pc_head() and
drive->request_sense_rq.
tj: * Init request sense ide_atapi_pc from sense request. In the
longer timer, it would probably better to fold
ide_create_request_sense_cmd() into its only current user -
ide_floppy_get_format_progress().
* ide_retry_pc() no longer takes @disk.
CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Preallocate a sense request in the ->do_request method and reinitialize
it only on demand, in case it's been consumed in the IRQ handler path.
The reason for this is that we don't want to be mapping rq to bio in
the IRQ path and introduce all kinds of unnecessary hacks to the block
layer.
tj: * Both user and kernel PC requests expect sense data to be stored
in separate storage other than drive->sense_data. Copy sense
data to rq->sense on completion if rq->sense is not NULL. This
fixes bogus sense data on PC requests.
As a result, remove cdrom_queue_request_sense.
CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This is in preparation of removing the queueing of a sense request out
of the IRQ handler path.
Use struct request_sense as a general sense buffer for all ATAPI
devices ide-{floppy,tape,cd}.
tj: * blk_get_request(__GFP_WAIT) can't be called from do_request() as
it can cause deadlock. Converted to use inline struct request
and blk_rq_init().
* Added xfer / cdb len selection depending on device type.
* All sense prep logics folded into ide_prep_sense() which never
fails.
* hwif->rq clearing and sense_rq used handling moved into
ide_queue_sense_rq().
* blk_rq_map_kern() conversion is moved to later patch.
CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Impact: rq->buffer usage cleanup
ide-cd uses rq->buffer to carry pointer to the original request when
issuing REQUEST_SENSE. Use rq->special instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Impact: rq->buffer usage cleanup
ide-atapi uses rq->buffer as private opaque value for internal special
requests. rq->special isn't used for these cases (the only case where
rq->special is used is for ide-tape rw requests). Use rq->special
instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Impact: rq->buffer usage cleanup
ide_raw_taskfile() directly uses rq->buffer to carry pointer to the
data buffer. This complicates both block interface and ide backend
request handling. Use blk_rq_map_kern() instead and drop special
handling for REQ_TYPE_ATA_TASKFILE from ide_map_sg().
Note that REQ_RW setting is moved upwards as blk_rq_map_kern() uses it
to initialize bio rw flag.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Impact: remove unnecessary code path
Block pc requests always use bio and rq->data is always NULL. No need
to worry about !rq->bio cases in idefloppy_block_pc_cmd(). Note that
ide-atapi uses ide_pio_bytes() for bio PIO transfer which handle sg
fine.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Impact: code simplification
ide_cd_request_sense_fixup() clears the tail of the sense buffer if
the device didn't completely fill it. This patch makes
cdrom_queue_request_sense() clear the sense buffer before issuing the
command instead of clearing it afterwards. This simplifies code and
eases future changes.
Signed-off-by: Tejun Heo <tj@kernel.org>
Impact: fix an oops which always triggers
ide_tape_issue_pc() assumed drive->pc isn't NULL on invocation when
checking for back-to-back request sense issues but drive->pc can be
NULL and even when it's not NULL, it's not safe to dereference it once
the previous command is complete because pc could have been freed or
was on stack. Kill back-to-back REQUEST_SENSE detection.
Signed-off-by: Tejun Heo <tj@kernel.org>
Bugfixes noted by checking the code against the controller
documentation (TI document number SPRUE21):
- Remove declarations for eight non-existent registers (!);
and remove accesses to two of them.
- Remove access to various non-existent bitfields in some of
the registers which *do* exist (those fields must-be-zero).
- Provide comment to replace bogus reset logic (removed above,
it relied on non-existent bitfields). Resets require GPIO
help; this driver doesn't currently know about that.
With some minor cleanup: relocate a comment, avoid an extra
lookup of the PIO timings.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
My laptop (Acer Travelmate 660) always cuts the power when rebooting
which causes the disk to emergency-park it's head.
Add a dmi check to stop disk as for shutdown on this laptop.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
With 2.6.30-rc2 I face a kernel crash on the 32bit hppa architecture
due to ide-cd when udev creates the device nodes at startup:
Kernel Fault: Code=26 regs=8ed34c40 (Addr=00000024)
IASQ: 00000000 00000000 IAOQ: 1034b5ac 1034b5b0
IIR: 4ab30048 ISR: 00000000 IOR: 00000024
CPU: 0 CR30: 8ed34000 CR31: ffff55ff
ORIG_R28: 00000000
IAOQ[0]: ide_complete_rq+0x2c/0x70
IAOQ[1]: ide_complete_rq+0x30/0x70
RP(r2): cdrom_newpc_intr+0x178/0x46c
Backtrace:
[<1035c608>] cdrom_newpc_intr+0x178/0x46c
[<1034c494>] ide_intr+0x1b0/0x214
[<1016d284>] handle_IRQ_event+0x70/0x150
[<1016d4b0>] __do_IRQ+0x14c/0x1cc
[<102f7864>] superio_interrupt+0x88/0xbc
[<1016d284>] handle_IRQ_event+0x70/0x150
[<1016d4b0>] __do_IRQ+0x14c/0x1cc
[<10112efc>] do_cpu_irq_mask+0x9c/0xd0
[<10116068>] intr_return+0x0/0x4
This crash seems to happen due to an uninitialized variable "rc".
The compiler even warns about that:
CC drivers/ide/ide-cd.o
/mnt/sda4/home/cvs/parisc/git-kernel/linus-linux-2.6/drivers/ide/ide-cd.c: In function `cdrom_newpc_intr':
/mnt/sda4/home/cvs/parisc/git-kernel/linus-linux-2.6/drivers/ide/ide-cd.c:612: warning: `rc' might be used uninitialized in this function
After applying the trivial patch below, which just initializes
the variable to zero, the kernel doesn't crash any longer:
Starting the hotplug events dispatcher: udevd.
Synthesizing the initial hotplug events...
hda: command error: status=0x51 { DriveReady SeekComplete Error }
hda: command error: error=0x54 <3>{ AbortedCommand LastFailedSense=0x05 }
ide: failed opcode was: unknown
done.
Signed-off-by: Helge Deller <deller@gmx.de>
Acked-by: Borislav Petkov <petkovbb@gmail.com>
Cc: Linus <torvalds@linux-foundation.org>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Fix UDMA throughput bug: tCYC averages t2CYCTYP/2, but the code
previously assumed it was the same as t2CYCTYP. (That is, it was
using just one clock edge, not both.) Move the table's type
declaration so it's adjacent to the table, making it more clear
what those numbers mean.
On one system this change increased throughput by almost 4x: UDMA/66
sometimes topped 23 MB/sec (on a drive known to do much better). On
another system it was around a 10% win (UDMA/66 up to 7+ MB/sec).
The difference might be caused by the ratio between memory and IDE
clocks. In the system with large speedup, this was exactly 2 (as a
workaround for a rev 1.1 silicon bug). The other system used a more
standard ratio of 1.63 (and rev 2.1 silicon) ... clock domain synch
might have some issues, they're not unheard-of.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
The driver somehow got merged with the initializer for the dma_sff_read_status()
method missing which caused kernel panic on bootup.
This should fix the kernel.org bug #13026...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Freeing non-slab objects is bad and results in an oops. Fix it.
Reported-and-tested-by: Andrew Price <andy@andrewprice.me.uk>
Cc: Theodore Tso <tytso@mit.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Remove uneeded void casts
Signed-off-by: Jack Stone <jwjstone@fastmail.fm>
Cc: jeff@garzik.org
Cc: kernel-janitors@vger.kernel.org
Cc: Jack Stone <jwjstone@fastmail.fm>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Use ATA_DMA_* constants instead of the bare numbers for the BMIDE register bits.
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
The big driver change in 2.4.19-rc1 introduced a regression for many HPT370[A]
chips -- DMA stopped to work completely, only causing endless timeouts...
The culprit has been identified (at last!): it turned to be the code resetting
the DMA state machine before each transfer. Stop doing it now as this counter-
measure has clearly caused more harm than good.
This should fix the kernel.org bug #7703.
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
As we have already PIO 6 transfer mode supported in IDE layer, we can turn
it on in the driver.
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
Tested-by: "Steve Wootton" <swootton@esi-estech.com>
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Make the case of flushing the drive's cache explicit.
There should be no functional change resulting from this patch.
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Simplify tf_read() method, making it deal only with 'struct ide_taskfile' and
the validity flags that the upper layer passes, and factoring out the code that
deals with the high order bytes into ide_tf_readback() to be called from the
only two functions interested, ide_complete_cmd() and ide_dump_sector().
This should stop the needless code duplication in this method and so make
it about twice smaller than it was; along with simplifying the setup for
the method call, this should save both time and space...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Simplify tf_load() method, making it deal only with 'struct ide_taskfile' and
the validity flags that the upper layer passes, and moving the code that deals
with the high order bytes into the only function interested, do_rw_taskfile().
This should stop the needless code duplication in this method and so make
it about twice smaller than it was; along with simplifying the setup for the
method call, this should save both time and space...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Use write_devctl() method to clear/set the HOB bit in tf_read() method.
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Move device register masking (and setting drive->select) out of tf_load() method
and into the only function that needs to use this code, do_rw_taskfile()...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
[bart: fix whitespace error]
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Make 'struct ide_taskfile' cover only 8 register values and thus put two such
fields ('tf' and 'hob') into 'struct ide_cmd', dropping unnecessary 'tf_array'
field from it.
This required changing the prototype of ide_get_lba_addr() and ide_tf_dump().
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
[bart: fix setting of ATA_LBA bit for LBA48 commands in __ide_do_rw_disk()]
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Replace IDE_TFLAG_{IN|OUT}_* flags meaning to the taskfile register validity on
input/output by the IDE_VALID_* flags and introduce 4 symmetric 8-bit register
validity indicator subfields, 'valid.{input/output}.{tf|hob}', into the 'struct
ide_cmd' instead of using the 'tf_flags' field for that purpose (this field can
then be turned from 32-bit into 8-bit one).
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Based on earlier work by Borislav Petkov.
Fix intendation in cdrom_decode_status(), no real code changes.
While at it:
- beautify comments
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Based on earlier work by Borislav Petkov.
Unify handling of fs and pc requests in cdrom_decode_status().
While at it:
- remove unreachable code
The only change in functionality is that for pc requests more
detailed error message will be printed for following sense keys:
* ILLEGAL_REQUEST
* DATA_PROTECT
* MEDIUM_ERROR
* BLANK_CHECK
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Based on earlier work by Borislav Petkov.
Convert cdrom_decode_status() to use switch statements in
preparation to unify handling of fs and pc requests.
While at it:
- remove superfluous comments and do minor CodingStyle fixups
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
There should be no functional change resulting from this patch.
Suggested-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Now tx493[89]ide_tf_{load,read} do not contain word I/O operations.
They are endian-free now.
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>,
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
There should be no functional change resulting from this patch.
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
There should be no functional change resulting from this patch.
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Since tf_{read|load}() methods of this driver have now become identical to their
standard counterparts using I/O port accesses, there's no need to override those
anymore...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Since tf_{read|load}() methods of this driver have now become identical to their
standard counterparts using MMIO accesses, there's no need to override those
anymore...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Remove two no longer used functions that I've overlooked...
Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
* define CHECK_DMA_MASK
* remove use of wmb()
Reported-by: Grant Grundler <grundler@google.com>
Reviewed-by: Grant Grundler <grundler@google.com>
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
This patch:
o replaces "mask" variable in ide_dma_end() with #define.
o removes use of wmb() in ide-dma-sff.c and scc_pata.c.
o is not tested - I don't have (or want) the HW.
Signed-off-by: Grant Grundler <grundler@google.com>
Cc: KOBAYASHI Yoshitake <yoshitake.kobayashi@toshiba.co.jp>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Both of commits f94116aeec ("ide: cleanup
<asm-m68k/ide.h>") and 15a453a955 ("ide: include
<asm/ide.h> only when needed") break falconide:
| Uniform Multi-Platform E-IDE driver
| ide: Falcon IDE controller
| Probing IDE interface ide0...
| hda: Sarge m68k, ATA DISK drive
| ide0 at 0xfff00000 on irq 15 (serialized)
| ide-gd driver 1.18
| hda: max request size: 128KiB
| hda: 2118816 sectors (1084 MB) w/256KiB Cache, CHS=2102/16/63
| hda:<4>hda: lost interrupt
This happens because falconide relies on {in,out}sw() being redefined in
<asm/ide.h>, as included by <linux/ide.h>, which is no longer the case.
Use __ide_mm_{in,out}sw() from <asm/ide.h> instead, just like
ide_{in,out}put_data() do.
The same problem seems to exist in q40ide.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
commit 255115fb35 ("ide: allow host drivers to
specify IRQ flags") added irq_flags fields to struct ide_port_info and struct
ide_host. Drivers can now set ide_port_info.irq_flags = IRQF_SHARED, while
init_irq() passes ide_host.irq_flags to request_irq().
Unfortunately ide_host.irq_flags is never set, causing (on ARAnyM):
| Uniform Multi-Platform E-IDE driver
| ide: Falcon IDE controller
| Probing IDE interface ide0...
| hda: Sarge m68k, ATA DISK drive
| init_irq: sa = 0
| ide0: disabled, unable to get IRQ 15
| ide0: failed to initialize IDE interface
| ide0: disabling port
Solve this by copying ide_port_info.irq_flags to ide_host.irq_flags in
ide_host_alloc().
This bug probably affects the following IDE host drivers:
- buddha
- delkin_cb
- falconide
- gayle
- ide-cs
- macide
- q40ide
- scc_pata
- sgiioc4
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
