Re: [PATCH] arm64: dts: sun50i: h5: NanoPI Neo 2: phy-mode rgmii-id

[Clément Bœsch]

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@xxxxxxxxx>
> 
> 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.