Received: by 2002:a05:6358:11c7:b0:104:8066:f915 with SMTP id i7csp1590471rwl; Fri, 31 Mar 2023 13:34:56 -0700 (PDT) X-Google-Smtp-Source: AK7set8BNbNHo3EiX5nVDWOPtme976P2MEvy/5BJ+7DvsrccUZ7D4uoUVJroE7lFj0t+NFFfTC1X X-Received: by 2002:a05:6a20:7da6:b0:d9:3de:541e with SMTP id v38-20020a056a207da600b000d903de541emr25824187pzj.6.1680294895942; Fri, 31 Mar 2023 13:34:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680294895; cv=none; d=google.com; s=arc-20160816; b=pQbOmPpjuNjFB78qwlI2nBTFXC4zjYcrOZROU/FvijlXG1X77rIesaM9YcQqXPxdU3 99PuPbIr2oEaP9fLH7XpcwdJj0jQ5z3T7SKiogCBSbmfqR80HyzSMfwnFm/bygmNICwr DEdOp2Al9QOpCOubxC8FBPgLzAwfvSVsF0AaYnJXwcTNk4bhKUa35hL1mLbPSnrxrhWP lXcPBfW5MfQKLympl8fxYJEzJDmjxk/UXvUyLhiCF25DGGcHUbTzoTNdXao3ct+ahp9r Pa/hORxFfPkhqrYHR7tsP/0eSHVMf7VMghHPTQlVnYVbMCfcBI+rhBoAPC3TYT3WPE6v u9ZA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=/nhIea/FX9Tsn6rw8tWaBrlCILhzPstoaPbBbr89xes=; b=qSmhPtuUdvczAdsHCzVnS9cMUKJD9vZ1nhQf9b/544jrzttxkC747+vvccHKxnWd0w mcV1YVLf2op0984Cy0UwUlsUbR9OgMyHXfVkbu5iik2sRcSX3tL1d1VRMgrOoBPpstb+ w2ryNrB6tG5QEnjckaE0OAgR0jiSqucI16ANo+ctUAD0oN0v3tNk0El+GHiwd5BGm11P 5cMWbJm/xX1F6OOzKR6mz32SNfMDhvbBOPpjGSfOmfdvfFrXSToWiBZQLHRsm1Z6feYM 2XwcE0xYJP0zxtcOAiw+5b3xOSdgwm728LCOTeTqmGijJvj/CLP8R4uwh528K3xGvF18 qDAA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q1-20020aa79601000000b00627eace679asi3244654pfg.8.2023.03.31.13.34.21; Fri, 31 Mar 2023 13:34:55 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232426AbjCaUeG (ORCPT + 99 others); Fri, 31 Mar 2023 16:34:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51508 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229967AbjCaUeF (ORCPT ); Fri, 31 Mar 2023 16:34:05 -0400 Received: from fudo.makrotopia.org (fudo.makrotopia.org [IPv6:2a07:2ec0:3002::71]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D04491B7C3; Fri, 31 Mar 2023 13:34:03 -0700 (PDT) Received: from local by fudo.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.96) (envelope-from ) id 1piLRa-0007Dg-1j; Fri, 31 Mar 2023 22:33:54 +0200 Date: Fri, 31 Mar 2023 21:33:50 +0100 From: Daniel Golle To: =?utf-8?B?QXLEsW7DpyDDnE5BTA==?= 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 , Florian Fainelli , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Matthias Brugger , AngeloGioacchino Del Regno , Sean Wang , Landen Chao , DENG Qingfang , Philipp Zabel , Sam Shih , Lorenzo Bianconi , John Crispin , Felix Fietkau , Luiz Angelo Daros de Luca Subject: Re: [PATCH net-next 14/15] net: dsa: mt7530: introduce driver for MT7988 built-in switch Message-ID: References: <6a7c5f81-a8a3-27b5-4af3-7175a3313f9a@arinc9.com> <7d0acaef-0cec-91b9-a5c6-d094b71e3dbd@arinc9.com> <28d048c9-6389-749b-d0eb-18a9c2d83c4e@arinc9.com> <56adf82a-3db0-5909-e948-e21717e3fe03@arinc9.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <56adf82a-3db0-5909-e948-e21717e3fe03@arinc9.com> X-Spam-Status: No, score=0.0 required=5.0 tests=SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > > > > --- > > > > > >    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 > > > > > >    M:    Landen Chao > > > > > >    M:    DENG Qingfang > > > > > > +M:    Daniel Golle > > > > > >    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 ' 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.