diff mbox series

[net-next,8/8] net: dsa: rtl8366: Drop and depromote pointless prints

Message ID 20210913144300.1265143-9-linus.walleij@linaro.org
State Superseded
Headers show
Series RTL8366(RB) cleanups part 1 | expand

Commit Message

Linus Walleij Sept. 13, 2021, 2:43 p.m. UTC
We don't need a message for every VLAN association, dbg
is fine. The message about adding the DSA or CPU
port to a VLAN is directly misleading, this is perfectly
fine.

Cc: Vladimir Oltean <olteanv@gmail.com>
Cc: Mauri Sandberg <sandberg@mailfence.com>
Cc: Alvin Šipraga <alsi@bang-olufsen.dk>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
ChangeLog v1->v4:
- New patch to deal with confusing messages and too talkative
  DSA bridge.
---
 drivers/net/dsa/rtl8366.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

-- 
2.31.1

Comments

Florian Fainelli Sept. 13, 2021, 5:35 p.m. UTC | #1
On 9/13/2021 7:43 AM, Linus Walleij wrote:
> We don't need a message for every VLAN association, dbg

> is fine. The message about adding the DSA or CPU

> port to a VLAN is directly misleading, this is perfectly

> fine.

> 

> Cc: Vladimir Oltean <olteanv@gmail.com>

> Cc: Mauri Sandberg <sandberg@mailfence.com>

> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

> Cc: Florian Fainelli <f.fainelli@gmail.com>

> Cc: DENG Qingfang <dqfext@gmail.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>


Maybe at some point we should think about moving that kind of debugging 
messages towards the DSA core, and just leave drivers with debug prints 
that track an internal state not visible to the DSA framework.
-- 
Florian
Vladimir Oltean Sept. 13, 2021, 5:38 p.m. UTC | #2
On Mon, Sep 13, 2021 at 10:35:53AM -0700, Florian Fainelli wrote:
> On 9/13/2021 7:43 AM, Linus Walleij wrote:

> > We don't need a message for every VLAN association, dbg

> > is fine. The message about adding the DSA or CPU

> > port to a VLAN is directly misleading, this is perfectly

> > fine.

> > 

> > Cc: Vladimir Oltean <olteanv@gmail.com>

> > Cc: Mauri Sandberg <sandberg@mailfence.com>

> > Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

> > Cc: Florian Fainelli <f.fainelli@gmail.com>

> > Cc: DENG Qingfang <dqfext@gmail.com>

> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> 

> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> 

> Maybe at some point we should think about moving that kind of debugging

> messages towards the DSA core, and just leave drivers with debug prints that

> track an internal state not visible to the DSA framework.


I have some trace points on the bridge driver and in DSA for FDB
entries, maybe something along those lines?
Florian Fainelli Sept. 13, 2021, 6:13 p.m. UTC | #3
On 9/13/2021 10:38 AM, Vladimir Oltean wrote:
> On Mon, Sep 13, 2021 at 10:35:53AM -0700, Florian Fainelli wrote:

>> On 9/13/2021 7:43 AM, Linus Walleij wrote:

>>> We don't need a message for every VLAN association, dbg

>>> is fine. The message about adding the DSA or CPU

>>> port to a VLAN is directly misleading, this is perfectly

>>> fine.

>>>

>>> Cc: Vladimir Oltean <olteanv@gmail.com>

>>> Cc: Mauri Sandberg <sandberg@mailfence.com>

>>> Cc: Alvin Šipraga <alsi@bang-olufsen.dk>

>>> Cc: Florian Fainelli <f.fainelli@gmail.com>

>>> Cc: DENG Qingfang <dqfext@gmail.com>

>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

>>

>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

>>

>> Maybe at some point we should think about moving that kind of debugging

>> messages towards the DSA core, and just leave drivers with debug prints that

>> track an internal state not visible to the DSA framework.

> 

> I have some trace points on the bridge driver and in DSA for FDB

> entries, maybe something along those lines?


Yes trace points would work really well, thanks!
-- 
Florian
diff mbox series

Patch

diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index fd725cfa18e7..cf2e9d91d62d 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -325,12 +325,9 @@  int rtl8366_vlan_add(struct dsa_switch *ds, int port,
 		return ret;
 	}
 
-	dev_info(smi->dev, "add VLAN %d on port %d, %s, %s\n",
-		 vlan->vid, port, untagged ? "untagged" : "tagged",
-		 pvid ? " PVID" : "no PVID");
-
-	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
-		dev_err(smi->dev, "port is DSA or CPU port\n");
+	dev_dbg(smi->dev, "add VLAN %d on port %d, %s, %s\n",
+		vlan->vid, port, untagged ? "untagged" : "tagged",
+		pvid ? " PVID" : "no PVID");
 
 	member |= BIT(port);
 
@@ -363,7 +360,7 @@  int rtl8366_vlan_del(struct dsa_switch *ds, int port,
 	struct realtek_smi *smi = ds->priv;
 	int ret, i;
 
-	dev_info(smi->dev, "del VLAN %04x on port %d\n", vlan->vid, port);
+	dev_dbg(smi->dev, "del VLAN %d on port %d\n", vlan->vid, port);
 
 	for (i = 0; i < smi->num_vlan_mc; i++) {
 		struct rtl8366_vlan_mc vlanmc;