On 9/14/23 07:19, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> commit 1e641060c4b5 ("libata: clear eh_info on reset completion") added
> a workaround that broke the retry mechanism in ATA EH.
>
> Tejun himself suggested to remove this workaround when it was identified
> to cause additional problems:
> https://lore.kernel.org/linux-ide/20110426135027.GI878@xxxxxxxxxxxxxx/
>
> He and even said:
> "Hmm... it seems I wasn't thinking straight when I added that work around."
> https://lore.kernel.org/linux-ide/20110426155229.GM878@xxxxxxxxxxxxxx/
>
> While removing the workaround solved the issue, however, the workaround was
> kept to avoid "spurious hotplug events during reset", and instead another
> workaround was added on top of the existing workaround in commit
> 8c56cacc724c ("libata: fix unexpectedly frozen port after ata_eh_reset()").
>
> Because these IRQs happened when the port was frozen, we know that they
> were actually a side effect of PxIS and IS.IPS(x) not being cleared before
> the COMRESET. This is now done in commit 94152042eaa9 ("ata: libahci: clear
> pending interrupt status"), so these workarounds can now be removed.
>
> Since commit 1e641060c4b5 ("libata: clear eh_info on reset completion") has
> now been reverted, the ATA EH retry mechanism is functional again, so there
> is once again no need to thaw the port more than once in ata_eh_reset().
>
> This reverts "the workaround on top of the workaround" introduced in commit
> 8c56cacc724c ("libata: fix unexpectedly frozen port after ata_eh_reset()").
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
We need a fixes tag. Same for patch 1.
> ---
> drivers/ata/libata-eh.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 5c493b6316eb..4cf4f57e57b8 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2803,9 +2803,6 @@ int ata_eh_reset(struct ata_link *link, int classify,
> slave->eh_info.serror = 0;
> spin_unlock_irqrestore(link->ap->lock, flags);
>
> - if (ata_port_is_frozen(ap))
> - ata_eh_thaw_port(ap);
> -
> /*
> * Make sure onlineness and classification result correspond.
> * Hotplug could have happened during reset and some
--
Damien Le Moal
Western Digital Research
