| Message ID | 20210913144300.1265143-9-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | RTL8366(RB) cleanups part 1 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 23 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
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.
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?
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!
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;
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(-)