LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: "Clément Bœsch" <u@pkh.me>
Cc: Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Willy Liu <willy.liu@realtek.com>,
	Rob Herring <robh+dt@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: Re: [PATCH] arm64: dts: sun50i: h5: NanoPI Neo 2: phy-mode rgmii-id
Date: Tue, 31 Aug 2021 22:17:56 +0200	[thread overview]
Message-ID: <4192454.QcIURlJl1r@kista> (raw)
In-Reply-To: <YS1Mwz+fT9P2qskW@ssq0.pkh.me>

Hi!

Dne ponedeljek, 30. avgust 2021 ob 23:25:23 CEST je Clément Bœsch napisal(a):
> 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?

With that grep command you limited yourself only to those commits which 
reference this particular commit. There are others, which also change "rgmii" 
to "rgmii-id" and reference other commits, like:
544cc3f8573b ("arm64: dts: allwinner: h6: orangepi-one-plus: Fix ethernet")
97a38c1c213b ("arm64: dts: allwinner: beelink-gs1: Enable both RGMII RX/TX 
delay")
(there are more).

Anyway, let's continue this discussion in Andrew's thread.

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

Please wait until discussion reaches a conclusion.

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

That's just for the future reference. No need to do anything now for this 
patch. If you bork any send attempt in the future, recreate same patch(es) 
with this additional tag ("RESEND") and send them again.

> 
> > 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).

I guess you speak for all boards, not just those with Allwinner SoC? I fixed as 
many boards as I have - testing is very desired. On some boards rgmii was 
changed to rgmii-txid, so it's not as straightforward as search and replace. 
It could be also deducted from schematics, but at least one Allwinner board 
has this configured in HW wrong (SW fix was needed for that board) and other 
boards might not have schematics publicy available.

In short, I wouldn't start mass generating patches for this.

Best regards,
Jernej

> 
> -- 
> Clément B.
> 



  reply	other threads:[~2021-08-31 20:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30 15:16 Clément Bœsch
2021-08-30 20:49 ` Jernej Škrabec
2021-08-30 21:25   ` Clément Bœsch
2021-08-31 20:17     ` Jernej Škrabec [this message]
2021-08-30 21:50   ` Andrew Lunn
2021-08-31 19:53     ` Jernej Škrabec
2021-08-31 20:53       ` Andrew Lunn
2021-09-01 15:31         ` Jernej Škrabec
2021-09-03  9:21           ` Maxime Ripard
2021-09-05  0:20   ` [PATCH v2] arm64: dts: allwinner: h5: NanoPI Neo 2: Fix ethernet node Clément Bœsch
2021-09-05  0:42     ` Andrew Lunn
2021-09-06  8:03     ` Maxime Ripard
2021-08-30 21:52 ` [PATCH] arm64: dts: sun50i: h5: NanoPI Neo 2: phy-mode rgmii-id Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4192454.QcIURlJl1r@kista \
    --to=jernej.skrabec@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=mripard@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=u@pkh.me \
    --cc=wens@csie.org \
    --cc=willy.liu@realtek.com \
    --subject='Re: Re: [PATCH] arm64: dts: sun50i: h5: NanoPI Neo 2: phy-mode rgmii-id' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).