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

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

[Colin Foster]

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" 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,
 	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
 			   DEV_CLOCK_CFG);
 
-	/* No PFC */
-	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
-			 ANA_PFC_PFC_CFG, port);
-
 	/* Core: Enable port for frame transfer */
 	ocelot_fields_write(ocelot, port,
 			    QSYS_SWITCH_PORT_MODE_PORT_ENA, 1);
-- 
2.25.1

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

[Vladimir Oltean]

On Wed, Sep 15, 2021 at 06:09:37PM -0700, Colin Foster wrote:
> 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" 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,
>  	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
>  			   DEV_CLOCK_CFG);
>  
> -	/* No PFC */
> -	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
> -			 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://patchwork.kernel.org/project/netdevbpf/patch/20210916012341.518512-1-colin.foster@in-advantage.com/

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. If fixing bugs explain the impact.
git send-email \
	--to='netdev@vger.kernel.org' \
	--to='linux-kernel@vger.kernel.org' \
	--cc='Vladimir Oltean <vladimir.oltean@nxp.com>' \
	--cc='Claudiu Manoil <claudiu.manoil@nxp.com>' \
	--cc='Alexandre Belloni <alexandre.belloni@bootlin.com>' \
	--cc='UNGLinuxDriver@microchip.com' \
	--cc='"David S. Miller" <davem@davemloft.net>' \
	--cc='Jakub Kicinski <kuba@kernel.org>' \
	/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://lore.kernel.org/r/%s
[alias]
	b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"

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

[Jakub Kicinski]

On Thu, 16 Sep 2021 11:49:18 +0000 Vladimir Oltean wrote:
> git format-patch -2 --cover-letter 

Nice instructions, let me toss this version from pw.

FWIW the patchwork checks don't complain about 2-patch series without 
a cover letter [1]. Having cover letters is a good rule of thumb but 
I thought I'd mention that 'cause unlikely anyone would realize otherwise.

[1] https://github.com/kuba-moo/nipa/blob/master/tests/series/cover_letter/test.py

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

[Vladimir Oltean]

On Thu, Sep 16, 2021 at 07:52:39AM -0700, Jakub Kicinski wrote:
> On Thu, 16 Sep 2021 11:49:18 +0000 Vladimir Oltean wrote:
> > git format-patch -2 --cover-letter
>
> Nice instructions, let me toss this version from pw.
>
> FWIW the patchwork checks don't complain about 2-patch series without
> a cover letter [1]. Having cover letters is a good rule of thumb but
> I thought I'd mention that 'cause unlikely anyone would realize otherwise.
>
> [1] https://github.com/kuba-moo/nipa/blob/master/tests/series/cover_letter/test.py

In my certainly limited experience I have found out that forcing
yourself to write a change log and a cover letter makes you think more,
which is sadly sometimes needed.

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

[Colin Foster]

On Thu, Sep 16, 2021 at 11:49:18AM +0000, Vladimir Oltean wrote:
> 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://patchwork.kernel.org/project/netdevbpf/patch/20210916012341.518512-1-colin.foster@in-advantage.com/
> 
> Also, why didn't you bump the version counter of the patch, and we're
> still at v1 despite the earlier attempt?

I wasn't sure if changing the names of the patch and the intent is what 
would constitute a new "patch series" so then restart the counter for 
the counters or not. I had figured "two new patches, two new counters"
which I understand now was incorrect.

In this particular case, should I have stuck with my first submission
title:
[PATCH v2 net] bug fix when writing MAC speed
and submitted the two patches? 

I assume it would only cause headaches if I incremented the counter and
changed the name to something like
[PATCH v2 net] remove unnecessary register writes
or something simliar? Although your example below suggests I should
maybe submit the next set as
[PATCH v3 net] ocelot phylink fixes

> 
> 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. If fixing bugs explain the impact.
> git send-email \
> 	--to='netdev@vger.kernel.org' \
> 	--to='linux-kernel@vger.kernel.org' \
> 	--cc='Vladimir Oltean <vladimir.oltean@nxp.com>' \
> 	--cc='Claudiu Manoil <claudiu.manoil@nxp.com>' \
> 	--cc='Alexandre Belloni <alexandre.belloni@bootlin.com>' \
> 	--cc='UNGLinuxDriver@microchip.com' \
> 	--cc='"David S. Miller" <davem@davemloft.net>' \
> 	--cc='Jakub Kicinski <kuba@kernel.org>' \
> 	/opt/patches/linux/ocelot-phylink-fixes/v3/*.patch

I've been using --to-cmd and --cc-cmd with get_maintainer.pl. If this is
ill-advised, I'll stop. I noticed it seemed to determine the list on a
per-patch-file basis instead of generating one single list.

> 
> 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://lore.kernel.org/r/%s
> [alias]
> 	b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"

Thank you for all this. I understand you have better things to do than
to hold my hand through this process, so I greatly appreciate your help.

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

[Joakim Zhang]

Hi Vladimir,

> -----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
> <alexandre.belloni@bootlin.com>; UNGLinuxDriver@microchip.com; David S.
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> 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:
> > 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"
> > 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,
> >  	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
> >  			   DEV_CLOCK_CFG);
> >
> > -	/* No PFC */
> > -	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
> > -			 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%2Fpatchw
> ork.kernel.org%2Fproject%2Fnetdevbpf%2Fpatch%2F20210916012341.518512-
> 1-colin.foster%40in-advantage.com%2F&amp;data=04%7C01%7Cqiangqing.zh
> ang%40nxp.com%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637673897688805938%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C1000&amp;sdata=fmGI6K2dS36tm5xuuKLKdVF1pEj9umv
> 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.
> If fixing bugs explain the impact.
> git send-email \
> 	--to='netdev@vger.kernel.org' \
> 	--to='linux-kernel@vger.kernel.org' \
> 	--cc='Vladimir Oltean <vladimir.oltean@nxp.com>' \
> 	--cc='Claudiu Manoil <claudiu.manoil@nxp.com>' \
> 	--cc='Alexandre Belloni <alexandre.belloni@bootlin.com>' \
> 	--cc='UNGLinuxDriver@microchip.com' \
> 	--cc='"David S. Miller" <davem@davemloft.net>' \
> 	--cc='Jakub Kicinski <kuba@kernel.org>' \
> 	/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%2Flore.ker
> nel.org%2Fr%2F%2525s&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.co
> m%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C637673897688815892%7CUnknown%7CTWFpbGZsb3d
> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C1000&amp;sdata=t8N%2F%2FAnLVLtoMDzNDL%2Fv7ixEkBeiIqB6Go%2F
> 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.

$ git b4 20210916010938.517698-1-colin.foster@in-advantage.com
f() { b4 am -t -o - $@ | git am -3; }; f: 1: f() { b4 am -t -o - $@ | git am -3; }; f: b4: not found

Best Regards,
Joakim Zhang

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

[Colin Foster]

On Fri, Sep 17, 2021 at 02:34:37AM +0000, Joakim Zhang wrote:
> 
> Hi Vladimir,
> 
> > -----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
> > <alexandre.belloni@bootlin.com>; UNGLinuxDriver@microchip.com; David S.
> > Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> > 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:
> > > 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"
> > > 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,
> > >  	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
> > >  			   DEV_CLOCK_CFG);
> > >
> > > -	/* No PFC */
> > > -	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
> > > -			 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%2Fpatchw
> > ork.kernel.org%2Fproject%2Fnetdevbpf%2Fpatch%2F20210916012341.518512-
> > 1-colin.foster%40in-advantage.com%2F&amp;data=04%7C01%7Cqiangqing.zh
> > ang%40nxp.com%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b
> > 4c6fa92cd99c5c301635%7C0%7C0%7C637673897688805938%7CUnknown%7
> > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > CJXVCI6Mn0%3D%7C1000&amp;sdata=fmGI6K2dS36tm5xuuKLKdVF1pEj9umv
> > 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.
> > If fixing bugs explain the impact.
> > git send-email \
> > 	--to='netdev@vger.kernel.org' \
> > 	--to='linux-kernel@vger.kernel.org' \
> > 	--cc='Vladimir Oltean <vladimir.oltean@nxp.com>' \
> > 	--cc='Claudiu Manoil <claudiu.manoil@nxp.com>' \
> > 	--cc='Alexandre Belloni <alexandre.belloni@bootlin.com>' \
> > 	--cc='UNGLinuxDriver@microchip.com' \
> > 	--cc='"David S. Miller" <davem@davemloft.net>' \
> > 	--cc='Jakub Kicinski <kuba@kernel.org>' \
> > 	/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%2Flore.ker
> > nel.org%2Fr%2F%2525s&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.co
> > m%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b4c6fa92cd99c5
> > c301635%7C0%7C0%7C637673897688815892%7CUnknown%7CTWFpbGZsb3d
> > 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C1000&amp;sdata=t8N%2F%2FAnLVLtoMDzNDL%2Fv7ixEkBeiIqB6Go%2F
> > 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.

> 
> $ git b4 20210916010938.517698-1-colin.foster@in-advantage.com
> f() { b4 am -t -o - $@ | git am -3; }; f: 1: f() { b4 am -t -o - $@ | git am -3; }; f: b4: not found
> 
> Best Regards,
> Joakim Zhang

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

[Joakim Zhang]

> -----Original Message-----
> From: Colin Foster <colin.foster@in-advantage.com>
> Sent: 2021年9月17日 11:38
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; UNGLinuxDriver@microchip.com; David S.
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> 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:
> >
> > Hi Vladimir,
> >
> > > -----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
> > > <alexandre.belloni@bootlin.com>; UNGLinuxDriver@microchip.com; David
> S.
> > > Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> > > 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:
> > > > 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"
> > > > 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,
> > > >  	ocelot_port_writel(ocelot_port,
> DEV_CLOCK_CFG_LINK_SPEED(speed),
> > > >  			   DEV_CLOCK_CFG);
> > > >
> > > > -	/* No PFC */
> > > > -	ocelot_write_gix(ocelot,
> ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
> > > > -			 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
> > > -
> > >
> 1-colin.foster%40in-advantage.com%2F&amp;data=04%7C01%7Cqiangqing.zh
> > >
> ang%40nxp.com%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637673897688805938%7CUnknown%7
> > >
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > >
> CJXVCI6Mn0%3D%7C1000&amp;sdata=fmGI6K2dS36tm5xuuKLKdVF1pEj9umv
> > > 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.
> > > If fixing bugs explain the impact.
> > > git send-email \
> > > 	--to='netdev@vger.kernel.org' \
> > > 	--to='linux-kernel@vger.kernel.org' \
> > > 	--cc='Vladimir Oltean <vladimir.oltean@nxp.com>' \
> > > 	--cc='Claudiu Manoil <claudiu.manoil@nxp.com>' \
> > > 	--cc='Alexandre Belloni <alexandre.belloni@bootlin.com>' \
> > > 	--cc='UNGLinuxDriver@microchip.com' \
> > > 	--cc='"David S. Miller" <davem@davemloft.net>' \
> > > 	--cc='Jakub Kicinski <kuba@kernel.org>' \
> > > 	/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
> > >
> a4d1b35bf08d9798c95f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C6
> > >
> 37674466980672633%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIj
> > >
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=WCM%2
> FE6Sy
> > > 6ZXaNq3v7x%2BeIQ%2BX7P7bK2IZFUsBz55l%2BRU%3D&amp;reserved=0
> > >
> nel.org%2Fr%2F%2525s&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.co
> > >
> m%7C546aa03ab17b45f0891a08d97908095f%7C686ea1d3bc2b4c6fa92cd99c5
> > >
> c301635%7C0%7C0%7C637673897688815892%7CUnknown%7CTWFpbGZsb3d
> > >
> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > >
> D%7C1000&amp;sdata=t8N%2F%2FAnLVLtoMDzNDL%2Fv7ixEkBeiIqB6Go%2F
> > > 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

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

[Konstantin Ryabitsev]

On Fri, Sep 17, 2021 at 10:39:18AM +0000, Joakim Zhang wrote:
> 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>
                              ^^^^^^^^^^^
You seem to be trying to run it with python 2.7

>     import email.policy
> ImportError: No module named policy

I'm not sure how you managed to make it install, but it won't work with python
versions < 3.6. Python version 2 is no longer maintained.

-K