Re: [PATCH] ata: libata: Fix sata_down_spd_limit() when no link speed is reported

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Mon, Jan 30, 2023 at 08:56:45PM +0900, Damien Le Moal wrote:
> On 1/30/23 20:47, Niklas Cassel wrote:
> > On Mon, Jan 30, 2023 at 08:07:21PM +0900, Damien Le Moal wrote:
> >> Commit 2dc0b46b5ea3 ("libata: sata_down_spd_limit should return if
> >> driver has not recorded sstatus speed") changed the behavior of
> >> sata_down_spd_limit() to return doing nothing if a drive does not report
> >> a current link speed, to avoid reducing the link speed to the lowest 1.5
> >> Gbps speed.
> >>
> >> However, the change assumed that a speed was recorded before probing
> >> (e.g. before a suspend/resume) and set in link->sata_spd. This causes
> >> problems with adapters/drives combination failing to establish a link
> >> speed during probe autonegotiation. One exampe reported of this problem
> > 
> > s/exampe/example/
> > 
> >> is an mvebu adapter with a 3Gbps port-multiplier box: autonegotiation
> >> fails, leaving no recorded link speed and no rep@orted current link
> > 
> > s/rep@orted/reported/
> > 
> >> speed. Probe retries also fail as no action is taken by sata_set_spd()
> >> after each retry.
> >>
> >> Fix this by returning early in sata_down_spd_limit() only if we do have
> >> a recorded link speed, that is, if link->sata_spd is not 0. With this
> >> fix, a failed probe not leading to a recorded link speed is retried at
> >> the lower 1.5 Gbps speed, with the link speed potentially increased
> >> later on the second revalidate of the device if the device reports
> >> that it supports higher link speeds.
> >>
> >> Reported-by: Marius Dinu <marius@psihoexpert.ro>
> >> Fixes: 2dc0b46b5ea3 ("libata: sata_down_spd_limit should return if driver has not recorded sstatus speed")
> > 
> > IIRC, checkpatch.pl with the default options should put the author on the
> > commit being fixed on CC. CC:ing the original author is a good practice IMHO.
> 
> Funny that I did run checkpatch and it did not complain about the above typos :)
> Thanks for the review, will fix this.

Sorry, I meant that get_maintainer.pl with the default options usually returns
the author of the commit being fixed (see "blamed_fixes"):

$ ./scripts/get_maintainer.pl 0001-ata-libata-Fix-sata_down_spd_limit-when-no-link-spee.patch
Damien Le Moal <damien.lemoal@opensource.wdc.com> (maintainer:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers))
Tejun Heo <tj@kernel.org> (blamed_fixes:1/1=100%)
David Milburn <dmilburn@redhat.com> (blamed_fixes:1/1=100%)
linux-ide@vger.kernel.org (open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers))
linux-kernel@vger.kernel.org (open list)


Kind regards,
Niklas