From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>,
Florian Fainelli <f.fainelli@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
bcm-kernel-feedback-list@broadcom.com, netdev@vger.kernel.org
Subject: Re: [RFC PATCH net-next 3/4] net: systemport: use standard netdevice notifier to detect DSA presence
Date: Fri, 18 Dec 2020 20:08:56 -0800 [thread overview]
Message-ID: <e9f3188d-558c-cb3a-6d5c-17d7d93c5416@gmail.com> (raw)
In-Reply-To: <20201218223852.2717102-4-vladimir.oltean@nxp.com>
On 12/18/2020 2:38 PM, Vladimir Oltean wrote:
> The SYSTEMPORT driver maps each port of the embedded Broadcom DSA switch
> port to a certain queue of the master Ethernet controller. For that it
> currently uses a dedicated notifier infrastructure which was added in
> commit 60724d4bae14 ("net: dsa: Add support for DSA specific notifiers").
>
> However, since commit 2f1e8ea726e9 ("net: dsa: link interfaces with the
> DSA master to get rid of lockdep warnings"), DSA is actually an upper of
> the Broadcom SYSTEMPORT as far as the netdevice adjacency lists are
> concerned. So naturally, the plain NETDEV_CHANGEUPPER net device notifiers
> are emitted. It looks like there is enough API exposed by DSA to the
> outside world already to make the call_dsa_notifiers API redundant. So
> let's convert its only user to plain netdev notifiers.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
The CHANGEUPPER has a slightly different semantic than the current DSA
notifier, and so events that would look like this during
bcm_sysport_init_tx_ring() (good):
[ 6.781064] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=0
[ 6.789214] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=0
[ 6.797337] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=0
[ 6.805464] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=0
[ 6.813583] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=1
[ 6.821701] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=1
[ 6.829819] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=1
[ 6.837944] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=1
[ 6.846063] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=2
[ 6.854183] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=2
[ 6.862303] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=2
[ 6.870425] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=2
[ 6.878544] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=5
[ 6.886663] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=5
[ 6.894783] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=5
[ 6.902906] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=5
now we are getting (bad):
[ 6.678157] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=0
[ 6.686302] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=0
[ 6.694434] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=0
[ 6.702554] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=0
[ 6.710679] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=0
[ 6.718797] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=0
[ 6.726914] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=0
[ 6.735033] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=0
[ 6.743156] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=1
[ 6.751275] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=1
[ 6.759395] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=1
[ 6.767514] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=1
[ 6.775636] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=0,port=1
[ 6.783754] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=1,port=1
[ 6.791874] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=2,port=1
[ 6.799992] brcm-systemport 9300000.ethernet eth0: TDMA cfg,
size=256, switch q=3,port=1
Looking further in bcm_sysport_map_queues() we are getting the following:
6.223042] brcm-systemport 9300000.ethernet eth0: mapping q=0, p=0
[ 6.229369] brcm-systemport 9300000.ethernet eth0: mapping q=1, p=0
[ 6.235659] brcm-systemport 9300000.ethernet eth0: mapping q=2, p=0
[ 6.241945] brcm-systemport 9300000.ethernet eth0: mapping q=3, p=0
[ 6.248232] brcm-systemport 9300000.ethernet eth0: mapping q=4, p=0
[ 6.254519] brcm-systemport 9300000.ethernet eth0: mapping q=5, p=0
[ 6.260805] brcm-systemport 9300000.ethernet eth0: mapping q=6, p=0
[ 6.267092] brcm-systemport 9300000.ethernet eth0: mapping q=7, p=0
which means that the call to netif_set_real_num_tx_queues() that is
executed for the SYSTEMPORT Lite is not taking effect because it is
after the register_netdevice(). Insead of using a CHANGEUPPER notifier,
we can use a REGISTER notifier event and doing that works just fine with
the same semantics as the DSA notifier being removed. This incremental
patch on top of your patch works for me (tm):
https://github.com/ffainelli/linux/commit/f5095ab5c1f31db133d62273928b224674626b75
Thanks!
--
Florian
next prev parent reply other threads:[~2020-12-19 4:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-18 22:38 [RFC PATCH net-next 0/4] Reduce coupling between DSA and Broadcom SYSTEMPORT driver Vladimir Oltean
2020-12-18 22:38 ` [RFC PATCH net-next 1/4] net: dsa: move the Broadcom tag information in a separate header file Vladimir Oltean
2020-12-19 0:20 ` Florian Fainelli
2020-12-18 22:38 ` [RFC PATCH net-next 2/4] net: dsa: export dsa_slave_dev_check Vladimir Oltean
2020-12-19 0:20 ` Florian Fainelli
2020-12-18 22:38 ` [RFC PATCH net-next 3/4] net: systemport: use standard netdevice notifier to detect DSA presence Vladimir Oltean
2020-12-19 0:26 ` Florian Fainelli
2020-12-19 4:08 ` Florian Fainelli [this message]
2020-12-19 12:12 ` Vladimir Oltean
2020-12-21 4:53 ` Florian Fainelli
2020-12-21 22:33 ` Florian Fainelli
2020-12-21 23:06 ` Vladimir Oltean
2020-12-21 23:17 ` Florian Fainelli
2020-12-21 23:41 ` Vladimir Oltean
2020-12-18 22:38 ` [RFC PATCH net-next 4/4] net: dsa: remove the DSA specific notifiers Vladimir Oltean
2020-12-19 0:24 ` Florian Fainelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e9f3188d-558c-cb3a-6d5c-17d7d93c5416@gmail.com \
--to=f.fainelli@gmail.com \
--cc=andrew@lunn.ch \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@gmail.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.