[PATCH RESEND 0/5] ata: fix platform driver probe, removal and locking issues

[PATCH RESEND 0/5] ata: fix platform driver probe, removal and locking issues

[shao.mingyin]

The ATA platform drivers in this series have independent defects in
their probe, removal or locking paths.  They share the same shape:
state established in probe is not checked, torn down or serialized
consistently.

Patch 1 fixes recursive locking in the IXP4xx CF PIO callback, which
can hang the first data transfer, including IDENTIFY during probe.  Patch 2
returns the host activation error from the Buddha probe path instead of
reporting a successful bind.  Patch 3 makes the QorIQ and Seattle
drivers inherit ahci_platform_ops so that host teardown runs host_stop.
Patch 4 stops only the Gemini SATA bridges that started successfully.
Patch 5 serializes the controller-wide SGPIO update in the Seattle LED
callback.

Each patch touches one driver, except patch 3, which covers the QorIQ
and Seattle AHCI platform drivers.  The patches are independent and any
subset can be applied; the order lists the more severe failures first.

All five patches carry QEMU A/B test descriptions after the --- line.

This is a resend of [1].  Patches 1/5 and 3/5 lost their final
context line in transport, so git apply reported a corrupt patch;
5/5 had the same trailing line.  The diff content is unchanged; all
five patches now end with the format-patch signature, which keeps the
blank context line in place.  Sorry for the noise.

[1] https://lore.kernel.org/all/20261112221230052E8YTrPU5pi8AJJy6dnh9E@zte.com.cn/

Shao Mingyin (5):
  ata: pata_ixp4xx_cf: avoid recursive locking in the PIO callback
  ata: pata_buddha: propagate host activation errors
  ata: ahci: release QorIQ and Seattle platform resources on removal
  ata: pata_ftide010: stop only successfully started Gemini bridges
  ata: ahci_seattle: serialize SGPIO register updates

 drivers/ata/ahci_qoriq.c     |  2 +-
 drivers/ata/ahci_seattle.c   |  8 ++++----
 drivers/ata/pata_buddha.c    |  6 ++----
 drivers/ata/pata_ftide010.c  | 37 +++++++++++++++++++------------------
 drivers/ata/pata_ixp4xx_cf.c |  5 +----
 5 files changed, 27 insertions(+), 31 deletions(-)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

[PATCH RESEND 1/5] ata: pata_ixp4xx_cf: avoid recursive locking in the PIO callback

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

ixp4xx_mmio_data_xfer() takes ap->lock, but the SFF state machine
already holds that lock when it invokes ->sff_data_xfer(), both
from interrupt context and from the PIO work item. The first data
transfer, including IDENTIFY during probe, therefore recursively
takes the same spinlock and can hang the CPU.

Use the lock held by the caller while switching the expansion bus
width and transferring data. Assert that the lock is held rather
than acquiring it again.

Fixes: 47adef20e67d ("pata: ixp4xx: Rewrite to use device tree")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.4.0-rc1-next-20261105 under QEMU/KVM (x86_64, 2 vCPUs),
with lockdep, DEBUG_SPINLOCK and KASAN. Both kernels carry the same
TEST-ONLY harness: a synthetic ATA host and RAM-backed registers feed
the original SFF state machine and IXP4xx PIO callback. Only this fix
differs between the two kernels.

The unpatched kernel reports recursive locking of host->lock from
ixp4xx_mmio_data_xfer() and stops in the first SFF transfer. With the
fix, SFF calls with in_wq=0 and in_wq=1 each transfer 512 bytes and
restore the 8-bit bus timing. Separate 513-byte read/write checks also
pass, including the trailing byte and buffer sentinel. The patched
guest shuts down normally without lockdep or KASAN reports. This
tests the callback and SFF interaction, not IXP4xx hardware, probe,
interrupt delivery or PIO worker scheduling.

 drivers/ata/pata_ixp4xx_cf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 80f6a91acf..6ea671d218 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -131,11 +131,10 @@ static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc,
 	struct ata_port *ap = qc->dev->link->ap;
 	void __iomem *mmio = ap->ioaddr.data_addr;
 	struct ixp4xx_pata *ixpp = ap->host->private_data;
-	unsigned long flags;
 
 	ata_dev_dbg(adev, "%s %d bytes\n", (rw == READ) ? "READ" : "WRITE",
 		    buflen);
-	spin_lock_irqsave(ap->lock, flags);
+	lockdep_assert_held(ap->lock);
 
 	/* set the expansion bus in 16bit mode and restore
 	 * 8 bit mode after the transaction.
@@ -169,8 +168,6 @@ static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc,
 	ixp4xx_set_8bit_timing(ixpp, adev->pio_mode);
 	udelay(5);
 
-	spin_unlock_irqrestore(ap->lock, flags);
-
 	return words << 1;
 }
 
-- 
2.27.0

[PATCH RESEND 2/5] ata: pata_buddha: propagate host activation errors

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pata_buddha_probe() returns success even if ata_host_activate()
fails to start the host, request its IRQ or register its ports.
This leaves a Buddha or Catweasel device bound without usable ATA
ports and allows the remove callback to detach an unregistered host.

Return the activation error so that the driver core rejects the
binding and releases the resources allocated during probe.

Fixes: 12ce6b0d21c2 ("ata/pata_buddha: Probe via modalias instead of initcall")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.4.0-rc1-next-20261105 under QEMU/TCG (m68k virt), with
SLUB debugging. Both kernels use the same TEST-ONLY harness, which
registers synthetic Buddha and Catweasel devices on the real Zorro
bus and calls the original probe. RAM represents board registers;
dummy port methods and invalid IRQ/SCSI parameters exercise the real
ata_host_activate() failure paths. Only this fix differs.

In 20 scenarios per kernel, all 16 activation failures incorrectly
bind the device and retain its managed resources on the unpatched
kernel. With the fix, probe preserves -ENOMEM/-EBUSY/-EINVAL, leaves
the device unbound and releases the MMIO region and devres immediately.
Four successful controls still register and detach normally.

A guard records and skips unsafe detach of unregistered hosts on the
unpatched kernel; no crash is claimed. AutoConfig hardware discovery,
physical IRQ/PIO, disk I/O and the manual X-Surf path were not tested.

 drivers/ata/pata_buddha.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/pata_buddha.c b/drivers/ata/pata_buddha.c
index 5a580ef4fb..bc8719e5d5 100644
--- a/drivers/ata/pata_buddha.c
+++ b/drivers/ata/pata_buddha.c
@@ -238,10 +238,8 @@ static int pata_buddha_probe(struct zorro_dev *z,
 			      ctl ? board + buddha_bases[i] + ctl : 0);
 	}
 
-	ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
-			  IRQF_SHARED, &pata_buddha_sht);
-
-	return 0;
+	return ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
+				 IRQF_SHARED, &pata_buddha_sht);
 }
 
 static void pata_buddha_remove(struct zorro_dev *z)
-- 
2.27.0

[PATCH RESEND 3/5] ata: ahci: release QorIQ and Seattle platform resources on removal

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

The QorIQ and Seattle drivers enable platform clocks, PHYs and
regulators during probe, but their port operations inherit directly
from ahci_ops. This omits the platform host_stop callback, so device
removal releases the resource handles without undoing their enables.
Repeated unbind and bind cycles can leave enable counts unbalanced.

Inherit from ahci_platform_ops so that host teardown disables the
platform resources after stopping the ports. Cover both Seattle
operation tables, including the variant without SGPIO support.

Fixes: ecfb4598512a ("ahci: added a new driver for supporting Freescale AHCI sata")
Fixes: 535dac4ab5f4 ("ata: add AMD Seattle platform driver")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on next-20261105 in an x86-64 QEMU/KVM guest with KASAN and
lock checking. TEST-ONLY hooks supply RAM-backed AHCI registers and
observable clock, PHY and regulator providers, retaining the original
QorIQ and both Seattle operation tables and probe rollback paths.

Each variant runs 12 start/devres-release cycles: unpatched, clock and
PHY counts grow to 12 and all four regulators remain enabled; patched,
ports stop before one platform host_stop and all counts return to zero.
Per variant, 12 IRQ activation failures and 12 PHY enable failures also
leave resources balanced in both kernels, without duplicate disables.
Firmware discovery, full host registration and real SATA I/O are outside
this test.

 drivers/ata/ahci_qoriq.c   | 2 +-
 drivers/ata/ahci_seattle.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
index fc6b8cd75b..3e3ce1a02b 100644
--- a/drivers/ata/ahci_qoriq.c
+++ b/drivers/ata/ahci_qoriq.c
@@ -147,7 +147,7 @@ static int ahci_qoriq_hardreset(struct ata_link *link, unsigned int *class,
 }
 
 static struct ata_port_operations ahci_qoriq_ops = {
-	.inherits		= &ahci_ops,
+	.inherits		= &ahci_platform_ops,
 	.reset.hardreset	= ahci_qoriq_hardreset,
 };
 
diff --git a/drivers/ata/ahci_seattle.c b/drivers/ata/ahci_seattle.c
index 9758fa0260..0187eb2d27 100644
--- a/drivers/ata/ahci_seattle.c
+++ b/drivers/ata/ahci_seattle.c
@@ -48,7 +48,7 @@ struct seattle_plat_data {
 };
 
 static struct ata_port_operations ahci_port_ops = {
-	.inherits		= &ahci_ops,
+	.inherits		= &ahci_platform_ops,
 };
 
 static const struct ata_port_info ahci_port_info = {
@@ -59,7 +59,7 @@ static const struct ata_port_info ahci_port_info = {
 };
 
 static struct ata_port_operations ahci_seattle_ops = {
-	.inherits		= &ahci_ops,
+	.inherits		= &ahci_platform_ops,
 	.transmit_led_message   = seattle_transmit_led_message,
 };
 
-- 
2.27.0

[PATCH RESEND 4/5] ata: pata_ftide010: stop only successfully started Gemini bridges

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

A Gemini ATA port can start successfully when only one of its SATA
bridges comes online. gemini_sata_start_bridge() disables the clock
of a failed bridge, but port_stop later stops every mapped bridge,
including the failed one. A missing disk on one bridge can thus cause
an unbalanced clk_disable() warning when the port is removed.

Record which bridges start successfully and stop only those bridges.
Clear the bitmap after stopping them so a later start begins with
no bridges marked active.

Fixes: be4e456ed3a5 ("ata: Add driver for Faraday Technology FTIDE010")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260626091006.5A2C11F00A3A@smtp.kernel.org/
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.4.0-rc1-next-20261105 under QEMU/KVM (x86_64, 2 vCPUs),
with lockdep and KASAN. Both kernels carry the same TEST-ONLY harness
using synthetic ATA hosts, RAM-backed bridge registers and clock
providers. The original port and bridge callbacks run through libata
and the real common clock framework; only this fix differs.

Each kernel completes 24 start/stop cycles covering both partial-failure
directions, both bridges ready, neither ready, single-bridge controls
and repeated starts with the same private state. The unpatched kernel
reports six CCF "already disabled" warnings; the patched kernel reports
none. Both retain 16 successful starts, eight -EINVAL returns and zero
clock enables left over. The patched guest shuts down without lockdep
or KASAN reports. Real Gemini probe, PHY hardware, disk I/O, DMA
allocation failure and full device unbind were not exercised.

 drivers/ata/pata_ftide010.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/ata/pata_ftide010.c b/drivers/ata/pata_ftide010.c
index c41da296eb..ca683fd96f 100644
--- a/drivers/ata/pata_ftide010.c
+++ b/drivers/ata/pata_ftide010.c
@@ -29,6 +29,7 @@
  * @master_cbl: master cable type
  * @slave_cbl: slave cable type
  * @sg: Gemini SATA bridge pointer, if running on the Gemini
+ * @sata_bridge_mask: bitmap of Gemini SATA bridges successfully started
  * @master_to_sata0: Gemini SATA bridge: the ATA master is connected
  * to the SATA0 bridge
  * @slave_to_sata0: Gemini SATA bridge: the ATA slave is connected
@@ -47,6 +48,7 @@ struct ftide010 {
 	unsigned int slave_cbl;
 	/* Gemini-specific properties */
 	struct sata_gemini *sg;
+	unsigned int sata_bridge_mask;
 	bool master_to_sata0;
 	bool slave_to_sata0;
 	bool master_to_sata1;
@@ -281,28 +283,36 @@ static int pata_ftide010_gemini_port_start(struct ata_port *ap)
 	if (ftide->master_to_sata0) {
 		dev_info(dev, "SATA0 (master) start\n");
 		ret = gemini_sata_start_bridge(sg, 0);
-		if (!ret)
+		if (!ret) {
+			ftide->sata_bridge_mask |= BIT(0);
 			bridges++;
+		}
 	}
 	if (ftide->master_to_sata1) {
 		dev_info(dev, "SATA1 (master) start\n");
 		ret = gemini_sata_start_bridge(sg, 1);
-		if (!ret)
+		if (!ret) {
+			ftide->sata_bridge_mask |= BIT(1);
 			bridges++;
+		}
 	}
 	/* Avoid double-starting */
 	if (ftide->slave_to_sata0 && !ftide->master_to_sata0) {
 		dev_info(dev, "SATA0 (slave) start\n");
 		ret = gemini_sata_start_bridge(sg, 0);
-		if (!ret)
+		if (!ret) {
+			ftide->sata_bridge_mask |= BIT(0);
 			bridges++;
+		}
 	}
 	/* Avoid double-starting */
 	if (ftide->slave_to_sata1 && !ftide->master_to_sata1) {
 		dev_info(dev, "SATA1 (slave) start\n");
 		ret = gemini_sata_start_bridge(sg, 1);
-		if (!ret)
+		if (!ret) {
+			ftide->sata_bridge_mask |= BIT(1);
 			bridges++;
+		}
 	}
 
 	dev_info(dev, "brought %d bridges online\n", bridges);
@@ -315,24 +325,15 @@ static void pata_ftide010_gemini_port_stop(struct ata_port *ap)
 	struct device *dev = ftide->dev;
 	struct sata_gemini *sg = ftide->sg;
 
-	if (ftide->master_to_sata0) {
-		dev_info(dev, "SATA0 (master) stop\n");
-		gemini_sata_stop_bridge(sg, 0);
-	}
-	if (ftide->master_to_sata1) {
-		dev_info(dev, "SATA1 (master) stop\n");
-		gemini_sata_stop_bridge(sg, 1);
-	}
-	/* Avoid double-stopping */
-	if (ftide->slave_to_sata0 && !ftide->master_to_sata0) {
-		dev_info(dev, "SATA0 (slave) stop\n");
+	if (ftide->sata_bridge_mask & BIT(0)) {
+		dev_info(dev, "SATA0 stop\n");
 		gemini_sata_stop_bridge(sg, 0);
 	}
-	/* Avoid double-stopping */
-	if (ftide->slave_to_sata1 && !ftide->master_to_sata1) {
-		dev_info(dev, "SATA1 (slave) stop\n");
+	if (ftide->sata_bridge_mask & BIT(1)) {
+		dev_info(dev, "SATA1 stop\n");
 		gemini_sata_stop_bridge(sg, 1);
 	}
+	ftide->sata_bridge_mask = 0;
 }
 
 static int pata_ftide010_gemini_cable_detect(struct ata_port *ap)
-- 
2.27.0

[PATCH RESEND 5/5] ata: ahci_seattle: serialize SGPIO register updates

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

seattle_transmit_led_message() updates a controller-wide SGPIO
register before taking ap->lock. Concurrent LED updates for different
ports can therefore read the same register value and overwrite each
other's changes, leaving activity, locate or fault LEDs in the wrong
state. The lock currently protects only the saved LED state.

Take the shared host lock before reading the SGPIO register and hold
it through the write and the saved-state update, so that each update
preserves the other ports' LED bits.

Fixes: 535dac4ab5f4 ("ata: add AMD Seattle platform driver")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on next-20261105 in an x86-64 QEMU/KVM guest with KASAN and
lock checking. Two kernel threads on separate CPUs call the original
LED callback for synthetic libata ports verified to share the host
lock, using a RAM-backed SGPIO register and TEST-ONLY MMIO observers.
The observer waits only when unlocked, with IRQs enabled; it never
waits for another thread while holding the callback's spinlock.

In 1025 controlled update pairs, the unpatched kernel loses register
bits 897 times (including expected 0x900, actual 0x100); the patched
kernel loses none. Another 256 pairs with IRQ-disabled entry preserve
all updates after the fix. Saved PMP LED states and entry IRQ state
are correct in both kernels; 482 invalid-PMP calls return -EINVAL.
This tests callback synchronization, not Seattle hardware or LED I/O.

 drivers/ata/ahci_seattle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci_seattle.c b/drivers/ata/ahci_seattle.c
index 9758fa0260..a398778ebf 100644
--- a/drivers/ata/ahci_seattle.c
+++ b/drivers/ata/ahci_seattle.c
@@ -92,6 +92,8 @@ static ssize_t seattle_transmit_led_message(struct ata_port *ap, u32 state,
 		return -EINVAL;
 	emp = &pp->em_priv[pmp];
 
+	spin_lock_irqsave(ap->lock, flags);
+
 	val = ioread32(plat_data->sgpio_ctrl);
 	if (state & ACTIVITY_MASK)
 		val |= 1 << ACTIVITY_BIT_POS((ap->port_no));
@@ -110,8 +112,6 @@ static ssize_t seattle_transmit_led_message(struct ata_port *ap, u32 state,
 
 	iowrite32(val, plat_data->sgpio_ctrl);
 
-	spin_lock_irqsave(ap->lock, flags);
-
 	/* save off new led state for port/slot */
 	emp->led_state = state;
 
-- 
2.27.0

Re: [PATCH RESEND 0/5] ata: fix platform driver probe, removal and locking issues

[Niklas Cassel]

Hello Shao,

On Sat, Nov 14, 2026 at 02:03:05AM +0800, shao.mingyin@zte.com.cn wrote:
> The ATA platform drivers in this series have independent defects in
> their probe, removal or locking paths.  They share the same shape:
> state established in probe is not checked, torn down or serialized
> consistently.
> 
> Patch 1 fixes recursive locking in the IXP4xx CF PIO callback, which
> can hang the first data transfer, including IDENTIFY during probe.  Patch 2
> returns the host activation error from the Buddha probe path instead of
> reporting a successful bind.  Patch 3 makes the QorIQ and Seattle
> drivers inherit ahci_platform_ops so that host teardown runs host_stop.
> Patch 4 stops only the Gemini SATA bridges that started successfully.
> Patch 5 serializes the controller-wide SGPIO update in the Seattle LED
> callback.
> 
> Each patch touches one driver, except patch 3, which covers the QorIQ
> and Seattle AHCI platform drivers.  The patches are independent and any
> subset can be applied; the order lists the more severe failures first.
> 
> All five patches carry QEMU A/B test descriptions after the --- line.
> 
> This is a resend of [1].  Patches 1/5 and 3/5 lost their final
> context line in transport, so git apply reported a corrupt patch;
> 5/5 had the same trailing line.  The diff content is unchanged; all
> five patches now end with the format-patch signature, which keeps the
> blank context line in place.  Sorry for the noise.
> 
> [1] https://lore.kernel.org/all/20261112221230052E8YTrPU5pi8AJJy6dnh9E@zte.com.cn/
> 
> Shao Mingyin (5):
>   ata: pata_ixp4xx_cf: avoid recursive locking in the PIO callback
>   ata: pata_buddha: propagate host activation errors
>   ata: ahci: release QorIQ and Seattle platform resources on removal
>   ata: pata_ftide010: stop only successfully started Gemini bridges
>   ata: ahci_seattle: serialize SGPIO register updates
> 
>  drivers/ata/ahci_qoriq.c     |  2 +-
>  drivers/ata/ahci_seattle.c   |  8 ++++----
>  drivers/ata/pata_buddha.c    |  6 ++----
>  drivers/ata/pata_ftide010.c  | 37 +++++++++++++++++++------------------
>  drivers/ata/pata_ixp4xx_cf.c |  5 +----
>  5 files changed, 27 insertions(+), 31 deletions(-)
> 
> ---
> base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

This series looks good to me.

Unless any review comment shows up, I will apply it early next week.


Kind regards,
Niklas