| Message ID | 20190821144547.15113-1-opensource@vdorst.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: dsa: mt7530: Convert to PHYLINK and add support for port 5 | expand |
On Wed, Aug 21, 2019 at 04:45:44PM +0200, René van Dorst wrote: > 1. net: dsa: mt7530: Convert to PHYLINK API > This patch converts mt7530 to PHYLINK API. > 2. dt-bindings: net: dsa: mt7530: Add support for port 5 > 3. net: dsa: mt7530: Add support for port 5 > These 2 patches adding support for port 5 of the switch. > > v1->v2: > * Mostly phylink improvements after review. Hi René You are addressing comments mostly from Russell King. It would of been good to Cc: him on the patchset. Andrew
Hi, tested on BPI-R2 (mt7623) with 2 Problems (already reported to Rene, just to inform everyone)...maybe anybody has an idea - linux-next (i know it's not part of the series, but a pitfall on testing other devices) seems to break power-regulator somewhere here: priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core"); returns 517 #define EPROBE_DEFER517/* Driver requests probe retry */ https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L1726 without linux-next switch came up including dsa-ports - RX-traffic (run iperf3 -c x.x.x.x -R) is only 780 Mbits/sec (TX=940 Mbits/sec), same measure with 5.3-rc4 gives 940 MBit/s with same devices, maybe caused by changes for mt76x8? regards Frank
tested now also on bpi-r64 (mt7622) v0.1 (rtl8367 switch), without linux-next to avoid power-regulator-problems like on bpi-r2 dmesg without warnings/errors caused by this patches link came up as desired iperf3 looks good: 943 Mbits/sec in both directions and no other issues so it is currently only the rx-throughput-problem on mt7623/bpi-r2 regards Frank
From: René van Dorst <opensource@vdorst.com> Date: Wed, 21 Aug 2019 16:45:44 +0200 > 1. net: dsa: mt7530: Convert to PHYLINK API > This patch converts mt7530 to PHYLINK API. > 2. dt-bindings: net: dsa: mt7530: Add support for port 5 > 3. net: dsa: mt7530: Add support for port 5 > These 2 patches adding support for port 5 of the switch. > > v1->v2: > * Mostly phylink improvements after review. > rfc -> v1: > * Mostly phylink improvements after review. > * Drop phy isolation patches. Adds no value for now. This definitely needs some review before I'll apply it. Thanks.
On Thu, Aug 22, 2019 at 04:20:47PM -0700, David Miller wrote: > From: René van Dorst <opensource@vdorst.com> > Date: Wed, 21 Aug 2019 16:45:44 +0200 > > > 1. net: dsa: mt7530: Convert to PHYLINK API > > This patch converts mt7530 to PHYLINK API. > > 2. dt-bindings: net: dsa: mt7530: Add support for port 5 > > 3. net: dsa: mt7530: Add support for port 5 > > These 2 patches adding support for port 5 of the switch. > > > > v1->v2: > > * Mostly phylink improvements after review. > > rfc -> v1: > > * Mostly phylink improvements after review. > > * Drop phy isolation patches. Adds no value for now. > > This definitely needs some review before I'll apply it. That would be Russell. We should try to improve MAINTAINER so that Russell King gets picked by the get_maintainer script. Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: Fri, 23 Aug 2019 03:09:28 +0200 > That would be Russell. > > We should try to improve MAINTAINER so that Russell King gets picked > by the get_maintainer script. Shoule he be added to the mt7530 entry?
65;5402;1cOn Sat, Aug 24, 2019 at 02:18:03PM -0700, David Miller wrote: > From: Andrew Lunn <andrew@lunn.ch> > Date: Fri, 23 Aug 2019 03:09:28 +0200 > > > That would be Russell. > > > > We should try to improve MAINTAINER so that Russell King gets picked > > by the get_maintainer script. > > Shoule he be added to the mt7530 entry? Hi David No. I think we need a phylink entry. And then make use of the K: line format to list keywords. I hope that even though changes like this don't touch any files listed as being part of phylink, they will match the keyword and pickup Russell. I need to do some testing and see if this actually works. Andrew
On Sat, Aug 24, 2019 at 02:18:03PM -0700, David Miller wrote: > From: Andrew Lunn <andrew@lunn.ch> > Date: Fri, 23 Aug 2019 03:09:28 +0200 > > > That would be Russell. > > > > We should try to improve MAINTAINER so that Russell King gets picked > > by the get_maintainer script. > > Shoule he be added to the mt7530 entry? Probably some way to make MAINTAINERS pick up on phylink-containing patches. Something like: K: phylink maybe?
On Wed, Aug 21, 2019 at 04:45:44PM +0200, René van Dorst wrote: > 1. net: dsa: mt7530: Convert to PHYLINK API > This patch converts mt7530 to PHYLINK API. > 2. dt-bindings: net: dsa: mt7530: Add support for port 5 > 3. net: dsa: mt7530: Add support for port 5 > These 2 patches adding support for port 5 of the switch. > > v1->v2: > * Mostly phylink improvements after review. > rfc -> v1: > * Mostly phylink improvements after review. > * Drop phy isolation patches. Adds no value for now. > René van Dorst (3): > net: dsa: mt7530: Convert to PHYLINK API > dt-bindings: net: dsa: mt7530: Add support for port 5 > net: dsa: mt7530: Add support for port 5 > > .../devicetree/bindings/net/dsa/mt7530.txt | 218 ++++++++++ > drivers/net/dsa/mt7530.c | 371 +++++++++++++++--- > drivers/net/dsa/mt7530.h | 61 ++- > 3 files changed, 577 insertions(+), 73 deletions(-) Having looked through this set of patches, I don't see anything from the phylink point of view that concerns me. So, for the series from the phylink perspective: Acked-by: Russell King <rmk+kernel@armlinux.org.uk> Thanks. I did notice a dev_info() in patch 3 that you may like to consider whether they should be printed at info level or debug level. You may keep my ack on the patch when fixing that. I haven't considered whether the patch passes davem's style requirements for networking code; what I spotted did look like the declarations were upside-down christmas tree.
On Sun, Aug 25, 2019 at 12:15:19AM +0200, Andrew Lunn wrote: > 65;5402;1cOn Sat, Aug 24, 2019 at 02:18:03PM -0700, David Miller wrote: > > From: Andrew Lunn <andrew@lunn.ch> > > Date: Fri, 23 Aug 2019 03:09:28 +0200 > > > > > That would be Russell. > > > > > > We should try to improve MAINTAINER so that Russell King gets picked > > > by the get_maintainer script. > > > > Shoule he be added to the mt7530 entry? > > Hi David > > No. I think we need a phylink entry. And then make use of the K: line > format to list keywords. I hope that even though changes like this > don't touch any files listed as being part of phylink, they will match > the keyword and pickup Russell. Note that phylink itself is already covered by "SFF/SFP/SFP+ MODULE SUPPORT" but doesn't pick up on keywords.
Hi Russell, Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>: > On Wed, Aug 21, 2019 at 04:45:44PM +0200, René van Dorst wrote: >> 1. net: dsa: mt7530: Convert to PHYLINK API >> This patch converts mt7530 to PHYLINK API. >> 2. dt-bindings: net: dsa: mt7530: Add support for port 5 >> 3. net: dsa: mt7530: Add support for port 5 >> These 2 patches adding support for port 5 of the switch. >> >> v1->v2: >> * Mostly phylink improvements after review. >> rfc -> v1: >> * Mostly phylink improvements after review. >> * Drop phy isolation patches. Adds no value for now. >> René van Dorst (3): >> net: dsa: mt7530: Convert to PHYLINK API >> dt-bindings: net: dsa: mt7530: Add support for port 5 >> net: dsa: mt7530: Add support for port 5 >> >> .../devicetree/bindings/net/dsa/mt7530.txt | 218 ++++++++++ >> drivers/net/dsa/mt7530.c | 371 +++++++++++++++--- >> drivers/net/dsa/mt7530.h | 61 ++- >> 3 files changed, 577 insertions(+), 73 deletions(-) > > Having looked through this set of patches, I don't see anything > from the phylink point of view that concerns me. So, for the > series from the phylink perspective: > > Acked-by: Russell King <rmk+kernel@armlinux.org.uk> Thanks and thanks for reviewing. Greats, René > > Thanks. > > I did notice a dev_info() in patch 3 that you may like to consider > whether they should be printed at info level or debug level. You > may keep my ack on the patch when fixing that. > > I haven't considered whether the patch passes davem's style > requirements for networking code; what I spotted did look like > the declarations were upside-down christmas tree. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up > According to speedtest.net: 11.9Mbps down 500kbps up