Re: [PATCH 1/2] ata: libata-core: Disable LPM on some WD drives

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect loop iteration in `ata_scsi_lpm_supported` completely ignores `ATA_QUIRK_NOLPM` for devices attached via a Port Multiplier.
--

--- Patch [1]: [PATCH 1/2] ata: libata-core: Disable LPM on some WD drives ---
commit ddcafc7d1d056ed46964475cc9dcb5abcb0d3b87
Author: Niklas Cassel <cassel@kernel.org>

ata: libata-core: Disable LPM on some WD drives

This commit adds the ATA_QUIRK_NOLPM quirk for specific WD drives that
have problems with Link Power Management (LPM).

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220693

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index c43bd28b20b1a..15781b4f04897 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4413,6 +4413,14 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
>  	{ "WDC WD3000JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  	{ "WDC WD3200JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  
> +	/*
> +	 * WD drives with LPM issues (irrespective of supported SATA speeds).
> +	 * (Unlike ATA_QUIRK_WD_BROKEN_LPM, which is only applied if the drive
> +	 * exposes SATA Gen1 speed support, and SATA Gen1 speed support only.)
> +	 */
> +	{ "WDC WD100EFGX-68CPLN0",	NULL,	ATA_QUIRK_NOLPM },
> +	{ "WDC WD102KFBX-68M95N0",	NULL,	ATA_QUIRK_NOLPM },

[Severity: High]
This is a pre-existing issue, but will these devices still experience LPM
issues if they are attached via a Port Multiplier?

Looking at ata_scsi_lpm_supported() in drivers/ata/libata-sata.c:

static bool ata_scsi_lpm_supported(struct ata_port *ap)
{
	struct ata_link *link;
	struct ata_device *dev;

	if ((ap->flags & ATA_FLAG_NO_LPM) || !ap->ops->set_lpm)
		return false;

	ata_for_each_link(link, ap, EDGE) {
		ata_for_each_dev(dev, &ap->link, ENABLED) {
			if (dev->quirks & ATA_QUIRK_NOLPM)
				return false;
		}
	}

	return true;
}

The outer loop correctly iterates over all links, including PMP links, but
the inner loop iterates over &ap->link instead of the current link being
evaluated.

Could this cause the code to repeatedly check the host link's devices while
entirely ignoring devices on PMP links? If so, the ATA_QUIRK_NOLPM quirk
might be bypassed for these buggy drives when they are attached via a Port
Multiplier and power management is configured.

> +
>  	/*
>  	 * This sata dom device goes on a walkabout when the ATA_LOG_DIRECTORY
>  	 * log page is accessed. Ensure we never ask for this log page with

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728111310.722450-4-cassel@kernel.org?part=1