diff mbox series

[net-next,v3,6/7] net: dsa: mt7530: do not set priv->p5_interface on mt7530_setup_port5()

Message ID 20240122-for-netnext-mt7530-improvements-1-v3-6-042401f2b279@arinc9.com (mailing list archive)
State New, archived
Headers show
Series MT7530 DSA Subdriver Improvements Act I | expand

Commit Message

Arınç ÜNAL via B4 Relay Jan. 22, 2024, 5:35 a.m. UTC 
From: Arınç ÜNAL <arinc.unal@arinc9.com>

Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
of configuring port 5, including phylink.

Setting priv->p5_interface under mt7530_setup_port5() makes sure that
mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.

The commit ("net: dsa: mt7530: improve code path for setting up port 5")
makes so that mt7530_setup_port5() from mt7530_setup() runs only on
non-phylink cases.

Get rid of unnecessarily setting priv->p5_interface under
mt7530_setup_port5() as port 5 phylink configuration will be done by
running mt7530_setup_port5() from mt753x_phylink_mac_config() now.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Arınç ÜNAL Jan. 25, 2024, 10:48 p.m. UTC | #1 
Vladimir could you review this too? This is the only patch remaining
without a review or ACK.

Arınç

On 22.01.2024 08:35, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> of configuring port 5, including phylink.
> 
> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
> 
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
> 
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>   drivers/net/dsa/mt7530.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 33c15f10de34..5394d8c6a40e 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -978,8 +978,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
>   	dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
>   		val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
>   
> -	priv->p5_interface = interface;
> -
>   unlock_exit:
>   	mutex_unlock(&priv->reg_mutex);
>   }
>
Vladimir Oltean Jan. 29, 2024, 12:52 p.m. UTC | #2 
On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> of configuring port 5, including phylink.
> 
> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
> 
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
> 
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

I hope this moves the patch set out of the 'deferred' state.

---
pw-bot: under-review
Arınç ÜNAL Jan. 29, 2024, 4:22 p.m. UTC | #3 
On 29.01.2024 15:52, Vladimir Oltean wrote:
> On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
>> of configuring port 5, including phylink.
>>
>> Setting priv->p5_interface under mt7530_setup_port5() makes sure that
>> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
>>
>> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
>> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
>> non-phylink cases.
>>
>> Get rid of unnecessarily setting priv->p5_interface under
>> mt7530_setup_port5() as port 5 phylink configuration will be done by
>> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
> 
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> 
> I hope this moves the patch set out of the 'deferred' state.
> 
> ---
> pw-bot: under-review

I still see deferred. I guess I'll have to submit this again. :/

Arınç
Vladimir Oltean Jan. 29, 2024, 4:27 p.m. UTC | #4 
On Mon, Jan 29, 2024 at 07:22:28PM +0300, Arınç ÜNAL wrote:
> On 29.01.2024 15:52, Vladimir Oltean wrote:
> > On Mon, Jan 22, 2024 at 08:35:57AM +0300, Arınç ÜNAL via B4 Relay wrote:
> > > From: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > 
> > > Running mt7530_setup_port5() from mt7530_setup() used to handle all cases
> > > of configuring port 5, including phylink.
> > > 
> > > Setting priv->p5_interface under mt7530_setup_port5() makes sure that
> > > mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
> > > 
> > > The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> > > makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> > > non-phylink cases.
> > > 
> > > Get rid of unnecessarily setting priv->p5_interface under
> > > mt7530_setup_port5() as port 5 phylink configuration will be done by
> > > running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
> > > 
> > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > ---
> > 
> > Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> > 
> > I hope this moves the patch set out of the 'deferred' state.
> > 
> > ---
> > pw-bot: under-review
> 
> I still see deferred. I guess I'll have to submit this again. :/
> 
> Arınç

Please wait for a few more hours for one of the networking maintainers
to have a chance to see this and ask you to resend, if necessary.
Jakub Kicinski Jan. 29, 2024, 4:31 p.m. UTC | #5 
On Mon, 29 Jan 2024 19:22:28 +0300 Arınç ÜNAL wrote:
> > I hope this moves the patch set out of the 'deferred' state.
> > 
> > ---
> > pw-bot: under-review  
> 
> I still see deferred. I guess I'll have to submit this again. :/

Took me an hour to fix the mailbot:
https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
email is the most quirky thing ever.
Vladimir Oltean Jan. 29, 2024, 4:52 p.m. UTC | #6 
On Mon, Jan 29, 2024 at 08:31:52AM -0800, Jakub Kicinski wrote:
> On Mon, 29 Jan 2024 19:22:28 +0300 Arınç ÜNAL wrote:
> > > I hope this moves the patch set out of the 'deferred' state.
> > > 
> > > ---
> > > pw-bot: under-review  
> > 
> > I still see deferred. I guess I'll have to submit this again. :/
> 
> Took me an hour to fix the mailbot:
> https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
> email is the most quirky thing ever.

Ah, so it was my neomutt encoding email as base64...

I see Arınç's series is now in the proper state, thanks!
Jakub Kicinski Jan. 29, 2024, 5 p.m. UTC | #7 
On Mon, 29 Jan 2024 18:52:01 +0200 Vladimir Oltean wrote:
> > > I still see deferred. I guess I'll have to submit this again. :/  
> > 
> > Took me an hour to fix the mailbot:
> > https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
> > email is the most quirky thing ever.  
> 
> Ah, so it was my neomutt encoding email as base64...

Something magical going on there, the email is encoded.. twice?
See the attachment. That's already thru a round of base64 decode 
and there's another copy of the email with base64 inside it :o
Anyway, unwrapping it once is good enough for the bot to see the
command, and enough time spent on this ;)

> I see Arınç's series is now in the proper state, thanks!

np!
Arınç ÜNAL Jan. 30, 2024, 2:26 p.m. UTC | #8 
On 29.01.2024 20:00, Jakub Kicinski wrote:
> On Mon, 29 Jan 2024 18:52:01 +0200 Vladimir Oltean wrote:
>>>> I still see deferred. I guess I'll have to submit this again. :/
>>>
>>> Took me an hour to fix the mailbot:
>>> https://github.com/kuba-moo/nipa/commit/6766e97e72ac91ffb42ed2259bc8e2ace446d0ef
>>> email is the most quirky thing ever.
>>
>> Ah, so it was my neomutt encoding email as base64...
> 
> Something magical going on there, the email is encoded.. twice?
> See the attachment. That's already thru a round of base64 decode
> and there's another copy of the email with base64 inside it :o
> Anyway, unwrapping it once is good enough for the bot to see the
> command, and enough time spent on this ;)

I don't claim to be an email expert. I've received Vladimir's email with
the "Content-Transfer-Encoding: 8bit" header. The body was plaintext, not
base64 encoded. I have checked how the netdev mailing list distributed
Vladimir's email, its body is plaintext as well, not base64 encoded. Only
the linux-arm-kernel mailing list distributed the body base64 encoded, the
header is "Content-Transfer-Encoding: base64".

And the attachment you've provided seems to be from the raw output of
lore.kernel.org/all which seems to put together the email distribution from
all mailing lists.

raw from all:

https://lore.kernel.org/all/20240129125241.gu4srgufad6hpwor@skbuf/raw

raw from netdev:

https://lore.kernel.org/netdev/20240129125241.gu4srgufad6hpwor@skbuf/raw

raw from linux-arm-kernel:

https://lore.kernel.org/linux-arm-kernel/20240129125241.gu4srgufad6hpwor@skbuf/raw

I don't know which mailing list mailbot looks at in case of an email is
sent with multiple mailing lists being CC'd or TO'd. It seems to be that it
looked at linux-arm-kernel in this instance.

Arınç
Jakub Kicinski Jan. 30, 2024, 4:02 p.m. UTC | #9 
On Tue, 30 Jan 2024 17:26:29 +0300 Arınç ÜNAL wrote:
> I don't claim to be an email expert. I've received Vladimir's email with
> the "Content-Transfer-Encoding: 8bit" header. The body was plaintext, not
> base64 encoded. I have checked how the netdev mailing list distributed
> Vladimir's email, its body is plaintext as well, not base64 encoded. Only
> the linux-arm-kernel mailing list distributed the body base64 encoded, the
> header is "Content-Transfer-Encoding: base64".
> 
> And the attachment you've provided seems to be from the raw output of
> lore.kernel.org/all which seems to put together the email distribution from
> all mailing lists.
> 
> raw from all:
> 
> https://lore.kernel.org/all/20240129125241.gu4srgufad6hpwor@skbuf/raw
> 
> raw from netdev:
> 
> https://lore.kernel.org/netdev/20240129125241.gu4srgufad6hpwor@skbuf/raw
> 
> raw from linux-arm-kernel:
> 
> https://lore.kernel.org/linux-arm-kernel/20240129125241.gu4srgufad6hpwor@skbuf/raw
> 
> I don't know which mailing list mailbot looks at in case of an email is
> sent with multiple mailing lists being CC'd or TO'd. It seems to be that it
> looked at linux-arm-kernel in this instance.

It's the Python library that base-encodes it for some reason :o

>>>>> $ tail -20 raw.5 
> mt7530_setup_port5() from mt753x_phylink_mac_config() won't run.
> 
> The commit ("net: dsa: mt7530: improve code path for setting up port 5")
> makes so that mt7530_setup_port5() from mt7530_setup() runs only on
> non-phylink cases.
> 
> Get rid of unnecessarily setting priv->p5_interface under
> mt7530_setup_port5() as port 5 phylink configuration will be done by
> running mt7530_setup_port5() from mt753x_phylink_mac_config() now.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

I hope this moves the patch set out of the 'deferred' state.

---
pw-bot: under-review


>>>>> $ cat /tmp/p.py
#!/bin/env python3

import email
from email.policy import default
import sys

with open(sys.argv[1], 'rb') as fp:
    raw = fp.read()

msg = email.message_from_bytes(raw, policy=default)
print(msg.get_body())


>>>>> $ /tmp/p.py  raw.5 | tail -20
 <20240122-for-netnext-mt7530-improvements-1-v3-6-042401f2b279@arinc9.com>

T24gTW9uLCBKYW4gMjIsIDIwMjQgYXQgMDg6MzU6NTdBTSArMDMwMCwgQXLEsW7DpyDDnE5BTCB2
aWEgQjQgUmVsYXkgd3JvdGU6Cj4gRnJvbTogQXLEsW7DpyDDnE5BTCA8YXJpbmMudW5hbEBhcmlu
YzkuY29tPgo+IAo+IFJ1bm5pbmcgbXQ3NTMwX3NldHVwX3BvcnQ1KCkgZnJvbSBtdDc1MzBfc2V0
dXAoKSB1c2VkIHRvIGhhbmRsZSBhbGwgY2FzZXMKPiBvZiBjb25maWd1cmluZyBwb3J0IDUsIGlu
Y2x1ZGluZyBwaHlsaW5rLgo+IAo+IFNldHRpbmcgcHJpdi0+cDVfaW50ZXJmYWNlIHVuZGVyIG10
NzUzMF9zZXR1cF9wb3J0NSgpIG1ha2VzIHN1cmUgdGhhdAo+IG10NzUzMF9zZXR1cF9wb3J0NSgp
IGZyb20gbXQ3NTN4X3BoeWxpbmtfbWFjX2NvbmZpZygpIHdvbid0IHJ1bi4KPiAKPiBUaGUgY29t
bWl0ICgibmV0OiBkc2E6IG10NzUzMDogaW1wcm92ZSBjb2RlIHBhdGggZm9yIHNldHRpbmcgdXAg
cG9ydCA1IikKPiBtYWtlcyBzbyB0aGF0IG10NzUzMF9zZXR1cF9wb3J0NSgpIGZyb20gbXQ3NTMw
X3NldHVwKCkgcnVucyBvbmx5IG9uCj4gbm9uLXBoeWxpbmsgY2FzZXMuCj4gCj4gR2V0IHJpZCBv
ZiB1bm5lY2Vzc2FyaWx5IHNldHRpbmcgcHJpdi0+cDVfaW50ZXJmYWNlIHVuZGVyCj4gbXQ3NTMw
X3NldHVwX3BvcnQ1KCkgYXMgcG9ydCA1IHBoeWxpbmsgY29uZmlndXJhdGlvbiB3aWxsIGJlIGRv
bmUgYnkKPiBydW5uaW5nIG10NzUzMF9zZXR1cF9wb3J0NSgpIGZyb20gbXQ3NTN4X3BoeWxpbmtf
bWFjX2NvbmZpZygpIG5vdy4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBBcsSxbsOnIMOcTkFMIDxhcmlu
Yy51bmFsQGFyaW5jOS5jb20+Cj4gLS0tCgpSZXZpZXdlZC1ieTogVmxhZGltaXIgT2x0ZWFuIDxv
bHRlYW52QGdtYWlsLmNvbT4KCkkgaG9wZSB0aGlzIG1vdmVzIHRoZSBwYXRjaCBzZXQgb3V0IG9m
IHRoZSAnZGVmZXJyZWQnIHN0YXRlLgoKLS0tCnB3LWJvdDogdW5kZXItcmV2aWV3Cgo=
diff mbox series

Patch

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 33c15f10de34..5394d8c6a40e 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -978,8 +978,6 @@  static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 	dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
 		val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
 
-	priv->p5_interface = interface;
-
 unlock_exit:
 	mutex_unlock(&priv->reg_mutex);
 }