diff mbox series

[net-next,14/15] net: dsa: mt7530: introduce driver for MT7988 built-in switch

Message ID fef2cb2fe3d2b70fa46e93107a0c862f53bb3bfa.1680180959.git.daniel@makrotopia.org
State New
Headers
Series net: dsa: add support for MT7988 |

Commit Message

Daniel Golle March 30, 2023, 3:23 p.m. UTC 
  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

Comments

Andrew Lunn March 30, 2023, 9:03 p.m. UTC | #1 
On Thu, Mar 30, 2023 at 04:23:42PM +0100, 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>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Arınç ÜNAL March 31, 2023, 5:50 a.m. UTC | #2 
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?

The mmio driver could be useful in the future for the MT7530 on the 
MT7620 SoCs or generally new hardware that would use MMIO to be controlled.

Luiz did this for the Realtek switches that use MDIO and SMI to be 
controlled.

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Kconfig

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Makefile

Arınç
Daniel Golle March 31, 2023, 10:16 a.m. UTC | #3 
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).

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.

Hence I decided to go with
CONFIG_NET_DSA_MT7530 selects the MDIO driver, just like before
CONFIG_NET_DSA_MT7988 selects the new MMIO driver
CONFIG_NET_DSA_MT7530_COMMON is hidden, selected by both of the above

> 
> The mmio driver could be useful in the future for the MT7530 on the MT7620
> SoCs or generally new hardware that would use MMIO to be controlled.
> 

Sure, it would be a bit confusing once we add support for MT7620A/N (if
that ever happens...), then CONFIG_NET_DSA_MT7988 would need to be
selected to support this ancient MIPS SoC...

If you are planning to work on support for MT7620A/N feel free to suggest
a better way to name the Kconfig symbols.

> Luiz did this for the Realtek switches that use MDIO and SMI to be
> controlled.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Kconfig
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Makefile

Are you suggesting to split-off a device-specific driver which would
then select the access-method driver (MDIO vs. MMIO) and the 
common/shared driver? To me this looks like overkill for MT7530, given
that the designs of all MT7530 are pretty similar, ie. same tag format
and also otherwise very similar.


Thank you for reviewing!


Daniel
Arınç ÜNAL March 31, 2023, 12:06 p.m. UTC | #4 
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

> 
> Hence I decided to go with
> CONFIG_NET_DSA_MT7530 selects the MDIO driver, just like before
> CONFIG_NET_DSA_MT7988 selects the new MMIO driver
> CONFIG_NET_DSA_MT7530_COMMON is hidden, selected by both of the above
> 
>>
>> The mmio driver could be useful in the future for the MT7530 on the MT7620
>> SoCs or generally new hardware that would use MMIO to be controlled.
>>
> 
> Sure, it would be a bit confusing once we add support for MT7620A/N (if
> that ever happens...), then CONFIG_NET_DSA_MT7988 would need to be
> selected to support this ancient MIPS SoC...
> 
> If you are planning to work on support for MT7620A/N feel free to suggest
> a better way to name the Kconfig symbols.

I don't plan to but Luiz may. Onto my suggestions.

Firstly, all of the functions on the mt7530-mmio driver should be 
changed from mt7988_* to mt7530_mmio_*. Same goes for the mt7530-mdio 
driver too as some of the functions don't start with mt7530_mdio_*. The 
MDIO and MMIO drivers are supposed to be used for the switches the 
MT7530 DSA driver supports. The mt7530_ prefix is derived from that. The 
mmio_ or mdio_ prefix is derived from, well, the driver itself.

You're calling the .name of the MMIO driver, mt7988-switch; the MDIO 
driver mt7530. That doesn't make sense. They should be mt7530-mmio and 
mt7530-mdio (or mediatek-mmio and mediatek-mdio).

What I'm going to say next depends on how generic the MMIO and MDIO 
drivers are so that they can be used on all MediaTek architecture 
switches. Let's say, a new MediaTek switch is introduced. It seems 
likely that either the MMIO or MDIO driver will be used to control the 
switch. Maybe the driver for this new switch won't be under mt7530.c, 
like on Realtek, but that doesn't change the outcome.

You know the MMIO and MDIO drivers better than I do, so if this makes 
sense to you, I'd rather call the MDIO and MMIO drivers MediaTek MDIO 
and MediaTek MMIO, and change the code accordingly. E.g. mt7988_* to 
mediatek_mmio_*, the filename from mt7530-mmio.c to mediatek-mmio.c, 
kernel config option from NET_DSA_MT7530_MMIO to NET_DSA_MEDIATEK_MMIO. 
This is currently the case with the Realtek MDIO and SMI drivers.

If not, call it MediaTek MT7530 MMIO and MediaTek MT7530 MDIO.

> 
>> Luiz did this for the Realtek switches that use MDIO and SMI to be
>> controlled.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Kconfig
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/realtek/Makefile
> 
> Are you suggesting to split-off a device-specific driver which would
> then select the access-method driver (MDIO vs. MMIO) and the
> common/shared driver? To me this looks like overkill for MT7530, given
> that the designs of all MT7530 are pretty similar, ie. same tag format
> and also otherwise very similar.

No, and agreed. I just wanted to show the Realtek MDIO and SMI drivers' 
kconfig as an example.

> 
> 
> Thank you for reviewing!

Great work so far!

Arınç
Andrew Lunn March 31, 2023, 12:45 p.m. UTC | #5 
> Firstly, all of the functions on the mt7530-mmio driver should be changed
> from mt7988_* to mt7530_mmio_*. Same goes for the mt7530-mdio driver too as
> some of the functions don't start with mt7530_mdio_*. The MDIO and MMIO
> drivers are supposed to be used for the switches the MT7530 DSA driver
> supports. The mt7530_ prefix is derived from that. The mmio_ or mdio_ prefix
> is derived from, well, the driver itself.

There are examples of similar naming schemes in other DSA drivers. For
the marvell mv88e6xxx driver, all generic functions use the mv88e6xxx_
prefix. For functions which are specific to a family of marvell
switches, we use a prefix for when the feature was introduced. So for
example we have mv88e6352_g1_reset(), where that method of resetting
the devices was introduced in the mv88e6352. This also gives us some
namespace space, so we can also have mv88e6185_g1_reset() which is
used for a different family.

So i personally don't have a problem using different prefixes within
one driver, if it helps with understanding and name space issues.

> What I'm going to say next depends on how generic the MMIO and MDIO drivers
> are so that they can be used on all MediaTek architecture switches. Let's
> say, a new MediaTek switch is introduced. It seems likely that either the
> MMIO or MDIO driver will be used to control the switch. Maybe the driver for
> this new switch won't be under mt7530.c, like on Realtek, but that doesn't
> change the outcome.

My experience with silicon vendors is that they like to change the
hardware in none backwards compatible ways. So i would actually avoid
generic names, it makes it harder to deal with different variants.

	Andrew
Arınç ÜNAL March 31, 2023, 1:18 p.m. UTC | #6 
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
	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"
	default NET_DSA_MT7530
	depends on NET_DSA_MT7530
	help
	  This enables support for the MediaTek MT7530 switch chips which are
	  connected via MDIO.

config NET_DSA_MT7530_MMIO
	tristate "MediaTek MT7530 MMIO interface driver"
	depends on HAS_IOMEM
	depends on NET_DSA_MT7530
	help
	  This enables support for the MediaTek MT7530 switch chips which are
	  connected via MMIO.

You should also change MODULE_DESCRIPTION for both drivers to something
like "Driver for MediaTek MT7530 ethernet switches connected via MMIO
interface".

Arınç
Arınç ÜNAL March 31, 2023, 1:19 p.m. UTC | #7 
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
>      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"
>      default NET_DSA_MT7530

This is unnecessary.

Arınç
Daniel Golle March 31, 2023, 2:10 p.m. UTC | #8 
On Fri, Mar 31, 2023 at 04:18:19PM +0300, 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
> 	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
                                                             ^^^^^^
You probably meant MT7988.

The built-in Fast Ethernet switch of older Ralink SoCs as well as MT7628 and
MT7688 is a whole different story:
https://github.com/stroese/linux/blob/gardena-v5.5/drivers/net/dsa/mt7628-esw.c

> 	  supported.
> 
> config NET_DSA_MT7530_MDIO
> 	tristate "MediaTek MT7530 MDIO interface driver"
> 	default NET_DSA_MT7530
> 	depends on NET_DSA_MT7530
> 	help
> 	  This enables support for the MediaTek MT7530 switch chips which are
> 	  connected via MDIO.
> 
> config NET_DSA_MT7530_MMIO
> 	tristate "MediaTek MT7530 MMIO interface driver"
> 	depends on HAS_IOMEM
> 	depends on NET_DSA_MT7530
> 	help
> 	  This enables support for the MediaTek MT7530 switch chips which are
> 	  connected via MMIO.
> 
> You should also change MODULE_DESCRIPTION for both drivers to something
> like "Driver for MediaTek MT7530 ethernet switches connected via MMIO
> interface".

Ack. Will do.
Arınç ÜNAL March 31, 2023, 2:11 p.m. UTC | #9 
On 31.03.2023 17:10, Daniel Golle wrote:
> On Fri, Mar 31, 2023 at 04:18:19PM +0300, 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
>> 	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
>                                                               ^^^^^^
> You probably meant MT7988.
> 
> The built-in Fast Ethernet switch of older Ralink SoCs as well as MT7628 and
> MT7688 is a whole different story:
> https://github.com/stroese/linux/blob/gardena-v5.5/drivers/net/dsa/mt7628-esw.c

Yup, typo. "is" at the end should also be "are".

Arınç
Arınç ÜNAL March 31, 2023, 8:07 p.m. UTC | #10 
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?

>      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

Arınç
Daniel Golle March 31, 2023, 8:33 p.m. UTC | #11 
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.
Luiz Angelo Daros de Luca April 1, 2023, 8:05 a.m. UTC | #12 
> >> The mmio driver could be useful in the future for the MT7530 on the MT7620
> >> SoCs or generally new hardware that would use MMIO to be controlled.
> >>
> >
> > Sure, it would be a bit confusing once we add support for MT7620A/N (if
> > that ever happens...), then CONFIG_NET_DSA_MT7988 would need to be
> > selected to support this ancient MIPS SoC...
> >
> > If you are planning to work on support for MT7620A/N feel free to suggest
> > a better way to name the Kconfig symbols.
>
> I don't plan to but Luiz may. Onto my suggestions.

I did start a branch to bring mt7620 ethernet+dsa upstream, but since
I burned my test device's serial port, my mt7620 and rtl8367s dev days
are over. After me, I wouldn't bet someone else will invest the
required amount of time that mt7620 needs just to support an outdated
SoC. Its internal MT7530 still only supports FastEthernet, so to have
GbE, you need to add a second switch connected to its single Gigabit
RGMII port. If you need GbE, you can just use an mt7621.

Regards,

Luiz
diff mbox series

Patch

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
 obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
 obj-$(CONFIG_NET_DSA_RZN1_A5PSW) += rzn1_a5psw.o
 obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
new file mode 100644
index 0000000000000..41ebaeb551e1e
--- /dev/null
+++ b/drivers/net/dsa/mt7530-mmio.c
@@ -0,0 +1,101 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <net/dsa.h>
+
+#include "mt7530.h"
+
+static const struct of_device_id mt7988_of_match[] = {
+	{ .compatible = "mediatek,mt7988-switch", .data = &mt753x_table[ID_MT7988], },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mt7988_of_match);
+
+static int
+mt7988_probe(struct platform_device *pdev)
+{
+	static struct regmap_config *sw_regmap_config;
+	struct mt7530_priv *priv;
+	void __iomem *base_addr;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->bus = NULL;
+	priv->dev = &pdev->dev;
+
+	ret = mt7530_probe_common(priv);
+	if (ret)
+		return ret;
+
+	priv->rstc = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->rstc)) {
+		dev_err(&pdev->dev, "Couldn't get our reset line\n");
+		return PTR_ERR(priv->rstc);
+	}
+
+	base_addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base_addr)) {
+		dev_err(&pdev->dev, "cannot request I/O memory space\n");
+		return -ENXIO;
+	}
+
+	sw_regmap_config = devm_kzalloc(&pdev->dev, sizeof(*sw_regmap_config), GFP_KERNEL);
+	if (!sw_regmap_config)
+		return -ENOMEM;
+
+	sw_regmap_config->name = "switch";
+	sw_regmap_config->reg_bits = 16;
+	sw_regmap_config->val_bits = 32;
+	sw_regmap_config->reg_stride = 4;
+	sw_regmap_config->max_register = MT7530_CREV;
+	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base_addr, sw_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	return dsa_register_switch(priv->ds);
+}
+
+static int
+mt7988_remove(struct platform_device *pdev)
+{
+	struct mt7530_priv *priv = platform_get_drvdata(pdev);
+
+	if (priv)
+		mt7530_remove_common(priv);
+
+	return 0;
+}
+
+static void mt7988_shutdown(struct platform_device *pdev)
+{
+	struct mt7530_priv *priv = platform_get_drvdata(pdev);
+
+	if (!priv)
+		return;
+
+	dsa_switch_shutdown(priv->ds);
+
+	dev_set_drvdata(&pdev->dev, NULL);
+}
+
+static struct platform_driver mt7988_platform_driver = {
+	.probe  = mt7988_probe,
+	.remove = mt7988_remove,
+	.shutdown = mt7988_shutdown,
+	.driver = {
+		.name = "mt7988-switch",
+		.of_match_table = mt7988_of_match,
+	},
+};
+module_platform_driver(mt7988_platform_driver);
+
+MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
+MODULE_DESCRIPTION("Driver for Mediatek MT7530 Switch (MMIO)");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ac666da2d10dc..e5347dd2521b3 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1987,6 +1987,47 @@  static const struct irq_domain_ops mt7530_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void
+mt7988_irq_mask(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	priv->irq_enable &= ~BIT(d->hwirq);
+	mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
+}
+
+static void
+mt7988_irq_unmask(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	priv->irq_enable |= BIT(d->hwirq);
+	mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
+}
+
+static struct irq_chip mt7988_irq_chip = {
+	.name = KBUILD_MODNAME,
+	.irq_mask = mt7988_irq_mask,
+	.irq_unmask = mt7988_irq_unmask,
+};
+
+static int
+mt7988_irq_map(struct irq_domain *domain, unsigned int irq,
+	       irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_chip_and_handler(irq, &mt7988_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(irq, true);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops mt7988_irq_domain_ops = {
+	.map = mt7988_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
 static void
 mt7530_setup_mdio_irq(struct mt7530_priv *priv)
 {
@@ -2021,8 +2062,15 @@  mt7530_setup_irq(struct mt7530_priv *priv)
 		return priv->irq ? : -EINVAL;
 	}
 
-	priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
-						 &mt7530_irq_domain_ops, priv);
+	if (priv->id == ID_MT7988)
+		priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
+							 &mt7988_irq_domain_ops,
+							 priv);
+	else
+		priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
+							 &mt7530_irq_domain_ops,
+							 priv);
+
 	if (!priv->irq_domain) {
 		dev_err(dev, "failed to create IRQ domain\n");
 		return -ENOMEM;
@@ -2967,6 +3015,27 @@  static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int mt7988_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
+{
+	return 0;
+}
+
+static int mt7988_setup(struct dsa_switch *ds)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	/* Reset the switch */
+	reset_control_assert(priv->rstc);
+	usleep_range(20, 50);
+	reset_control_deassert(priv->rstc);
+	usleep_range(20, 50);
+
+	/* Reset the switch PHYs */
+	mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
+
+	return mt7531_setup_common(ds);
+}
+
 const struct dsa_switch_ops mt7530_switch_ops = {
 	.get_tag_protocol	= mtk_get_tag_protocol,
 	.setup			= mt753x_setup,
@@ -3041,6 +3110,19 @@  const struct mt753x_info mt753x_table[] = {
 		.mac_port_get_caps = mt7531_mac_port_get_caps,
 		.mac_port_config = mt7531_mac_config,
 	},
+	[ID_MT7988] = {
+		.id = ID_MT7988,
+		.pcs_ops = &mt7530_pcs_ops,
+		.sw_setup = mt7988_setup,
+		.phy_read_c22 = mt7531_ind_c22_phy_read,
+		.phy_write_c22 = mt7531_ind_c22_phy_write,
+		.phy_read_c45 = mt7531_ind_c45_phy_read,
+		.phy_write_c45 = mt7531_ind_c45_phy_write,
+		.pad_setup = mt7988_pad_setup,
+		.cpu_port_config = mt7531_cpu_port_config,
+		.mac_port_get_caps = mt7531_mac_port_get_caps,
+		.mac_port_config = mt7531_mac_config,
+	},
 };
 EXPORT_SYMBOL_GPL(mt753x_table);
 
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index ce02aa592a7a8..01db5c9724fa8 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -18,6 +18,7 @@  enum mt753x_id {
 	ID_MT7530 = 0,
 	ID_MT7621 = 1,
 	ID_MT7531 = 2,
+	ID_MT7988 = 3,
 };
 
 #define	NUM_TRGMII_CTRL			5
@@ -54,11 +55,11 @@  enum mt753x_id {
 #define  MT7531_MIRROR_PORT_SET(x)	(((x) & MIRROR_MASK) << 16)
 #define  MT7531_CPU_PMAP_MASK		GENMASK(7, 0)
 
-#define MT753X_MIRROR_REG(id)		(((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_REG(id)		((((id) == ID_MT7531) || ((id) == ID_MT7988)) ?	\
 					 MT7531_CFC : MT7530_MFC)
-#define MT753X_MIRROR_EN(id)		(((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_EN(id)		((((id) == ID_MT7531) || ((id) == ID_MT7988)) ?	\
 					 MT7531_MIRROR_EN : MIRROR_EN)
-#define MT753X_MIRROR_MASK(id)		(((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_MASK(id)		((((id) == ID_MT7531) || ((id) == ID_MT7988)) ?	\
 					 MT7531_MIRROR_MASK : MIRROR_MASK)
 
 /* Registers for BPDU and PAE frame control*/
@@ -295,9 +296,8 @@  enum mt7530_vlan_port_acc_frm {
 					 MT7531_FORCE_DPX | \
 					 MT7531_FORCE_RX_FC | \
 					 MT7531_FORCE_TX_FC)
-#define  PMCR_FORCE_MODE_ID(id)		(((id) == ID_MT7531) ? \
-					 MT7531_FORCE_MODE : \
-					 PMCR_FORCE_MODE)
+#define  PMCR_FORCE_MODE_ID(id)		((((id) == ID_MT7531) || ((id) == ID_MT7988)) ?	\
+					 MT7531_FORCE_MODE : PMCR_FORCE_MODE)
 #define  PMCR_LINK_SETTINGS_MASK	(PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
 					 PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
 					 PMCR_TX_FC_EN | PMCR_RX_FC_EN | \