[GIT PULL] Allwinner Fixes for 5.10

Arnd Bergmann arnd at kernel.org
Fri Nov 13 08:44:17 EST 2020


On Fri, Oct 30, 2020 at 11:29 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> On Fri, 30 Oct 2020 at 11:15, Arnd Bergmann <arnd at kernel.org> wrote:
> > On Fri, Oct 30, 2020 at 11:03 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> > > On Fri, 30 Oct 2020 at 10:45, Arnd Bergmann <arnd at kernel.org> wrote:
> > > >
> > > > So I suppose the best workaround we can hope for would be
> > > > to have a list of board compatible strings on which we remove
> > > > any phy-mode="rgmii" properties but leave everything alone?
> > > >
> > > > We shouldn't have different behavior between stable kernels and
> > > > future kernels looking at the same DTB, so I'd hope that would
> > > > work in all combinations.
> > >
> > > I think it would be better to redefine 'rgmii' as 'delay unspecified',
> > > and add 'rgmii-noid' which means that tx/rx delay should be disabled
> > > even when it is set by straps. The reason is that I don't think the
> > > latter will ever be used in practice - TX/RX delay is a h/w
> > > integration choice, and I don't see why you would enable it in
> > > hardware only to disable it again in software. But of course, I'm not
> > > really a networking person :-)
> >
> > I think the meaning of "rgmii" would need to be "phy driver specific
> > delay for backwards compatibility", but I'm not sure how many extra
> > strings need to be defined, possibly three, to mean:
> >
> > a. rgmii, but leave delay unchanged from firmware
> > b. rgmii, but use delay as configured from strapping pins
> > c. rgmii, but turn off internal delay
> >
> > I also wonder if there could be more than one of these in order
> > to work with older kernels that do not understand the new strings.
> > How would existing kernels deal with this?
> >
> >      phy-mode = "rgmii", "rgmii-noid";
> >
> > (note that this would be slightly backwards, using the more generic
> >  string before the more specific one).
> >
>
> I think this is all making it needlessly complicated, especially
> because it is really only needed to fix problems created by a patch
> that was backported without any evidence that it fixes any issues in
> practice.
>
> Basically, the Realtek PHY driver has always ignored the TX/RX delay
> settings given by software, even after the original fix went in for
> v5.2. Therefore, the followup fix fixed a theoretical issue, which
> could never have been the cause of a regression because the first fix
> was broken in the first place.
>
> Adhering strictly to the PHY mode going forward is fine. Backporting a
> change that knowingly breaks platforms, without evidence that it
> actually fixes any is just another symptom of our -stable disease,
> where everything that applies lexically is considered for -stable,
> without regards for what the change actually does.

Sorry for delaying this, I ended up merging the fixes now as it seems
worse to not have it fixed in v5.10.

I actually still disagree with the driver fix even in mainline, we should
really not break existing systems that are deployed with dtb files
that only work by accident. Having the driver work the same way
in stable and in mainline doesn't bother me as long as it doesn't
regress but only warns about cases that are possibly broken.

       Arnd



More information about the linux-arm-kernel mailing list