RE: [PATCH v1 net] net: mscc: ocelot: remove buggy and useless write to ANA_PFC_PFC_CFG

From: Joakim Zhang <hidden>
Date: 2021-09-17 10:39:24
Also in: lkml

-----Original Message-----
From: Colin Foster <colin.foster@in-advantage.com>
Sent: 2021年9月17日 11:38
To: Joakim Zhang <redacted>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Claudiu Manoil
[off-list ref]; Alexandre Belloni
[off-list ref]; UNGLinuxDriver@microchip.com; David S.
Miller [off-list ref]; Jakub Kicinski [off-list ref];
netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 net] net: mscc: ocelot: remove buggy and useless write
to ANA_PFC_PFC_CFG

On Fri, Sep 17, 2021 at 02:34:37AM +0000, Joakim Zhang wrote:

quoted

Hi Vladimir,

quoted

-----Original Message-----
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Sent: 2021年9月16日 19:49
To: Colin Foster <colin.foster@in-advantage.com>
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Alexandre Belloni
[off-list ref]; UNGLinuxDriver@microchip.com; David

S.

quoted

Miller [off-list ref]; Jakub Kicinski [off-list ref];
netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 net] net: mscc: ocelot: remove buggy and
useless write to ANA_PFC_PFC_CFG

On Wed, Sep 15, 2021 at 06:09:37PM -0700, Colin Foster wrote:

quoted

A useless write to ANA_PFC_PFC_CFG was left in while refactoring
ocelot to phylink. Since priority flow control is disabled,
writing the speed has no effect.

Further, it was using ethtool.h SPEED_ instead of OCELOT_SPEED_
macros, which are incorrectly offset for GENMASK.

Lastly, for priority flow control to properly function, some
scenarios would rely on the rate adaptation from the PCS while the
MAC speed would be fixed. So it isn't used, and even if it was, neither

"speed"

quoted

nor "mac_speed" are necessarily the correct values to be used.

Fixes: e6e12df625f2 ("net: mscc: ocelot: convert to phylink")
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c

b/drivers/net/ethernet/mscc/ocelot.c
index c581b955efb3..08be0440af28 100644

--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c

@@ -569,10 +569,6 @@ void ocelot_phylink_mac_link_up(struct ocelot

*ocelot, int port,

quoted

 	ocelot_port_writel(ocelot_port,

DEV_CLOCK_CFG_LINK_SPEED(speed),

quoted

 			   DEV_CLOCK_CFG);

-	/* No PFC */
-	ocelot_write_gix(ocelot,

ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),

quoted

-			 ANA_PFC_PFC_CFG, port);
-

This will conflict with the other patch.... why didn't you send both
as part of a series? By not doing that, you are telling patchwork to
build-test them in parallel, which of course does not work:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
tchw

ork.kernel.org%2Fproject%2Fnetdevbpf%2Fpatch%2F20210916012341.518512

quoted

1-colin.foster%40in-advantage.com%2F&amp;data=04%7C01%7Cqiangqing.zh

quoted

ang%40nxp.com%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b

quoted

4c6fa92cd99c5c301635%7C0%7C0%7C637673897688805938%7CUnknown%7

quoted

CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL

quoted

CJXVCI6Mn0%3D%7C1000&amp;sdata=fmGI6K2dS36tm5xuuKLKdVF1pEj9umv

quoted

FLA8kyfXWD3A%3D&amp;reserved=0

Also, why didn't you bump the version counter of the patch, and
we're still at v1 despite the earlier attempt?

git format-patch -2 --cover-letter --subject-prefix="PATCH v3 net"
-o /opt/patches/linux/ocelot-phylink-fixes/v3/
./scripts/get_maintainer.pl
/opt/patches/linux/ocelot-phylink-fixes/v3/*.patch
./scripts/checkpatch.pl --strict
/opt/patches/linux/ocelot-phylink-fixes/v3/*.patch
# Go through patches, write change log compared to v2 using vimdiff,
meld, git range-diff, whatever # Write cover letter summarizing what

changes and why.

quoted

If fixing bugs explain the impact.
git send-email \
	--to='netdev@vger.kernel.org' \
	--to='linux-kernel@vger.kernel.org' \
	--cc='Vladimir Oltean [off-list ref]' \
	--cc='Claudiu Manoil [off-list ref]' \
	--cc='Alexandre Belloni [off-list ref]' \
	--cc='UNGLinuxDriver@microchip.com' \
	--cc='"David S. Miller" [off-list ref]' \
	--cc='Jakub Kicinski [off-list ref]' \
	/opt/patches/linux/ocelot-phylink-fixes/v3/*.patch

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Please keep this tag but resend a new version. You can download the
patch with the review tags automatically using:
git b4 20210916010938.517698-1-colin.foster@in-advantage.com
git b4 20210916012341.518512-1-colin.foster@in-advantage.com

where "git b4" is an alias configured like this in ~/.gitconfig:

[b4]
	midmask =
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo

re.ker%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf6f8b45f
5e4

quoted

a4d1b35bf08d9798c95f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
%7C6

quoted

37674466980672633%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
AiLCJQIj

quoted

oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=WCM%2
FE6Sy

quoted

6ZXaNq3v7x%2BeIQ%2BX7P7bK2IZFUsBz55l%2BRU%3D&amp;reserved=0

nel.org%2Fr%2F%2525s&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.co

quoted

m%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b4c6fa92cd99c5

quoted

c301635%7C0%7C0%7C637673897688815892%7CUnknown%7CTWFpbGZsb3d

quoted

8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3

quoted

D%7C1000&amp;sdata=t8N%2F%2FAnLVLtoMDzNDL%2Fv7ixEkBeiIqB6Go%2F

quoted

zD19gisE%3D&amp;reserved=0
[alias]
	b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"

I came across this detailed suggestions, sometime we need download the
patch from the patchwork, so I have a try with above method(adding
these two symbol in my .gitconfig), but I met below error, could you please

tell me what I am missing? Thanks.

One that I can answer.

b4 is a Python command.
"pip install b4" should install it, then export /home/username/.local/bin into
PATH "export PATH=/home/colin/.local/bin:$PATH"

You can add this path to ~/.profile if you want it to persist.

Thanks Colin,

But it still failed at my side, after I google, have not found a solution, could you please
help have a look about below error?

$ git b4 20210916010938.517698-1-colin.foster@in-advantage.com
Traceback (most recent call last):
  File "/home/zqq/.local/bin/b4", line 7, in <module>
    from b4.command import cmd
  File "/home/zqq/.local/lib/python2.7/site-packages/b4/__init__.py", line 11, in <module>
    import email.policy
ImportError: No module named policy

Best Regards,
Joakim Zhang

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help