On 20/07/2023 01:42, Niklas Cassel wrote:
From: Hannes Reinecke <hare@xxxxxxx>
With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all
libata drivers now have the error_handler() callback provided,
so we can stop checking for non-existing error_handler callback.
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
[niklas: fixed review comments, rebased, solved conflicts during rebase,
fixed bug that unconditionally dumped all QCs, removed the now unused
function ata_dump_status(), removed the now unreachable failure paths in
atapi_qc_complete(), removed the non-EH function to request ATAPI sense]
Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
ata_qc_from_tag() still has a ap->ops->error_handler check, right?
---
drivers/ata/libata-core.c | 209 +++++++++++++++-----------------------
drivers/ata/libata-eh.c | 150 ++++++++++++---------------
drivers/ata/libata-sata.c | 7 +-
drivers/ata/libata-scsi.c | 142 ++------------------------
drivers/ata/libata-sff.c | 30 ++----
5 files changed, 166 insertions(+), 372 deletions(-)
...
/*
- * For new EH, all qcs are finished in one of three ways -
+ * For EH, all qcs are finished in one of three ways -
* normal completion, error completion, and SCSI timeout.
* Both completions can race against SCSI timeout. When normal
* completion wins, the qc never reaches EH. When error
@@ -659,94 +656,89 @@ EXPORT_SYMBOL(ata_scsi_cmd_error_handler);
void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap)
{
unsigned long flags;
+ struct ata_link *link;
/* invoke error handler */
Is this comment only really relevant when we may not previously invoked
the error handler?
- if (ap->ops->error_handler) {
- struct ata_link *link;
- /* acquire EH ownership */
- ata_eh_acquire(ap);
+ /* acquire EH ownership */
+ ata_eh_acquire(ap);
repeat:
- /* kill fast drain timer */
- del_timer_sync(&ap->fastdrain_timer);
+ /* kill fast drain timer */
+ del_timer_sync(&ap->fastdrain_timer);
- /* process port resume request */
- ata_eh_handle_port_resume(ap);
+ /* process port resume request */
+ ata_eh_handle_port_resume(ap);
- /* fetch & clear EH info */
- spin_lock_irqsave(ap->lock, flags);
+ /* fetch & clear EH info */
+ spin_lock_irqsave(ap->lock, flags);
...
* ata_to_sense_error - convert ATA error to SCSI error
* @id: ATA device number
@@ -904,7 +863,6 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
struct ata_taskfile *tf = &qc->result_tf;
unsigned char *sb = cmd->sense_buffer;
unsigned char *desc = sb + 8;
- int verbose = qc->ap->ops->error_handler == NULL;
u8 sense_key, asc, ascq;
memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
@@ -916,7 +874,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
if (qc->err_mask ||
tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
- &sense_key, &asc, &ascq, verbose);
+ &sense_key, &asc, &ascq, false);
ata_scsi_set_sense(qc->dev, cmd, sense_key, asc, ascq);
} else {
/*
@@ -999,7 +957,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
struct scsi_cmnd *cmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->result_tf;
unsigned char *sb = cmd->sense_buffer;
- int verbose = qc->ap->ops->error_handler == NULL;
u64 block;
u8 sense_key, asc, ascq;
@@ -1017,7 +974,7 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
if (qc->err_mask ||
tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
- &sense_key, &asc, &ascq, verbose);
+ &sense_key, &asc, &ascq, false);
Please check this - AFAICS, we only ever pass false for @verbose arg now
(so it would not be needed, and ata_to_sense_error() may be simplified)
ata_scsi_set_sense(dev, cmd, sense_key, asc, ascq);
} else {
/* Could not decode error */
@@ -1179,9 +1136,6 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
unsigned long flags;
struct ata_device *dev;
- if (!ap->ops->error_handler)
