From: Daniel Golle <daniel@makrotopia.org>
To: "Arınç ÜNAL" <arinc.unal@arinc9.com>
Cc: netdev@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, linux@armlinux.org.uk,
linux-kernel@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
Sean Wang <sean.wang@mediatek.com>,
Landen Chao <Landen.Chao@mediatek.com>,
DENG Qingfang <dqfext@gmail.com>,
Philipp Zabel <p.zabel@pengutronix.de>,
Sam Shih <Sam.Shih@mediatek.com>,
Lorenzo Bianconi <lorenzo@kernel.org>,
John Crispin <john@phrozen.org>, Felix Fietkau <nbd@nbd.name>,
Luiz Angelo Daros de Luca <luizluca@gmail.com>
Subject: Re: [PATCH net-next 14/15] net: dsa: mt7530: introduce driver for MT7988 built-in switch
Date: Fri, 31 Mar 2023 21:33:50 +0100 [thread overview]
Message-ID: <ZCdDrrynf5fg16VM@makrotopia.org> (raw)
In-Reply-To: <56adf82a-3db0-5909-e948-e21717e3fe03@arinc9.com>
On Fri, Mar 31, 2023 at 11:07:51PM +0300, Arınç ÜNAL wrote:
> On 31.03.2023 16:18, Arınç ÜNAL wrote:
> > On 31.03.2023 15:06, Arınç ÜNAL wrote:
> > > On 31.03.2023 13:16, Daniel Golle wrote:
> > > > On Fri, Mar 31, 2023 at 08:50:28AM +0300, Arınç ÜNAL wrote:
> > > > > On 30.03.2023 18:23, Daniel Golle wrote:
> > > > > > Add driver for the built-in Gigabit Ethernet switch which can be found
> > > > > > in the MediaTek MT7988 SoC.
> > > > > >
> > > > > > The switch shares most of its design with MT7530 and MT7531, but has
> > > > > > it's registers mapped into the SoCs register space rather than being
> > > > > > connected externally or internally via MDIO.
> > > > > >
> > > > > > Introduce a new platform driver to support that.
> > > > > >
> > > > > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > > > > ---
> > > > > > MAINTAINERS | 2 +
> > > > > > drivers/net/dsa/Kconfig | 12 ++++
> > > > > > drivers/net/dsa/Makefile | 1 +
> > > > > > drivers/net/dsa/mt7530-mmio.c | 101
> > > > > > ++++++++++++++++++++++++++++++++++
> > > > > > drivers/net/dsa/mt7530.c | 86 ++++++++++++++++++++++++++++-
> > > > > > drivers/net/dsa/mt7530.h | 12 ++--
> > > > > > 6 files changed, 206 insertions(+), 8 deletions(-)
> > > > > > create mode 100644 drivers/net/dsa/mt7530-mmio.c
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 14924aed15ca7..674673dbdfd8b 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -13174,9 +13174,11 @@ MEDIATEK SWITCH DRIVER
> > > > > > M: Sean Wang <sean.wang@mediatek.com>
> > > > > > M: Landen Chao <Landen.Chao@mediatek.com>
> > > > > > M: DENG Qingfang <dqfext@gmail.com>
> > > > > > +M: Daniel Golle <daniel@makrotopia.org>
> > > > > > L: netdev@vger.kernel.org
> > > > > > S: Maintained
> > > > > > F: drivers/net/dsa/mt7530-mdio.c
> > > > > > +F: drivers/net/dsa/mt7530-mmio.c
> > > > > > F: drivers/net/dsa/mt7530.*
> > > > > > F: net/dsa/tag_mtk.c
> > > > > > diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> > > > > > index c2551b13324c2..de4d86e37973f 100644
> > > > > > --- a/drivers/net/dsa/Kconfig
> > > > > > +++ b/drivers/net/dsa/Kconfig
> > > > > > @@ -52,6 +52,18 @@ config NET_DSA_MT7530
> > > > > > Multi-chip module MT7530 in MT7621AT, MT7621DAT, MT7621ST and
> > > > > > MT7623AI SoCs is supported as well.
> > > > > > +config NET_DSA_MT7988
> > > > > > + tristate "MediaTek MT7988 built-in Ethernet switch support"
> > > > > > + select NET_DSA_MT7530_COMMON
> > > > > > + depends on HAS_IOMEM
> > > > > > + help
> > > > > > + This enables support for the built-in Ethernet switch found
> > > > > > + in the MediaTek MT7988 SoC.
> > > > > > + The switch is a similar design as MT7531, however, unlike
> > > > > > + other MT7530 and MT7531 the switch registers are directly
> > > > > > + mapped into the SoCs register space rather than
> > > > > > being accessible
> > > > > > + via MDIO.
> > > > > > +
> > > > > > config NET_DSA_MV88E6060
> > > > > > tristate "Marvell 88E6060 ethernet switch chip support"
> > > > > > select NET_DSA_TAG_TRAILER
> > > > > > diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
> > > > > > index 71250d7dd41af..103a33e20de4b 100644
> > > > > > --- a/drivers/net/dsa/Makefile
> > > > > > +++ b/drivers/net/dsa/Makefile
> > > > > > @@ -8,6 +8,7 @@ endif
> > > > > > obj-$(CONFIG_NET_DSA_LANTIQ_GSWIP) += lantiq_gswip.o
> > > > > > obj-$(CONFIG_NET_DSA_MT7530_COMMON) += mt7530.o
> > > > > > obj-$(CONFIG_NET_DSA_MT7530) += mt7530-mdio.o
> > > > > > +obj-$(CONFIG_NET_DSA_MT7988) += mt7530-mmio.o
> > > > >
> > > > > I'm not fond of this way. Wouldn't it be better if we split
> > > > > the mdio and
> > > > > mmio drivers to separate modules and kept switch hardware support on
> > > > > mt7530.c?
> > > >
> > > > You mean this in terms of Kconfig symbols?
> > > > Because the way you describe is basically what I'm doing here:
> > > > * mt7530.c is the shared/common switch hardware driver
> > > > * mt7530-mdio.c contains the MDIO accessors and MDIO device
> > > > drivers for
> > > > MT7530, MT7531, MT7621, MT7623, ...
> > > > * mt7530-mmio.c contains the platform device driver for in-SoC
> > > > switches
> > > > which are accessed via MMIO, ie. MT7988 (and yes, this could be
> > > > extended to also support MT7620A/N).
> > >
> > > Ok great.
> > >
> > > >
> > > > In early drafts I also named the Kconfig symbols
> > > > CONFIG_NET_DSA_MT7530 for mt7530.c (ie. the common part)
> > > > CONFIG_NET_DSA_MT7530_MDIO for the MDIO driver
> > > > CONFIG_NET_DSA_MT7530_MMIO for the MMIO driver
> > > >
> > > > However, as existing kernel configurations expect
> > > > CONFIG_NET_DSA_MT7530 to
> > > > select the MDIO driver, I decided it would be better to hide the
> > > > symbol of
> > > > the common part and have CONFIG_NET_DSA_MT7530 select the MDIO
> > > > driver like
> > > > it was before.
> > >
> > > You can "imply NET_DSA_MT7530_MDIO" from NET_DSA_MT7530 so the MDIO
> > > driver is also enabled when NET_DSA_MT7530 is selected. For example,
> > > on Realtek, both MDIO and SMI drivers are enabled by default when
> > > either of the main drivers are selected.
> > >
> > > config NET_DSA_MT7530
> > > tristate "MediaTek MT7530 and MT7531 Ethernet switch support"
> > > select NET_DSA_TAG_MTK
> > > select MEDIATEK_GE_PHY
> > > select PCS_MTK_LYNXI
> > > imply NET_DSA_MT7530_MDIO
> > > imply NET_DSA_MT7530_MMIO
> >
> > The final kconfig should look like this:
> >
> > config NET_DSA_MT7530
> > tristate "MediaTek MT7530 and MT7531 Ethernet switch support"
> > select NET_DSA_TAG_MTK
> > select MEDIATEK_GE_PHY
> > select PCS_MTK_LYNXI
>
> Looks like PCS_MTK_LYNXI is used on NET_DSA_MT7530_MDIO instead now. I also
> see '#include <linux/pcs/pcs-mtk-lynxi.h>' on mt7530.c but don't see any
> functions called on it. Leftover?
Yes, you are right, it's only used in mt7530-mdio.c and the #include in
mt7530.c is a left-over which I shall remove.
>
> > imply NET_DSA_MT7530_MDIO
> > imply NET_DSA_MT7530_MMIO
> > help
> > This enables support for the MediaTek MT7530 and MT7531 Ethernet
> > switch chips. Multi-chip module MT7530 in MT7621AT, MT7621DAT,
> > MT7621ST and MT7623AI SoCs, and built-in switch in MT7688 SoC is
> > supported.
> >
> > config NET_DSA_MT7530_MDIO
> > tristate "MediaTek MT7530 MDIO interface driver"
>
> Should go here:
>
> select PCS_MTK_LYNXI
Yeah, in Kconfig I had taken care of this already, but forgot to remove
the include in mt7530.c...
Thank you for spotting this. I'll soon post v2 which includes these fixes.
next prev parent reply other threads:[~2023-03-31 20:34 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-30 15:19 [PATCH net-next 00/15] net: dsa: add support for MT7988 Daniel Golle
2023-03-30 15:19 ` [PATCH net-next 01/15] net: dsa: mt7530: make some noise if register read fails Daniel Golle
2023-03-30 20:58 ` Andrew Lunn
2023-03-30 15:19 ` [PATCH net-next 02/15] net: dsa: mt7530: refactor SGMII PCS creation Daniel Golle
2023-03-30 20:59 ` Andrew Lunn
2023-03-30 15:20 ` [PATCH net-next 03/15] net: dsa: mt7530: use unlocked regmap accessors Daniel Golle
2023-03-30 21:01 ` Andrew Lunn
2023-03-30 15:20 ` [PATCH net-next 04/15] net: dsa: mt7530: use regmap to access switch register space Daniel Golle
2023-03-30 21:02 ` Andrew Lunn
2023-03-31 2:28 ` Daniel Golle
2023-03-30 15:21 ` [PATCH net-next 05/15] net: dsa: mt7530: move SGMII PCS creation to mt7530_probe function Daniel Golle
2023-03-30 21:02 ` Andrew Lunn
2023-03-30 15:21 ` [PATCH net-next 06/15] net: dsa: mt7530: introduce mutex helpers Daniel Golle
2023-03-30 15:21 ` [PATCH net-next 07/15] net: dsa: mt7530: move p5_intf_modes() function to mt7530.c Daniel Golle
2023-03-30 15:22 ` [PATCH net-next 08/15] net: dsa: mt7530: introduce mt7530_probe_common helper function Daniel Golle
2023-03-30 15:22 ` [PATCH net-next 09/15] net: dsa: mt7530: introduce mt7530_remove_common " Daniel Golle
2023-03-30 15:22 ` [PATCH net-next 10/15] net: dsa: mt7530: split-off common parts from mt7531_setup Daniel Golle
2023-03-30 15:22 ` [PATCH net-next 11/15] net: dsa: mt7530: introduce separate MDIO driver Daniel Golle
2023-03-30 15:23 ` [PATCH net-next 12/15] net: dsa: mt7530: skip locking if MDIO bus isn't present Daniel Golle
2023-03-30 15:23 ` [PATCH net-next 13/15] net: dsa: mt7530: add support for 10G link modes for CPU port Daniel Golle
2023-04-01 8:56 ` Arınç ÜNAL
2023-04-01 13:05 ` Daniel Golle
2023-03-30 15:23 ` [PATCH net-next 14/15] net: dsa: mt7530: introduce driver for MT7988 built-in switch Daniel Golle
2023-03-30 21:03 ` Andrew Lunn
2023-03-31 5:50 ` Arınç ÜNAL
2023-03-31 10:16 ` Daniel Golle
2023-03-31 12:06 ` Arınç ÜNAL
2023-03-31 12:45 ` Andrew Lunn
2023-03-31 13:18 ` Arınç ÜNAL
2023-03-31 13:19 ` Arınç ÜNAL
2023-03-31 14:10 ` Daniel Golle
2023-03-31 14:11 ` Arınç ÜNAL
2023-03-31 20:07 ` Arınç ÜNAL
2023-03-31 20:33 ` Daniel Golle [this message]
2023-04-01 8:05 ` Luiz Angelo Daros de Luca
2023-03-30 15:23 ` [PATCH 15/15] dt-bindings: net: dsa: mediatek,mt7530: add mediatek,mt7988-switch Daniel Golle
2023-03-31 5:27 ` Arınç ÜNAL
2023-04-03 18:23 ` Daniel Golle
2023-03-31 5:27 ` [PATCH net-next 00/15] net: dsa: add support for MT7988 Arınç ÜNAL
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=ZCdDrrynf5fg16VM@makrotopia.org \
--to=daniel@makrotopia.org \
--cc=Landen.Chao@mediatek.com \
--cc=Sam.Shih@mediatek.com \
--cc=andrew@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=arinc.unal@arinc9.com \
--cc=davem@davemloft.net \
--cc=dqfext@gmail.com \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=john@phrozen.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=lorenzo@kernel.org \
--cc=luizluca@gmail.com \
--cc=matthias.bgg@gmail.com \
--cc=nbd@nbd.name \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=p.zabel@pengutronix.de \
--cc=pabeni@redhat.com \
--cc=sean.wang@mediatek.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).