* [PATCH 1/3] ata: libata-eh: Make ata_eh_followup_srst_needed() return a bool
2025-07-07 10:59 [PATCH 0/3] libata-eh cleanups Damien Le Moal
@ 2025-07-07 10:59 ` Damien Le Moal
2025-07-07 10:59 ` [PATCH 2/3] ata: libata-eh: Remove ata_do_eh() Damien Le Moal
2025-07-07 10:59 ` [PATCH 3/3] Documentation: driver-api: Update libata error handler information Damien Le Moal
2 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2025-07-07 10:59 UTC (permalink / raw)
To: linux-ide, Niklas Cassel
ata_eh_followup_srst_needed() returns an integer used as a boolean. So
change this function to return that type.
No functional changes.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/ata/libata-eh.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 42aafb1ddb5a..436536112043 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2781,15 +2781,15 @@ static int ata_do_reset(struct ata_link *link, ata_reset_fn_t reset,
return reset(link, classes, deadline);
}
-static int ata_eh_followup_srst_needed(struct ata_link *link, int rc)
+static bool ata_eh_followup_srst_needed(struct ata_link *link, int rc)
{
if ((link->flags & ATA_LFLAG_NO_SRST) || ata_link_offline(link))
- return 0;
+ return false;
if (rc == -EAGAIN)
- return 1;
+ return true;
if (sata_pmp_supported(link->ap) && ata_is_host_link(link))
- return 1;
- return 0;
+ return true;
+ return false;
}
int ata_eh_reset(struct ata_link *link, int classify,
--
2.50.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] ata: libata-eh: Remove ata_do_eh()
2025-07-07 10:59 [PATCH 0/3] libata-eh cleanups Damien Le Moal
2025-07-07 10:59 ` [PATCH 1/3] ata: libata-eh: Make ata_eh_followup_srst_needed() return a bool Damien Le Moal
@ 2025-07-07 10:59 ` Damien Le Moal
2025-07-08 4:31 ` kernel test robot
2025-07-07 10:59 ` [PATCH 3/3] Documentation: driver-api: Update libata error handler information Damien Le Moal
2 siblings, 1 reply; 5+ messages in thread
From: Damien Le Moal @ 2025-07-07 10:59 UTC (permalink / raw)
To: linux-ide, Niklas Cassel
The only reason for ata_do_eh() to exist is that the two caller sites,
ata_std_error_handler() and ata_sff_error_handler() may pass to it a
NULL hardreset operation so that the built-in (generic) hardreset
operation for a driver is ignored if the adapter SCR access is not
available.
However, ata_std_error_handler() and ata_sff_error_handler()
modifications of the hardreset port operation can easily be combined as
they are mutually exclusive. That is, a driver using
sata_std_hardreset() as its hardreset operation cannot use
sata_sff_hardreset() and vice-versa.
With this observation, ata_do_eh() can be removed and its code moved to
ata_std_error_handler() with the hardreset operation change of
ata_sff_error_handler(). With this, ata_sff_error_handler() is
simplified and only need to call ata_std_error_handler().
No functional changes.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/ata/libata-eh.c | 46 ++++++++++++----------------------------
drivers/ata/libata-sff.c | 10 +--------
include/linux/libata.h | 3 ---
3 files changed, 14 insertions(+), 45 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 436536112043..68581adc6f87 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -4067,59 +4067,39 @@ void ata_eh_finish(struct ata_port *ap)
}
/**
- * ata_do_eh - do standard error handling
+ * ata_std_error_handler - standard error handler
* @ap: host port to handle error for
*
- * @prereset: prereset method (can be NULL)
- * @softreset: softreset method (can be NULL)
- * @hardreset: hardreset method (can be NULL)
- * @postreset: postreset method (can be NULL)
- *
* Perform standard error handling sequence.
*
* LOCKING:
* Kernel thread context (may sleep).
*/
-void ata_do_eh(struct ata_port *ap, ata_prereset_fn_t prereset,
- ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
- ata_postreset_fn_t postreset)
+void ata_std_error_handler(struct ata_port *ap)
{
- struct ata_device *dev;
+ struct ata_port_operations *ops = ap->ops;
+ ata_reset_fn_t hardreset = ops->hardreset;
int rc;
+ /* Ignore built-in hardresets if SCR access is not available */
+ if ((hardreset == sata_std_hardreset ||
+ hardreset == sata_sff_hardreset) && !sata_scr_valid(&ap->link))
+ hardreset = NULL;
+
ata_eh_autopsy(ap);
ata_eh_report(ap);
- rc = ata_eh_recover(ap, prereset, softreset, hardreset, postreset,
- NULL);
+ rc = ata_eh_recover(ap, ops->prereset, ops->softreset,
+ hardreset, ops->postreset, NULL);
if (rc) {
+ struct ata_device *dev;
+
ata_for_each_dev(dev, &ap->link, ALL)
ata_dev_disable(dev);
}
ata_eh_finish(ap);
}
-
-/**
- * ata_std_error_handler - standard error handler
- * @ap: host port to handle error for
- *
- * Standard error handler
- *
- * LOCKING:
- * Kernel thread context (may sleep).
- */
-void ata_std_error_handler(struct ata_port *ap)
-{
- struct ata_port_operations *ops = ap->ops;
- ata_reset_fn_t hardreset = ops->hardreset;
-
- /* ignore built-in hardreset if SCR access is not available */
- if (hardreset == sata_std_hardreset && !sata_scr_valid(&ap->link))
- hardreset = NULL;
-
- ata_do_eh(ap, ops->prereset, ops->softreset, hardreset, ops->postreset);
-}
EXPORT_SYMBOL_GPL(ata_std_error_handler);
#ifdef CONFIG_PM
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 5a46c066abc3..e61f00779e40 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2054,8 +2054,6 @@ EXPORT_SYMBOL_GPL(ata_sff_drain_fifo);
*/
void ata_sff_error_handler(struct ata_port *ap)
{
- ata_reset_fn_t softreset = ap->ops->softreset;
- ata_reset_fn_t hardreset = ap->ops->hardreset;
struct ata_queued_cmd *qc;
unsigned long flags;
@@ -2077,13 +2075,7 @@ void ata_sff_error_handler(struct ata_port *ap)
spin_unlock_irqrestore(ap->lock, flags);
- /* ignore built-in hardresets if SCR access is not available */
- if ((hardreset == sata_std_hardreset ||
- hardreset == sata_sff_hardreset) && !sata_scr_valid(&ap->link))
- hardreset = NULL;
-
- ata_do_eh(ap, ap->ops->prereset, softreset, hardreset,
- ap->ops->postreset);
+ ata_std_error_handler(ap);
}
EXPORT_SYMBOL_GPL(ata_sff_error_handler);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index d092747be588..4a7f308fa4e5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1412,9 +1412,6 @@ extern void ata_eh_thaw_port(struct ata_port *ap);
extern void ata_eh_qc_complete(struct ata_queued_cmd *qc);
extern void ata_eh_qc_retry(struct ata_queued_cmd *qc);
-extern void ata_do_eh(struct ata_port *ap, ata_prereset_fn_t prereset,
- ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
- ata_postreset_fn_t postreset);
extern void ata_std_error_handler(struct ata_port *ap);
extern void ata_std_sched_eh(struct ata_port *ap);
extern void ata_std_end_eh(struct ata_port *ap);
--
2.50.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] Documentation: driver-api: Update libata error handler information
2025-07-07 10:59 [PATCH 0/3] libata-eh cleanups Damien Le Moal
2025-07-07 10:59 ` [PATCH 1/3] ata: libata-eh: Make ata_eh_followup_srst_needed() return a bool Damien Le Moal
2025-07-07 10:59 ` [PATCH 2/3] ata: libata-eh: Remove ata_do_eh() Damien Le Moal
@ 2025-07-07 10:59 ` Damien Le Moal
2 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2025-07-07 10:59 UTC (permalink / raw)
To: linux-ide, Niklas Cassel
Update ``->error_handler()`` section of the libata documentation file
Documentation/driver-api/libata.rst to remove the reference to the
function ata_do_eh() as that function was removed. The reference to the
function ata_bmdma_drive_eh() is also removed as that function does not
exist at all. And while at it, cleanup the description of the various
port reset operations using a bullet list.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
Documentation/driver-api/libata.rst | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Documentation/driver-api/libata.rst b/Documentation/driver-api/libata.rst
index 5da27a749246..e160ef2e0791 100644
--- a/Documentation/driver-api/libata.rst
+++ b/Documentation/driver-api/libata.rst
@@ -283,18 +283,21 @@ interrupts, start DMA engine, etc.
``->error_handler()`` is a driver's hook into probe, hotplug, and recovery
and other exceptional conditions. The primary responsibility of an
-implementation is to call :c:func:`ata_do_eh` or :c:func:`ata_bmdma_drive_eh`
-with a set of EH hooks as arguments:
+implementation is to call :c:func:`ata_std_error_handler`.
-'prereset' hook (may be NULL) is called during an EH reset, before any
-other actions are taken.
+:c:func:`ata_std_error_handler` will call the various port reset operations as
+needed.
-'postreset' hook (may be NULL) is called after the EH reset is
-performed. Based on existing conditions, severity of the problem, and
-hardware capabilities,
+* The 'prereset' operation (may be NULL) is called during an EH reset, before
+ any other action is taken.
-Either 'softreset' (may be NULL) or 'hardreset' (may be NULL) will be
-called to perform the low-level EH reset.
+* The 'postreset' hook (may be NULL) is called after the EH reset is performed.
+ Based on existing conditions, severity of the problem, and hardware
+ capabilities,
+
+* Either the 'softreset' operation (may be NULL) or the 'hardreset' operation
+ (may be NULL) will be called to perform the low-level EH reset. If both
+ operations are defined, 'hardreset' is preferred and used.
::
--
2.50.0
^ permalink raw reply related [flat|nested] 5+ messages in thread