Re: [PATCH 0/4] Attempt to fix regression with AR8035 speed downgrade - Russell King

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	tinywrkb <tinywrkb@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: [PATCH 0/4] Attempt to fix regression with AR8035 speed downgrade
Date: Sun, 22 Sep 2019 22:20:50 +0100	[thread overview]
Message-ID: <20190922212050.GS25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <268c0ea0-8b77-23eb-26cf-820cec1343e4@gmail.com>

On Sun, Sep 22, 2019 at 11:02:13AM -0700, Florian Fainelli wrote:
> On 9/22/2019 10:52 AM, Russell King - ARM Linux admin wrote:
> > On Sun, Sep 22, 2019 at 06:53:35PM +0200, Andrew Lunn wrote:
> >> On Sun, Sep 22, 2019 at 11:59:32AM +0100, Russell King - ARM Linux admin wrote:
> >>> Hi,
> >>>
> >>> tinywrkb, please can you test this series to ensure that it fixes
> >>> your problem - the previous version has turned out to be a non-starter
> >>> as it introduces more problems, thanks!
> >>>
> >>> The following series attempts to address an issue spotted by tinywrkb
> >>> with the AR8035 on the Cubox-i2 in a situation where the PHY downgrades
> >>> the negotiated link.
> >>
> >> Hi Russell
> >>
> >> This all looks sensible.
> >>
> >> One things we need to be careful of, is this is for net and so stable.
> > 
> > Since the regression was introduced in 5.1, it should be backported
> > to stable trees.
> > 
> >> But only some of the patches have fixes-tags. I don't know if we
> >> should add fixes tags to all the patches, just to give back porters a
> >> hint that they are all needed? It won't compile without the patches,
> >> so at least it fails safe.
> > 
> > I only put Fixes: tags on patches that are actually fixing something.
> > Quoting submitting-patches.rst:
> > 
> >   A Fixes: tag indicates that the patch fixes an issue in a previous
> >   commit.
> > 
> > Since the preceding two patches are just preparing for the fix, and
> > not actually fixing an issue in themselves, it seems wrong to add a
> > Fixes: tag for them.  However, mentioning it in the commit message
> > for the patch that does fix the issue is probably worth it.  Thanks.
> > 
> 
> This is not a criticism of your patch series, which is fine.
> 
> I believe Andrew's angle is that if you have fixes that rely on
> non-functional changes, then the fixes cannot be back ported as a
> standalone patch set towards specific stable trees.

Right, which makes it pointless adding a Fixes: tag to those changes.
The amount of change in phylib is quite high at the moment, but
thankfully the bug was introduced after the ethtool linkmode mask
conversion.  It looks like v5.2.x should be able to cope with all
three patches simply applied there.

However, v5.1.x will require a different pre-requisit patch, which
is probably easier to do as you suggest.

> This means that
> people who do care about such fixes may have to come up with a slightly
> different fix for earlier kernels affected by those bugs, such fixes
> would not rely on patch #2 and #3 in this series and open code
> phy_resolve_aneg() and genphy_read_lpa() within the at803x.c PHY driver.
> -- 
> Florian
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up