Skip to content

Commit 4790580

Browse files
committed
Merge tag 'ata-6.17-rc1-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux
Pull ata fixes from Damien Le Moal: - Cleanup whitespace in messages in libata-core and the pata_pdc2027x, pata_macio drivers (Colin) - Fix ata_to_sense_error() to avoid seeing nonsensical sense data for rare cases where we fail to get sense data from the drive. The complementary fix to this is to ensure that we always return the generic "ABORTED COMMAND" sense data for a failed command for which we have no status or error fields - The recent changes to link power management (LPM) which now prevent the user from attempting to set an LPM policy through the link_power_management_policy caused some regressions in test environments because of the error that is now returned when writing to that attribute when LPM is not supported. To allow users to not trip on this, introduce the new link_power_management_supported attribute to allow simple testing of a port/device LPM support (me) * tag 'ata-6.17-rc1-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux: ata: pata_pdc2027x: Remove space before newline and abbreviations ata: pata_macio: Remove space before newline ata: libata-core: Remove space before newline ata: libata-sata: Add link_power_management_supported sysfs attribute ata: libata-scsi: Return aborted command when missing sense and result TF ata: libata-scsi: Fix ata_to_sense_error() status handling
2 parents a530a36 + 6cb4373 commit 4790580

File tree

8 files changed

+71
-39
lines changed

8 files changed

+71
-39
lines changed

drivers/ata/ata_piix.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,7 @@ static struct ata_port_operations ich_pata_ops = {
10891089
};
10901090

10911091
static struct attribute *piix_sidpr_shost_attrs[] = {
1092+
&dev_attr_link_power_management_supported.attr,
10921093
&dev_attr_link_power_management_policy.attr,
10931094
NULL
10941095
};

drivers/ata/libahci.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
111111
static DEVICE_ATTR(em_message_supported, S_IRUGO, ahci_show_em_supported, NULL);
112112

113113
static struct attribute *ahci_shost_attrs[] = {
114+
&dev_attr_link_power_management_supported.attr,
114115
&dev_attr_link_power_management_policy.attr,
115116
&dev_attr_em_message_type.attr,
116117
&dev_attr_em_message.attr,

drivers/ata/libata-core.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4602,7 +4602,7 @@ static unsigned int ata_dev_init_params(struct ata_device *dev,
46024602
return AC_ERR_INVALID;
46034603

46044604
/* set up init dev params taskfile */
4605-
ata_dev_dbg(dev, "init dev params \n");
4605+
ata_dev_dbg(dev, "init dev params\n");
46064606

46074607
ata_tf_init(dev, &tf);
46084608
tf.command = ATA_CMD_INIT_DEV_PARAMS;

drivers/ata/libata-sata.c

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -900,14 +900,52 @@ static const char *ata_lpm_policy_names[] = {
900900
[ATA_LPM_MIN_POWER] = "min_power",
901901
};
902902

903+
/*
904+
* Check if a port supports link power management.
905+
* Must be called with the port locked.
906+
*/
907+
static bool ata_scsi_lpm_supported(struct ata_port *ap)
908+
{
909+
struct ata_link *link;
910+
struct ata_device *dev;
911+
912+
if (ap->flags & ATA_FLAG_NO_LPM)
913+
return false;
914+
915+
ata_for_each_link(link, ap, EDGE) {
916+
ata_for_each_dev(dev, &ap->link, ENABLED) {
917+
if (dev->quirks & ATA_QUIRK_NOLPM)
918+
return false;
919+
}
920+
}
921+
922+
return true;
923+
}
924+
925+
static ssize_t ata_scsi_lpm_supported_show(struct device *dev,
926+
struct device_attribute *attr, char *buf)
927+
{
928+
struct Scsi_Host *shost = class_to_shost(dev);
929+
struct ata_port *ap = ata_shost_to_port(shost);
930+
unsigned long flags;
931+
bool supported;
932+
933+
spin_lock_irqsave(ap->lock, flags);
934+
supported = ata_scsi_lpm_supported(ap);
935+
spin_unlock_irqrestore(ap->lock, flags);
936+
937+
return sysfs_emit(buf, "%d\n", supported);
938+
}
939+
DEVICE_ATTR(link_power_management_supported, S_IRUGO,
940+
ata_scsi_lpm_supported_show, NULL);
941+
EXPORT_SYMBOL_GPL(dev_attr_link_power_management_supported);
942+
903943
static ssize_t ata_scsi_lpm_store(struct device *device,
904944
struct device_attribute *attr,
905945
const char *buf, size_t count)
906946
{
907947
struct Scsi_Host *shost = class_to_shost(device);
908948
struct ata_port *ap = ata_shost_to_port(shost);
909-
struct ata_link *link;
910-
struct ata_device *dev;
911949
enum ata_lpm_policy policy;
912950
unsigned long flags;
913951

@@ -924,20 +962,11 @@ static ssize_t ata_scsi_lpm_store(struct device *device,
924962

925963
spin_lock_irqsave(ap->lock, flags);
926964

927-
if (ap->flags & ATA_FLAG_NO_LPM) {
965+
if (!ata_scsi_lpm_supported(ap)) {
928966
count = -EOPNOTSUPP;
929967
goto out_unlock;
930968
}
931969

932-
ata_for_each_link(link, ap, EDGE) {
933-
ata_for_each_dev(dev, &ap->link, ENABLED) {
934-
if (dev->quirks & ATA_QUIRK_NOLPM) {
935-
count = -EOPNOTSUPP;
936-
goto out_unlock;
937-
}
938-
}
939-
}
940-
941970
ap->target_lpm_policy = policy;
942971
ata_port_schedule_eh(ap);
943972
out_unlock:

drivers/ata/libata-scsi.c

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -859,18 +859,14 @@ static void ata_to_sense_error(u8 drv_stat, u8 drv_err, u8 *sk, u8 *asc,
859859
{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
860860
};
861861
static const unsigned char stat_table[][4] = {
862-
/* Must be first because BUSY means no other bits valid */
863-
{0x80, ABORTED_COMMAND, 0x47, 0x00},
864-
// Busy, fake parity for now
865-
{0x40, ILLEGAL_REQUEST, 0x21, 0x04},
866-
// Device ready, unaligned write command
867-
{0x20, HARDWARE_ERROR, 0x44, 0x00},
868-
// Device fault, internal target failure
869-
{0x08, ABORTED_COMMAND, 0x47, 0x00},
870-
// Timed out in xfer, fake parity for now
871-
{0x04, RECOVERED_ERROR, 0x11, 0x00},
872-
// Recovered ECC error Medium error, recovered
873-
{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
862+
/* Busy: must be first because BUSY means no other bits valid */
863+
{ ATA_BUSY, ABORTED_COMMAND, 0x00, 0x00 },
864+
/* Device fault: INTERNAL TARGET FAILURE */
865+
{ ATA_DF, HARDWARE_ERROR, 0x44, 0x00 },
866+
/* Corrected data error */
867+
{ ATA_CORR, RECOVERED_ERROR, 0x00, 0x00 },
868+
869+
{ 0xFF, 0xFF, 0xFF, 0xFF }, /* END mark */
874870
};
875871

876872
/*
@@ -942,6 +938,8 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
942938
if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {
943939
ata_dev_dbg(dev,
944940
"missing result TF: can't generate ATA PT sense data\n");
941+
if (qc->err_mask)
942+
ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
945943
return;
946944
}
947945

@@ -996,8 +994,8 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
996994

997995
if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {
998996
ata_dev_dbg(dev,
999-
"missing result TF: can't generate sense data\n");
1000-
return;
997+
"Missing result TF: reporting aborted command\n");
998+
goto aborted;
1001999
}
10021000

10031001
/* Use ata_to_sense_error() to map status register bits
@@ -1008,13 +1006,15 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
10081006
ata_to_sense_error(tf->status, tf->error,
10091007
&sense_key, &asc, &ascq);
10101008
ata_scsi_set_sense(dev, cmd, sense_key, asc, ascq);
1011-
} else {
1012-
/* Could not decode error */
1013-
ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",
1014-
tf->status, qc->err_mask);
1015-
ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
10161009
return;
10171010
}
1011+
1012+
/* Could not decode error */
1013+
ata_dev_warn(dev,
1014+
"Could not decode error 0x%x, status 0x%x (err_mask=0x%x)\n",
1015+
tf->error, tf->status, qc->err_mask);
1016+
aborted:
1017+
ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
10181018
}
10191019

10201020
void ata_scsi_sdev_config(struct scsi_device *sdev)

drivers/ata/pata_macio.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ static void pata_macio_irq_clear(struct ata_port *ap)
758758

759759
static void pata_macio_reset_hw(struct pata_macio_priv *priv, int resume)
760760
{
761-
dev_dbg(priv->dev, "Enabling & resetting... \n");
761+
dev_dbg(priv->dev, "Enabling & resetting...\n");
762762

763763
if (priv->mediabay)
764764
return;

drivers/ata/pata_pdc2027x.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
295295
}
296296

297297
/* Set the PIO timing registers using value table for 133MHz */
298-
ata_port_dbg(ap, "Set pio regs... \n");
298+
ata_port_dbg(ap, "Set PIO regs...\n");
299299

300300
ctcr0 = ioread32(dev_mmio(ap, adev, PDC_CTCR0));
301301
ctcr0 &= 0xffff0000;
@@ -308,7 +308,7 @@ static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
308308
ctcr1 |= (pdc2027x_pio_timing_tbl[pio].value2 << 24);
309309
iowrite32(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
310310

311-
ata_port_dbg(ap, "Set to pio mode[%u] \n", pio);
311+
ata_port_dbg(ap, "Set to PIO mode[%u]\n", pio);
312312
}
313313

314314
/**
@@ -341,7 +341,7 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
341341
iowrite32(ctcr1 & ~(1 << 7), dev_mmio(ap, adev, PDC_CTCR1));
342342
}
343343

344-
ata_port_dbg(ap, "Set udma regs... \n");
344+
ata_port_dbg(ap, "Set UDMA regs...\n");
345345

346346
ctcr1 = ioread32(dev_mmio(ap, adev, PDC_CTCR1));
347347
ctcr1 &= 0xff000000;
@@ -350,14 +350,14 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
350350
(pdc2027x_udma_timing_tbl[udma_mode].value2 << 16);
351351
iowrite32(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
352352

353-
ata_port_dbg(ap, "Set to udma mode[%u] \n", udma_mode);
353+
ata_port_dbg(ap, "Set to UDMA mode[%u]\n", udma_mode);
354354

355355
} else if ((dma_mode >= XFER_MW_DMA_0) &&
356356
(dma_mode <= XFER_MW_DMA_2)) {
357357
/* Set the MDMA timing registers with value table for 133MHz */
358358
unsigned int mdma_mode = dma_mode & 0x07;
359359

360-
ata_port_dbg(ap, "Set mdma regs... \n");
360+
ata_port_dbg(ap, "Set MDMA regs...\n");
361361
ctcr0 = ioread32(dev_mmio(ap, adev, PDC_CTCR0));
362362

363363
ctcr0 &= 0x0000ffff;
@@ -366,7 +366,7 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
366366

367367
iowrite32(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
368368

369-
ata_port_dbg(ap, "Set to mdma mode[%u] \n", mdma_mode);
369+
ata_port_dbg(ap, "Set to MDMA mode[%u]\n", mdma_mode);
370370
} else {
371371
ata_port_err(ap, "Unknown dma mode [%u] ignored\n", dma_mode);
372372
}

include/linux/libata.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes)
545545

546546
extern struct device_attribute dev_attr_unload_heads;
547547
#ifdef CONFIG_SATA_HOST
548+
extern struct device_attribute dev_attr_link_power_management_supported;
548549
extern struct device_attribute dev_attr_link_power_management_policy;
549550
extern struct device_attribute dev_attr_ncq_prio_supported;
550551
extern struct device_attribute dev_attr_ncq_prio_enable;

0 commit comments

Comments
 (0)