Messages in this thread | | | Date | Mon, 30 Aug 2021 23:25:23 +0200 | From | Clément Bœsch <> | Subject | Re: [PATCH] arm64: dts: sun50i: h5: NanoPI Neo 2: phy-mode rgmii-id |
| |
On Mon, Aug 30, 2021 at 10:49:37PM +0200, Jernej Škrabec wrote: > Hi! >
Hi,
> Dne ponedeljek, 30. avgust 2021 ob 17:16:45 CEST je Clément Bœsch napisal(a): > > Since commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay > > config") network is broken on the NanoPi Neo 2. > > > > This patch changes the phy-mode to use internal delays both for RX and > > TX as has been done for other boards affected by the same commit. > > > > Fixes: bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay config") > > This commit fixes DT issue, so "fixes" tag should be: > Fixes: 44a94c7ef989 ("arm64: dts: allwinner: H5: Restore EMAC changes") > > Here, a node with wrong phy-mode property was added to NanoPi Neo 2 board DT.
Shouldn't I add it instead of replacing? I followed what I observed in `git log --grep bbc4d71d63` where all the commits pretty much follow this pattern: that commit is the one causing the actual observed regression, while 44a94c7ef989 is much older, and while it's wrong, it wasn't causing an issue in practice.
Or did I misunderstand something?
> Other than that, this patch is fine and once fixes tag is fixed, you can add: > Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > For next version, you should: > - change fixed tag > - add my review-by tag right above your signed-off-by tag > - mark patch as v2 (add "-v2" parameter to git format-patch) > - describe change right under "---" line >
Will do.
> Note, if you borked something when sending, you should mark patch or patch > series as "RESEND", so recipients don't look for changes in two subsequent e- > mails (--subject-prefix="RESEND PATCH").
Not sure I follow you so before I disturb everyone with more noise I'd just like to confirm: you mean a git send-email in-reply-to=[broken patch attempt] (the one where I borked the --cc), right? But with what patch? I'm a bit confused here.
> Thanks for the fix!
No problem; I really think a scan of all the other boards would be meaningful though, it looks like a lot of them got fixed but there are many other candidates and the issue feels pretty critical to me (regression, and no network at all).
-- Clément B.
| |