All of lore.kernel.org
 help / color / mirror / Atom feed
From: Diederik de Haas <didi.debian@cknow.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Chen-Yu Tsai" <wens@kernel.org>,
	linux-rockchip@lists.infradead.org,
	"Uwe Kleine-König" <ukleinek@debian.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	"Jonas Karlman" <jonas@kwiboo.se>
Subject: Re: [PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection
Date: Mon, 04 Mar 2024 17:43:08 +0100	[thread overview]
Message-ID: <2662566.GSV3oLgti5@bagend> (raw)
In-Reply-To: <b614dd49-7dbe-452e-b3b5-cb014b30f0f8@lunn.ch>


[-- Attachment #1.1: Type: text/plain, Size: 3372 bytes --]

On Monday, 4 March 2024 16:59:38 CET Andrew Lunn wrote:
> On Mon, Mar 04, 2024 at 04:32:03PM +0100, Diederik de Haas wrote:
> > On Monday, 4 March 2024 14:09:15 CET Andrew Lunn wrote:
> > > > > Andrew already pointed out when I posted the patch introducing the
> > > > > gmac0 node that rgmii-id would be the preferred way to setup things.
> > > > > Back then this didn't happen because this change broke reception of
> > > > > network packets. However this only happend because I didn't have the
> > > > > right phy driver loaded.
> > > > 
> > > > It could be that the PHY is strapped to not use its internal RX delay.
> > > > And the PHY has some weird default TX delay, so having the driver
> > > > put some sensible values in is probably better.
> > > 
> > > It could also be the bootloader putting odd values into the PHY.
> > > 
> > > Anyway, it will work better with the correct PHY, and enable WoL
> > > support.
> > 
> > Not sure if this is the right place or way, but here we go...
> > 
> > A few days ago on #debian-kernel@OFTC:
> > [28.02 16:35] <ukleinek> u-boot should be out of the game
> > [28.02 16:36] <diederik> I'm not so sure anymore. On Quartz64 Model A and
> > B
> > (rk3566) I had massive packet loss and tracked it down to a change in
> > u-boot [28.02 16:37] <ukleinek> diederik: sounds like the Linux network
> > driver on that machine could do something better
> > [28.02 16:38] <diederik> yeah, probably
> > 
> > I reported this about a month ago to Jonas Karlman as I bisected the
> > problem> 
> > to a change in u-boot:
> > > diederik@bagend:~/dev/u-boot/u-boot$ git bisect bad
> > > 25f56459aebced8e4bb7d01061dcb1b765b197e2 is the first bad commit
> > > commit 25f56459aebced8e4bb7d01061dcb1b765b197e2
> > > Author: Jonas Karlman <jonas@kwiboo.se>
> > > Date:   Sun Oct 1 19:17:21 2023 +0000
> > > 
> > >     configs: rockchip: Enable ethernet driver on RK356x boards
> > >     
> > >     Enable DWC_ETH_QOS_ROCKCHIP and related PHY driver on RK356x boards
> > >     that
> > >     have an enabled gmac node.
> > 
> > I just checked and both Quartz64 Model A and B have `phy-mode = "rgmii";`
> > and set `tx_delay` and `rx_delay` to some (other) values.
> > Without knowing nor understanding the details, this seem very much
> > related?
> 
> If you don't have a specific Linux PHY driver, you are at the mercies
> of how the bootloader, or strapping set up the PHY. So it is always
> best to use the correct PHY driver. 

This part is a bit over my head (that's ok, no need to explain it).

> The Linux PHY driver should assume
> nothing and setup the hardware from scratch, removing anything odd the
> bootloader did. However, the fall back generic PHY driver has no chip
> specific knowledge, so it cannot undo whatever the bootloader did.
> 
> So, in an ideal world, we don't care about what the bootloader did,
> just use the correct MAC and PHY driver and it should work. And if it
> does not work, it is a Linux bug, which needs to be found and fixed.

I agree.

> > Not sure if this is the right place or way, but here we go...

That was because it's actually a bug report (wrt Quartz64 A and B), but 
especially your remark made all the pieces I found earlier fall into place.
Therefor I 'abused' this thread/patch to report it.

I'm happy to test patches, but I lack the knowledge to come up with one 
myself.

Cheers,
  Diederik

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Diederik de Haas <didi.debian@cknow.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Chen-Yu Tsai" <wens@kernel.org>,
	linux-rockchip@lists.infradead.org,
	"Uwe Kleine-König" <ukleinek@debian.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	"Jonas Karlman" <jonas@kwiboo.se>
Subject: Re: [PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection
Date: Mon, 04 Mar 2024 17:43:08 +0100	[thread overview]
Message-ID: <2662566.GSV3oLgti5@bagend> (raw)
In-Reply-To: <b614dd49-7dbe-452e-b3b5-cb014b30f0f8@lunn.ch>


[-- Attachment #1.1: Type: text/plain, Size: 3372 bytes --]

On Monday, 4 March 2024 16:59:38 CET Andrew Lunn wrote:
> On Mon, Mar 04, 2024 at 04:32:03PM +0100, Diederik de Haas wrote:
> > On Monday, 4 March 2024 14:09:15 CET Andrew Lunn wrote:
> > > > > Andrew already pointed out when I posted the patch introducing the
> > > > > gmac0 node that rgmii-id would be the preferred way to setup things.
> > > > > Back then this didn't happen because this change broke reception of
> > > > > network packets. However this only happend because I didn't have the
> > > > > right phy driver loaded.
> > > > 
> > > > It could be that the PHY is strapped to not use its internal RX delay.
> > > > And the PHY has some weird default TX delay, so having the driver
> > > > put some sensible values in is probably better.
> > > 
> > > It could also be the bootloader putting odd values into the PHY.
> > > 
> > > Anyway, it will work better with the correct PHY, and enable WoL
> > > support.
> > 
> > Not sure if this is the right place or way, but here we go...
> > 
> > A few days ago on #debian-kernel@OFTC:
> > [28.02 16:35] <ukleinek> u-boot should be out of the game
> > [28.02 16:36] <diederik> I'm not so sure anymore. On Quartz64 Model A and
> > B
> > (rk3566) I had massive packet loss and tracked it down to a change in
> > u-boot [28.02 16:37] <ukleinek> diederik: sounds like the Linux network
> > driver on that machine could do something better
> > [28.02 16:38] <diederik> yeah, probably
> > 
> > I reported this about a month ago to Jonas Karlman as I bisected the
> > problem> 
> > to a change in u-boot:
> > > diederik@bagend:~/dev/u-boot/u-boot$ git bisect bad
> > > 25f56459aebced8e4bb7d01061dcb1b765b197e2 is the first bad commit
> > > commit 25f56459aebced8e4bb7d01061dcb1b765b197e2
> > > Author: Jonas Karlman <jonas@kwiboo.se>
> > > Date:   Sun Oct 1 19:17:21 2023 +0000
> > > 
> > >     configs: rockchip: Enable ethernet driver on RK356x boards
> > >     
> > >     Enable DWC_ETH_QOS_ROCKCHIP and related PHY driver on RK356x boards
> > >     that
> > >     have an enabled gmac node.
> > 
> > I just checked and both Quartz64 Model A and B have `phy-mode = "rgmii";`
> > and set `tx_delay` and `rx_delay` to some (other) values.
> > Without knowing nor understanding the details, this seem very much
> > related?
> 
> If you don't have a specific Linux PHY driver, you are at the mercies
> of how the bootloader, or strapping set up the PHY. So it is always
> best to use the correct PHY driver. 

This part is a bit over my head (that's ok, no need to explain it).

> The Linux PHY driver should assume
> nothing and setup the hardware from scratch, removing anything odd the
> bootloader did. However, the fall back generic PHY driver has no chip
> specific knowledge, so it cannot undo whatever the bootloader did.
> 
> So, in an ideal world, we don't care about what the bootloader did,
> just use the correct MAC and PHY driver and it should work. And if it
> does not work, it is a Linux bug, which needs to be found and fixed.

I agree.

> > Not sure if this is the right place or way, but here we go...

That was because it's actually a bug report (wrt Quartz64 A and B), but 
especially your remark made all the pieces I found earlier fall into place.
Therefor I 'abused' this thread/patch to report it.

I'm happy to test patches, but I lack the knowledge to come up with one 
myself.

Cheers,
  Diederik

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Diederik de Haas <didi.debian@cknow.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Chen-Yu Tsai" <wens@kernel.org>,
	linux-rockchip@lists.infradead.org,
	"Uwe Kleine-König" <ukleinek@debian.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	"Jonas Karlman" <jonas@kwiboo.se>
Subject: Re: [PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection
Date: Mon, 04 Mar 2024 17:43:08 +0100	[thread overview]
Message-ID: <2662566.GSV3oLgti5@bagend> (raw)
In-Reply-To: <b614dd49-7dbe-452e-b3b5-cb014b30f0f8@lunn.ch>

[-- Attachment #1: Type: text/plain, Size: 3372 bytes --]

On Monday, 4 March 2024 16:59:38 CET Andrew Lunn wrote:
> On Mon, Mar 04, 2024 at 04:32:03PM +0100, Diederik de Haas wrote:
> > On Monday, 4 March 2024 14:09:15 CET Andrew Lunn wrote:
> > > > > Andrew already pointed out when I posted the patch introducing the
> > > > > gmac0 node that rgmii-id would be the preferred way to setup things.
> > > > > Back then this didn't happen because this change broke reception of
> > > > > network packets. However this only happend because I didn't have the
> > > > > right phy driver loaded.
> > > > 
> > > > It could be that the PHY is strapped to not use its internal RX delay.
> > > > And the PHY has some weird default TX delay, so having the driver
> > > > put some sensible values in is probably better.
> > > 
> > > It could also be the bootloader putting odd values into the PHY.
> > > 
> > > Anyway, it will work better with the correct PHY, and enable WoL
> > > support.
> > 
> > Not sure if this is the right place or way, but here we go...
> > 
> > A few days ago on #debian-kernel@OFTC:
> > [28.02 16:35] <ukleinek> u-boot should be out of the game
> > [28.02 16:36] <diederik> I'm not so sure anymore. On Quartz64 Model A and
> > B
> > (rk3566) I had massive packet loss and tracked it down to a change in
> > u-boot [28.02 16:37] <ukleinek> diederik: sounds like the Linux network
> > driver on that machine could do something better
> > [28.02 16:38] <diederik> yeah, probably
> > 
> > I reported this about a month ago to Jonas Karlman as I bisected the
> > problem> 
> > to a change in u-boot:
> > > diederik@bagend:~/dev/u-boot/u-boot$ git bisect bad
> > > 25f56459aebced8e4bb7d01061dcb1b765b197e2 is the first bad commit
> > > commit 25f56459aebced8e4bb7d01061dcb1b765b197e2
> > > Author: Jonas Karlman <jonas@kwiboo.se>
> > > Date:   Sun Oct 1 19:17:21 2023 +0000
> > > 
> > >     configs: rockchip: Enable ethernet driver on RK356x boards
> > >     
> > >     Enable DWC_ETH_QOS_ROCKCHIP and related PHY driver on RK356x boards
> > >     that
> > >     have an enabled gmac node.
> > 
> > I just checked and both Quartz64 Model A and B have `phy-mode = "rgmii";`
> > and set `tx_delay` and `rx_delay` to some (other) values.
> > Without knowing nor understanding the details, this seem very much
> > related?
> 
> If you don't have a specific Linux PHY driver, you are at the mercies
> of how the bootloader, or strapping set up the PHY. So it is always
> best to use the correct PHY driver. 

This part is a bit over my head (that's ok, no need to explain it).

> The Linux PHY driver should assume
> nothing and setup the hardware from scratch, removing anything odd the
> bootloader did. However, the fall back generic PHY driver has no chip
> specific knowledge, so it cannot undo whatever the bootloader did.
> 
> So, in an ideal world, we don't care about what the bootloader did,
> just use the correct MAC and PHY driver and it should work. And if it
> does not work, it is a Linux bug, which needs to be found and fixed.

I agree.

> > Not sure if this is the right place or way, but here we go...

That was because it's actually a bug report (wrt Quartz64 A and B), but 
especially your remark made all the pieces I found earlier fall into place.
Therefor I 'abused' this thread/patch to report it.

I'm happy to test patches, but I lack the knowledge to come up with one 
myself.

Cheers,
  Diederik

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2024-03-04 16:43 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04  8:46 [PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection Uwe Kleine-König
2024-03-04  8:46 ` Uwe Kleine-König
2024-03-04  8:46 ` Uwe Kleine-König
2024-03-04  9:07 ` Heiko Stübner
2024-03-04  9:07   ` Heiko Stübner
2024-03-04  9:07   ` Heiko Stübner
2024-03-04  9:15   ` Uwe Kleine-König
2024-03-04  9:15     ` Uwe Kleine-König
2024-03-04  9:15     ` Uwe Kleine-König
2024-03-04  9:24     ` Heiko Stübner
2024-03-04  9:24       ` Heiko Stübner
2024-03-04  9:24       ` Heiko Stübner
2024-03-04 21:03       ` Uwe Kleine-König
2024-03-04 21:03         ` Uwe Kleine-König
2024-03-04 21:03         ` Uwe Kleine-König
2024-03-04 10:07 ` Chen-Yu Tsai
2024-03-04 10:07   ` Chen-Yu Tsai
2024-03-04 10:07   ` Chen-Yu Tsai
2024-03-04 13:09   ` Andrew Lunn
2024-03-04 13:09     ` Andrew Lunn
2024-03-04 13:09     ` Andrew Lunn
2024-03-04 15:32     ` Diederik de Haas
2024-03-04 15:32       ` Diederik de Haas
2024-03-04 15:32       ` Diederik de Haas
2024-03-04 15:46       ` Jonas Karlman
2024-03-04 15:46         ` Jonas Karlman
2024-03-04 15:46         ` Jonas Karlman
2024-03-04 16:47         ` Diederik de Haas
2024-03-04 16:47           ` Diederik de Haas
2024-03-04 16:47           ` Diederik de Haas
2024-08-05 16:33           ` Heiko Stübner
2024-08-05 16:33             ` Heiko Stübner
2024-08-05 16:47             ` Diederik de Haas
2024-08-05 16:47               ` Diederik de Haas
2024-03-04 15:59       ` Andrew Lunn
2024-03-04 15:59         ` Andrew Lunn
2024-03-04 15:59         ` Andrew Lunn
2024-03-04 16:43         ` Diederik de Haas [this message]
2024-03-04 16:43           ` Diederik de Haas
2024-03-04 16:43           ` Diederik de Haas
2024-03-04 22:44           ` Uwe Kleine-König
2024-03-04 22:44             ` Uwe Kleine-König
2024-03-04 22:44             ` Uwe Kleine-König
2024-03-06  0:03             ` Diederik de Haas
2024-03-06  0:03               ` Diederik de Haas
2024-03-06  0:03               ` Diederik de Haas
2024-03-12 18:39               ` Dragan Simic
2024-03-12 18:39                 ` Dragan Simic
2024-03-12 18:39                 ` Dragan Simic
2024-08-05 16:23 ` Heiko Stuebner
2024-08-05 16:23   ` Heiko Stuebner

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=2662566.GSV3oLgti5@bagend \
    --to=didi.debian@cknow.org \
    --cc=andrew@lunn.ch \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=jonas@kwiboo.se \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh@kernel.org \
    --cc=ukleinek@debian.org \
    --cc=wens@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.