From 16169fb781827b82a4c0261195184dcc97a57af7 Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Wed, 10 Aug 2022 19:59:09 +0200 Subject: ata: libata-core: Print timeout value when internal command times Printing the timeout value may help in troubleshooting failures. Signed-off-by: David Milburn Signed-off-by: Tomas Henzl Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 826d41f341e4..9478194740e0 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1578,8 +1578,8 @@ static unsigned ata_exec_internal_sg(struct ata_device *dev, else ata_qc_complete(qc); - ata_dev_warn(dev, "qc timeout (cmd 0x%x)\n", - command); + ata_dev_warn(dev, "qc timeout after %u msecs (cmd 0x%x)\n", + timeout, command); } spin_unlock_irqrestore(ap->lock, flags); -- cgit v1.2.3 From 99ad3f9f829fafdcd606a8b1123e331b3b53eb04 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Tue, 16 Aug 2022 13:53:28 +0200 Subject: ata: libata-core: improve parameter names for ata_dev_set_feature() ata_dev_set_feature() is currently used for enabling/disabling any ATA feature, e.g. SETFEATURES_SPINUP and SETFEATURE_SENSE_DATA, i.e. it is not only used to enable/disable SATA specific features. For most features, the enable/disable bit is specified in the subcommand specific field "count". It is only for the specific subcommands "Enable SATA feature" (0x10) and "Disable SATA feature" (0x90) where the field "count" is used to specify a feature instead of enable/disable. The parameter names for this function are thus quite misleading. Rename the parameter names to be more generic and in line with ACS-5, and remove the references to "SATA FEATURES" in the kernel-doc. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 19 +++++++++---------- drivers/ata/libata.h | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 9478194740e0..864b26009eae 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4324,13 +4324,12 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev) } /** - * ata_dev_set_feature - Issue SET FEATURES - SATA FEATURES + * ata_dev_set_feature - Issue SET FEATURES * @dev: Device to which command will be sent - * @enable: Whether to enable or disable the feature - * @feature: The sector count represents the feature to set + * @subcmd: The SET FEATURES subcommand to be sent + * @action: The sector count represents a subcommand specific action * - * Issue SET FEATURES - SATA FEATURES command to device @dev - * on port @ap with sector count + * Issue SET FEATURES command to device @dev on port @ap with sector count * * LOCKING: * PCI/etc. bus probe sem. @@ -4338,23 +4337,23 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev) * RETURNS: * 0 on success, AC_ERR_* mask otherwise. */ -unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable, u8 feature) +unsigned int ata_dev_set_feature(struct ata_device *dev, u8 subcmd, u8 action) { struct ata_taskfile tf; unsigned int err_mask; unsigned int timeout = 0; /* set up set-features taskfile */ - ata_dev_dbg(dev, "set features - SATA features\n"); + ata_dev_dbg(dev, "set features\n"); ata_tf_init(dev, &tf); tf.command = ATA_CMD_SET_FEATURES; - tf.feature = enable; + tf.feature = subcmd; tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; tf.protocol = ATA_PROT_NODATA; - tf.nsect = feature; + tf.nsect = action; - if (enable == SETFEATURES_SPINUP) + if (subcmd == SETFEATURES_SPINUP) timeout = ata_probe_timeout ? ata_probe_timeout * 1000 : SETFEATURES_SPINUP_TIMEOUT; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, timeout); diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 98bc8649c63f..bc84fbb48c0a 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -64,7 +64,7 @@ extern int ata_dev_configure(struct ata_device *dev); extern int sata_down_spd_limit(struct ata_link *link, u32 spd_limit); extern int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel); extern unsigned int ata_dev_set_feature(struct ata_device *dev, - u8 enable, u8 feature); + u8 subcmd, u8 action); extern void ata_qc_free(struct ata_queued_cmd *qc); extern void ata_qc_issue(struct ata_queued_cmd *qc); extern void __ata_qc_complete(struct ata_queued_cmd *qc); -- cgit v1.2.3 From 614065aba7041fab831e7c69ca8c7adebbc0430c Mon Sep 17 00:00:00 2001 From: Jinpeng Cui Date: Tue, 23 Aug 2022 12:29:14 +0000 Subject: ata: libata-core: remove redundant err_mask variable Return value from ata_exec_internal() directly instead of taking this in another redundant variable. Reported-by: Zeal Robot Signed-off-by: Jinpeng Cui Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 864b26009eae..0ba0e692210f 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4340,7 +4340,6 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev) unsigned int ata_dev_set_feature(struct ata_device *dev, u8 subcmd, u8 action) { struct ata_taskfile tf; - unsigned int err_mask; unsigned int timeout = 0; /* set up set-features taskfile */ @@ -4356,9 +4355,8 @@ unsigned int ata_dev_set_feature(struct ata_device *dev, u8 subcmd, u8 action) if (subcmd == SETFEATURES_SPINUP) timeout = ata_probe_timeout ? ata_probe_timeout * 1000 : SETFEATURES_SPINUP_TIMEOUT; - err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, timeout); - return err_mask; + return ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, timeout); } EXPORT_SYMBOL_GPL(ata_dev_set_feature); -- cgit v1.2.3 From e00923c59e68b63c998a0fef4145b5279331968a Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Thu, 9 Jun 2022 12:33:13 +0900 Subject: ata: libata: Rename ATA_DFLAG_NCQ_PRIO_ENABLE Rename ATA_DFLAG_NCQ_PRIO_ENABLE to ATA_DFLAG_NCQ_PRIO_ENABLED to match the fact that this flags indicates if NCQ priority use is enabled by the user. Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 4 ++-- drivers/ata/libata-sata.c | 6 +++--- include/linux/libata.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0ba0e692210f..a5c51da10638 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -719,7 +719,7 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, if (tf->flags & ATA_TFLAG_FUA) tf->device |= 1 << 7; - if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE && + if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED && class == IOPRIO_CLASS_RT) tf->hob_nsect |= ATA_PRIO_HIGH << ATA_SHIFT_PRIO; } else if (dev->flags & ATA_DFLAG_LBA) { @@ -2171,7 +2171,7 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev) return; not_supported: - dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE; + dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLED; dev->flags &= ~ATA_DFLAG_NCQ_PRIO; } diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 7a5fe41aa5ae..eef57d101ed1 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -870,7 +870,7 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device, if (!dev) rc = -ENODEV; else - ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE; + ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED; spin_unlock_irq(ap->lock); return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable); @@ -905,9 +905,9 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device, } if (input) - dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLE; + dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLED; else - dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE; + dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLED; unlock: spin_unlock_irq(ap->lock); diff --git a/include/linux/libata.h b/include/linux/libata.h index 0269ff114f5a..a505cfb92ab3 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -101,7 +101,7 @@ enum { ATA_DFLAG_UNLOCK_HPA = (1 << 18), /* unlock HPA */ ATA_DFLAG_NCQ_SEND_RECV = (1 << 19), /* device supports NCQ SEND and RECV */ ATA_DFLAG_NCQ_PRIO = (1 << 20), /* device supports NCQ priority */ - ATA_DFLAG_NCQ_PRIO_ENABLE = (1 << 21), /* Priority cmds sent to dev */ + ATA_DFLAG_NCQ_PRIO_ENABLED = (1 << 21), /* Priority cmds sent to dev */ ATA_DFLAG_INIT_MASK = (1 << 24) - 1, ATA_DFLAG_DETACH = (1 << 24), -- cgit v1.2.3 From 066de3b9d93b6564e2f68005484d8c0597620748 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Mon, 25 Jul 2022 10:01:27 +0900 Subject: ata: libata-core: Simplify ata_build_rw_tf() Since ata_build_rw_tf() is only called from ata_scsi_rw_xlat() with the tf, dev and tag arguments obtained from the queued command structure, we can simplify the interface of ata_build_rw_tf() by passing directly the qc structure as argument. Furthermore, since ata_scsi_rw_xlat() is never used for internal commands, we can also remove the internal tag check for the NCQ case. Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 20 ++++++++++---------- drivers/ata/libata-scsi.c | 4 +--- drivers/ata/libata.h | 5 ++--- 3 files changed, 13 insertions(+), 16 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index a5c51da10638..0b62fa14a74c 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -665,33 +665,33 @@ u64 ata_tf_read_block(const struct ata_taskfile *tf, struct ata_device *dev) /** * ata_build_rw_tf - Build ATA taskfile for given read/write request - * @tf: Target ATA taskfile - * @dev: ATA device @tf belongs to + * @qc: Metadata associated with the taskfile to build * @block: Block address * @n_block: Number of blocks * @tf_flags: RW/FUA etc... - * @tag: tag * @class: IO priority class * * LOCKING: * None. * - * Build ATA taskfile @tf for read/write request described by - * @block, @n_block, @tf_flags and @tag on @dev. + * Build ATA taskfile for the command @qc for read/write request described + * by @block, @n_block, @tf_flags and @class. * * RETURNS: * * 0 on success, -ERANGE if the request is too large for @dev, * -EINVAL if the request is invalid. */ -int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, - u64 block, u32 n_block, unsigned int tf_flags, - unsigned int tag, int class) +int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block, + unsigned int tf_flags, int class) { + struct ata_taskfile *tf = &qc->tf; + struct ata_device *dev = qc->dev; + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; tf->flags |= tf_flags; - if (ata_ncq_enabled(dev) && !ata_tag_internal(tag)) { + if (ata_ncq_enabled(dev)) { /* yay, NCQ */ if (!lba_48_ok(block, n_block)) return -ERANGE; @@ -704,7 +704,7 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, else tf->command = ATA_CMD_FPDMA_READ; - tf->nsect = tag << 3; + tf->nsect = qc->hw_tag << 3; tf->hob_feature = (n_block >> 8) & 0xff; tf->feature = n_block & 0xff; diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 29e2f55c6faa..f3c64e796423 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1605,9 +1605,7 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc) qc->flags |= ATA_QCFLAG_IO; qc->nbytes = n_block * scmd->device->sector_size; - rc = ata_build_rw_tf(&qc->tf, qc->dev, block, n_block, tf_flags, - qc->hw_tag, class); - + rc = ata_build_rw_tf(qc, block, n_block, tf_flags, class); if (likely(rc == 0)) return 0; diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index bc84fbb48c0a..2c5c8273af01 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -44,9 +44,8 @@ static inline void ata_force_cbl(struct ata_port *ap) { } #endif extern u64 ata_tf_to_lba(const struct ata_taskfile *tf); extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf); -extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, - u64 block, u32 n_block, unsigned int tf_flags, - unsigned int tag, int class); +extern int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block, + unsigned int tf_flags, int class); extern u64 ata_tf_read_block(const struct ata_taskfile *tf, struct ata_device *dev); extern unsigned ata_exec_internal(struct ata_device *dev, -- cgit v1.2.3 From 024811a2da45a79d58ba61b524441722510d5dc5 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 26 Aug 2022 07:43:30 +0900 Subject: ata: libata-core: Simplify ata_dev_set_xfermode() The err_mask variable is not useful. Remove it. Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0b62fa14a74c..d0242c75aac5 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4295,7 +4295,6 @@ static void ata_dev_xfermask(struct ata_device *dev) static unsigned int ata_dev_set_xfermode(struct ata_device *dev) { struct ata_taskfile tf; - unsigned int err_mask; /* set up set-features taskfile */ ata_dev_dbg(dev, "set features - xfer mode\n"); @@ -4317,10 +4316,11 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev) else /* In the ancient relic department - skip all of this */ return 0; - /* On some disks, this command causes spin-up, so we need longer timeout */ - err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 15000); - - return err_mask; + /* + * On some disks, this command causes spin-up, so we need longer + * timeout. + */ + return ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 15000); } /** -- cgit v1.2.3 From 55d5ba550535c970c03cd0d0008ad1d61b238be4 Mon Sep 17 00:00:00 2001 From: Li Zhong Date: Sat, 3 Sep 2022 16:10:39 -0700 Subject: ata: libata-core: Check errors in sata_print_link_status() sata_scr_read() could return negative error code on failure. Check the return value when reading the control register. Signed-off-by: Li Zhong Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/ata/libata-core.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index d0242c75aac5..75b86913db1a 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -3021,7 +3021,8 @@ static void sata_print_link_status(struct ata_link *link) if (sata_scr_read(link, SCR_STATUS, &sstatus)) return; - sata_scr_read(link, SCR_CONTROL, &scontrol); + if (sata_scr_read(link, SCR_CONTROL, &scontrol)) + return; if (ata_phys_link_online(link)) { tmp = (sstatus >> 4) & 0xf; -- cgit v1.2.3