From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D474DC433DB for ; Tue, 22 Dec 2020 14:42:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 962EC229C6 for ; Tue, 22 Dec 2020 14:42:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727633AbgLVOmh (ORCPT ); Tue, 22 Dec 2020 09:42:37 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:36962 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727019AbgLVOmg (ORCPT ); Tue, 22 Dec 2020 09:42:36 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1krir7-00DOYM-6N; Tue, 22 Dec 2020 15:41:41 +0100 Date: Tue, 22 Dec 2020 15:41:41 +0100 From: Andrew Lunn To: Steen Hegelund Cc: "David S. Miller" , Jakub Kicinski , Russell King , Lars Povlsen , Bjarni Jonasson , Microchip Linux Driver Support , Alexandre Belloni , Madalin Bucur , Nicolas Ferre , Mark Einon , Masahiro Yamada , Arnd Bergmann , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH v2 3/8] net: sparx5: add hostmode with phylink support Message-ID: <20201222144141.GK3107610@lunn.ch> References: <20201217075134.919699-1-steen.hegelund@microchip.com> <20201217075134.919699-4-steen.hegelund@microchip.com> <20201219195133.GD3026679@lunn.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 22, 2020 at 10:46:12AM +0100, Steen Hegelund wrote: > Hi Andrew, > > On Sat, 2020-12-19 at 20:51 +0100, Andrew Lunn wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > know the content is safe > > > > > +     /* Create a phylink for PHY management.  Also handles SFPs */ > > > +     spx5_port->phylink_config.dev = &spx5_port->ndev->dev; > > > +     spx5_port->phylink_co > > > nfig.type = PHYLINK_NETDEV; > > > +     spx5_port->phylink_config.pcs_poll = true; > > > + > > > +     /* phylink needs a valid interface mode to parse dt node */ > > > +     if (phy_mode == PHY_INTERFACE_MODE_NA) > > > +             phy_mode = PHY_INTERFACE_MODE_10GBASER; > > > > Maybe just enforce a valid value in DT? > > Maybe I need to clarify that you must choose between an Ethernet cuPHY > or an SFP, so it is optional. But you also need to watch out for somebody putting a copper modules in an SFP port. phylink will then set the mode to SGMII for a 1G copper module, etc. > > > +/* Configuration */ > > > +static inline bool sparx5_use_cu_phy(struct sparx5_port *port) > > > +{ > > > +     return port->conf.phy_mode != PHY_INTERFACE_MODE_NA; > > > +} > > > > That is a rather odd definition of copper. > > Should I rather use a bool property to select between the two options > (cuPHY or SFP)? I guess what you are trying to indicate is between a hard wired Copper PHY and an SFP cage? You have some sort of MII switch which allows the MAC to be connected to either the QSGMII PHY, or an SFP cage? But since the SFP cage could be populated with a copper PHY, and PHYLINK will then instantiate a phylib copper PHY driver for it, looking at phy_mode is not reliable. You need a property which selects the port, not the technology. > > > +static int sparx5_port_open(struct net_device *ndev) > > > +{ > > > +     struct sparx5_port *port = netdev_priv(ndev); > > > +     int err = 0; > > > + > > > +     err = phylink_of_phy_connect(port->phylink, port->of_node, > > > 0); > > > +     if (err) { > > > +             netdev_err(ndev, "Could not attach to PHY\n"); > > > +             return err; > > > +     } > > > + > > > +     phylink_start(port->phylink); > > > + > > > +     if (!ndev->phydev) { > > > > Humm. When is ndev->phydev set? I don't think phylink ever sets it. > > Indirectly: phylink_of_phy_connect uses phy_attach_direct and that sets > the phydev. Ah, O.K. But watch out for a copper SFP module! > > > +static void sparx5_xtr_grp(struct sparx5 *sparx5, u8 grp, bool > > > byte_swap) > > > +{ > > > +     int i, byte_cnt = 0; > > > +     bool eof_flag = false, pruned_flag = false, abort_flag = > > > false; > > > +     u32 ifh[IFH_LEN]; > > > +     struct sk_buff *skb; > > > +     struct frame_info fi; > > > +     struct sparx5_port *port; > > > +     struct net_device *netdev; > > > +     u32 *rxbuf; > > > + > > > +     /* Get IFH */ > > > +     for (i = 0; i < IFH_LEN; i++) > > > +             ifh[i] = spx5_rd(sparx5, QS_XTR_RD(grp)); > > > + > > > +     /* Decode IFH (whats needed) */ > > > +     sparx5_ifh_parse(ifh, &fi); > > > + > > > +     /* Map to port netdev */ > > > +     port = fi.src_port < SPX5_PORTS ? > > > +             sparx5->ports[fi.src_port] : NULL; > > > +     if (!port || !port->ndev) { > > > +             dev_err(sparx5->dev, "Data on inactive port %d\n", > > > fi.src_port); > > > +             sparx5_xtr_flush(sparx5, grp); > > > +             return; > > > +     } > > > + > > > +     /* Have netdev, get skb */ > > > +     netdev = port->ndev; > > > +     skb = netdev_alloc_skb(netdev, netdev->mtu + ETH_HLEN); > > > +     if (!skb) { > > > +             sparx5_xtr_flush(sparx5, grp); > > > +             dev_err(sparx5->dev, "No skb allocated\n"); > > > +             return; > > > +     } > > > +     rxbuf = (u32 *)skb->data; > > > + > > > +     /* Now, pull frame data */ > > > +     while (!eof_flag) { > > > +             u32 val = spx5_rd(sparx5, QS_XTR_RD(grp)); > > > +             u32 cmp = val; > > > + > > > +             if (byte_swap) > > > +                     cmp = ntohl((__force __be32)val); > > > + > > > +             switch (cmp) { > > > +             case XTR_NOT_READY: > > > +                     break; > > > +             case XTR_ABORT: > > > +                     /* No accompanying data */ > > > +                     abort_flag = true; > > > +                     eof_flag = true; > > > +                     break; > > > +             case XTR_EOF_0: > > > +             case XTR_EOF_1: > > > +             case XTR_EOF_2: > > > +             case XTR_EOF_3: > > > +                     /* This assumes STATUS_WORD_POS == 1, Status > > > +                      * just after last data > > > +                      */ > > > +                     byte_cnt -= (4 - XTR_VALID_BYTES(val)); > > > +                     eof_flag = true; > > > +                     break; > > > +             case XTR_PRUNED: > > > +                     /* But get the last 4 bytes as well */ > > > +                     eof_flag = true; > > > +                     pruned_flag = true; > > > +                     fallthrough; > > > +             case XTR_ESCAPE: > > > +                     *rxbuf = spx5_rd(sparx5, QS_XTR_RD(grp)); > > > +                     byte_cnt += 4; > > > +                     rxbuf++; > > > +                     break; > > > +             default: > > > +                     *rxbuf = val; > > > +                     byte_cnt += 4; > > > +                     rxbuf++; > > > +             } > > > +     } > > > + > > > +     if (abort_flag || pruned_flag || !eof_flag) { > > > +             netdev_err(netdev, "Discarded frame: abort:%d > > > pruned:%d eof:%d\n", > > > +                        abort_flag, pruned_flag, eof_flag); > > > +             kfree_skb(skb); > > > +             return; > > > +     } > > > + > > > +     if (!netif_oper_up(netdev)) { > > > +             netdev_err(netdev, "Discarded frame: Interface not > > > up\n"); > > > +             kfree_skb(skb); > > > +             return; > > > +     } > > > > Why is it sending frames when it is not up? > > This is intended for received frames. A situation where the lower > layers have been enabled correctly but not the port. But why should that happen? It suggests you have the order wrong. The lower level should only be enabled once the port is opened. > > No DMA? What sort of performance do you get? Enough for the odd BPDU, > > IGMP frame etc, but i guess you don't want any real bulk data to be > > sent this way? > > Yes the register based injection/extration is not going to be fast, but > the FDMA and its driver is being sent later as separate series to keep > the size of this review down. FDMA? I need a bit more background here, just to make use this should be a pure switchdev driver and not a DSA driver. > > > > > > +irqreturn_t sparx5_xtr_handler(int irq, void *_sparx5) > > > +{ > > > +     struct sparx5 *sparx5 = _sparx5; > > > + > > > +     /* Check data in queue */ > > > +     while (spx5_rd(sparx5, QS_XTR_DATA_PRESENT) & BIT(XTR_QUEUE)) > > > +             sparx5_xtr_grp(sparx5, XTR_QUEUE, false); > > > + > > > +     return IRQ_HANDLED; > > > +} > > > > Is there any sort of limit how many times this will loop? If somebody > > is blasting 10Gbps at the CPU, will it ever get out of this loop? > > Hmmm, not at the moment but this is because the FDMA driver is intended > to be used in these scenarios. So throwing out an idea, which might be terrible. How about limiting it to 64 loops, the same as the NAPI poll? That might allow the machine to get some work done before the next interrupt? Does the hardware do interrupt coalescing? But is this is going to be quickly thrown away and replaced with FDMA, don't spend too much time on it. Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61FAAC433E0 for ; Tue, 22 Dec 2020 14:43:23 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0667C221FC for ; Tue, 22 Dec 2020 14:43:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0667C221FC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lunn.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MWp7om4YJDvyUzuxlS6SiTjcgR5jJeryTTz8gX9p/7k=; b=DYMS+Xd435Ufc4iLLfqkGy8j8 wuSzngUIolISL8uZdHEjVcOG0srKKo5ZqGWIuRMCR1OBBZXST0o8IQ2fx6R2fioQbj4FgYlD0+j4n SZVXSp7/gAvcf/Ych5wTqY+40/aIG7VYWRf7lCCPoasZmu6Vspc3eAcmk9oTYQ37+ws2TdpnRHUwj wZFpME2tF11Qe3JUmgauAz5Ah7jGvNtDUr7p7Acny32iUIsVyRrmI0n35DMNh+5ibxr0+chwvgqC1 /UCNPiwFybm5R2FsghHSZDQMVnYbEyt0HOQz9+d/bsUKnFe2IkpDBdDX9C1S2rlA6nW/ocgfhC292 ZEiw92+Vw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1krirN-0008Ay-Vs; Tue, 22 Dec 2020 14:41:58 +0000 Received: from vps0.lunn.ch ([185.16.172.187]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1krirL-0008Ac-Vj for linux-arm-kernel@lists.infradead.org; Tue, 22 Dec 2020 14:41:56 +0000 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1krir7-00DOYM-6N; Tue, 22 Dec 2020 15:41:41 +0100 Date: Tue, 22 Dec 2020 15:41:41 +0100 From: Andrew Lunn To: Steen Hegelund Subject: Re: [RFC PATCH v2 3/8] net: sparx5: add hostmode with phylink support Message-ID: <20201222144141.GK3107610@lunn.ch> References: <20201217075134.919699-1-steen.hegelund@microchip.com> <20201217075134.919699-4-steen.hegelund@microchip.com> <20201219195133.GD3026679@lunn.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201222_094156_167725_D3A9AFB3 X-CRM114-Status: GOOD ( 37.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Bjarni Jonasson , Alexandre Belloni , linux-kernel@vger.kernel.org, Arnd Bergmann , Madalin Bucur , netdev@vger.kernel.org, Masahiro Yamada , Russell King , Microchip Linux Driver Support , linux-arm-kernel@lists.infradead.org, Mark Einon , Jakub Kicinski , "David S. Miller" , Lars Povlsen Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Dec 22, 2020 at 10:46:12AM +0100, Steen Hegelund wrote: > Hi Andrew, > = > On Sat, 2020-12-19 at 20:51 +0100, Andrew Lunn wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > know the content is safe > > = > > > +=A0=A0=A0=A0 /* Create a phylink for PHY management.=A0 Also handles= SFPs */ > > > +=A0=A0=A0=A0 spx5_port->phylink_config.dev =3D &spx5_port->ndev->dev; > > > +=A0=A0=A0=A0 spx5_port->phylink_co > > > nfig.type =3D PHYLINK_NETDEV; > > > +=A0=A0=A0=A0 spx5_port->phylink_config.pcs_poll =3D true; > > > + > > > +=A0=A0=A0=A0 /* phylink needs a valid interface mode to parse dt nod= e */ > > > +=A0=A0=A0=A0 if (phy_mode =3D=3D PHY_INTERFACE_MODE_NA) > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 phy_mode =3D PHY_INTERFACE_MODE= _10GBASER; > > = > > Maybe just enforce a valid value in DT? > = > Maybe I need to clarify that you must choose between an Ethernet cuPHY > or an SFP, so it is optional. But you also need to watch out for somebody putting a copper modules in an SFP port. phylink will then set the mode to SGMII for a 1G copper module, etc. > > > +/* Configuration */ > > > +static inline bool sparx5_use_cu_phy(struct sparx5_port *port) > > > +{ > > > +=A0=A0=A0=A0 return port->conf.phy_mode !=3D PHY_INTERFACE_MODE_NA; > > > +} > > = > > That is a rather odd definition of copper. > = > Should I rather use a bool property to select between the two options > (cuPHY or SFP)? I guess what you are trying to indicate is between a hard wired Copper PHY and an SFP cage? You have some sort of MII switch which allows the MAC to be connected to either the QSGMII PHY, or an SFP cage? But since the SFP cage could be populated with a copper PHY, and PHYLINK will then instantiate a phylib copper PHY driver for it, looking at phy_mode is not reliable. You need a property which selects the port, not the technology. > > > +static int sparx5_port_open(struct net_device *ndev) > > > +{ > > > +=A0=A0=A0=A0 struct sparx5_port *port =3D netdev_priv(ndev); > > > +=A0=A0=A0=A0 int err =3D 0; > > > + > > > +=A0=A0=A0=A0 err =3D phylink_of_phy_connect(port->phylink, port->of_= node, > > > 0); > > > +=A0=A0=A0=A0 if (err) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 netdev_err(ndev, "Could not att= ach to PHY\n"); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return err; > > > +=A0=A0=A0=A0 } > > > + > > > +=A0=A0=A0=A0 phylink_start(port->phylink); > > > + > > > +=A0=A0=A0=A0 if (!ndev->phydev) { > > = > > Humm. When is ndev->phydev set? I don't think phylink ever sets it. > = > Indirectly: phylink_of_phy_connect uses phy_attach_direct and that sets > the phydev. Ah, O.K. But watch out for a copper SFP module! > > > +static void sparx5_xtr_grp(struct sparx5 *sparx5, u8 grp, bool > > > byte_swap) > > > +{ > > > +=A0=A0=A0=A0 int i, byte_cnt =3D 0; > > > +=A0=A0=A0=A0 bool eof_flag =3D false, pruned_flag =3D false, abort_f= lag =3D > > > false; > > > +=A0=A0=A0=A0 u32 ifh[IFH_LEN]; > > > +=A0=A0=A0=A0 struct sk_buff *skb; > > > +=A0=A0=A0=A0 struct frame_info fi; > > > +=A0=A0=A0=A0 struct sparx5_port *port; > > > +=A0=A0=A0=A0 struct net_device *netdev; > > > +=A0=A0=A0=A0 u32 *rxbuf; > > > + > > > +=A0=A0=A0=A0 /* Get IFH */ > > > +=A0=A0=A0=A0 for (i =3D 0; i < IFH_LEN; i++) > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ifh[i] =3D spx5_rd(sparx5, QS_X= TR_RD(grp)); > > > + > > > +=A0=A0=A0=A0 /* Decode IFH (whats needed) */ > > > +=A0=A0=A0=A0 sparx5_ifh_parse(ifh, &fi); > > > + > > > +=A0=A0=A0=A0 /* Map to port netdev */ > > > +=A0=A0=A0=A0 port =3D fi.src_port < SPX5_PORTS ? > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 sparx5->ports[fi.src_port] : NU= LL; > > > +=A0=A0=A0=A0 if (!port || !port->ndev) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(sparx5->dev, "Data on i= nactive port %d\n", > > > fi.src_port); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 sparx5_xtr_flush(sparx5, grp); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return; > > > +=A0=A0=A0=A0 } > > > + > > > +=A0=A0=A0=A0 /* Have netdev, get skb */ > > > +=A0=A0=A0=A0 netdev =3D port->ndev; > > > +=A0=A0=A0=A0 skb =3D netdev_alloc_skb(netdev, netdev->mtu + ETH_HLEN= ); > > > +=A0=A0=A0=A0 if (!skb) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 sparx5_xtr_flush(sparx5, grp); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(sparx5->dev, "No skb al= located\n"); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return; > > > +=A0=A0=A0=A0 } > > > +=A0=A0=A0=A0 rxbuf =3D (u32 *)skb->data; > > > + > > > +=A0=A0=A0=A0 /* Now, pull frame data */ > > > +=A0=A0=A0=A0 while (!eof_flag) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 u32 val =3D spx5_rd(sparx5, QS_= XTR_RD(grp)); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 u32 cmp =3D val; > > > + > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (byte_swap) > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 cmp =3D= ntohl((__force __be32)val); > > > + > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 switch (cmp) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_NOT_READY: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 break; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_ABORT: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* No a= ccompanying data */ > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 abort_f= lag =3D true; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 eof_fla= g =3D true; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 break; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_EOF_0: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_EOF_1: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_EOF_2: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_EOF_3: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* This= assumes STATUS_WORD_POS =3D=3D 1, Status > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 * ju= st after last data > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 */ > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 byte_cn= t -=3D (4 - XTR_VALID_BYTES(val)); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 eof_fla= g =3D true; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 break; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_PRUNED: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* But = get the last 4 bytes as well */ > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 eof_fla= g =3D true; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 pruned_= flag =3D true; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 fallthr= ough; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 case XTR_ESCAPE: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 *rxbuf = =3D spx5_rd(sparx5, QS_XTR_RD(grp)); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 byte_cn= t +=3D 4; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 rxbuf++; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 break; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 default: > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 *rxbuf = =3D val; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 byte_cn= t +=3D 4; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 rxbuf++; > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 } > > > +=A0=A0=A0=A0 } > > > + > > > +=A0=A0=A0=A0 if (abort_flag || pruned_flag || !eof_flag) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 netdev_err(netdev, "Discarded f= rame: abort:%d > > > pruned:%d eof:%d\n", > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0 abort_flag, pruned_flag, eof_flag); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 kfree_skb(skb); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return; > > > +=A0=A0=A0=A0 } > > > + > > > +=A0=A0=A0=A0 if (!netif_oper_up(netdev)) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 netdev_err(netdev, "Discarded f= rame: Interface not > > > up\n"); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 kfree_skb(skb); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return; > > > +=A0=A0=A0=A0 } > > = > > Why is it sending frames when it is not up? > = > This is intended for received frames. A situation where the lower > layers have been enabled correctly but not the port. But why should that happen? It suggests you have the order wrong. The lower level should only be enabled once the port is opened. > > No DMA? What sort of performance do you get? Enough for the odd BPDU, > > IGMP frame etc, but i guess you don't want any real bulk data to be > > sent this way? > = > Yes the register based injection/extration is not going to be fast, but > the FDMA and its driver is being sent later as separate series to keep > the size of this review down. FDMA? I need a bit more background here, just to make use this should be a pure switchdev driver and not a DSA driver. > = > > = > > > +irqreturn_t sparx5_xtr_handler(int irq, void *_sparx5) > > > +{ > > > +=A0=A0=A0=A0 struct sparx5 *sparx5 =3D _sparx5; > > > + > > > +=A0=A0=A0=A0 /* Check data in queue */ > > > +=A0=A0=A0=A0 while (spx5_rd(sparx5, QS_XTR_DATA_PRESENT) & BIT(XTR_Q= UEUE)) > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 sparx5_xtr_grp(sparx5, XTR_QUEU= E, false); > > > + > > > +=A0=A0=A0=A0 return IRQ_HANDLED; > > > +} > > = > > Is there any sort of limit how many times this will loop? If somebody > > is blasting 10Gbps at the CPU, will it ever get out of this loop? > = > Hmmm, not at the moment but this is because the FDMA driver is intended > to be used in these scenarios. So throwing out an idea, which might be terrible. How about limiting it to 64 loops, the same as the NAPI poll? That might allow the machine to get some work done before the next interrupt? Does the hardware do interrupt coalescing? But is this is going to be quickly thrown away and replaced with FDMA, don't spend too much time on it. Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel