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 AFD30C433DB for ; Sun, 20 Dec 2020 23:37:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7A97221D95 for ; Sun, 20 Dec 2020 23:37:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727079AbgLTXgu (ORCPT ); Sun, 20 Dec 2020 18:36:50 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:35060 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726776AbgLTXgu (ORCPT ); Sun, 20 Dec 2020 18:36:50 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1kr8Ep-00D2nd-74; Mon, 21 Dec 2020 00:35:43 +0100 Date: Mon, 21 Dec 2020 00:35:43 +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 4/8] net: sparx5: add port module support Message-ID: <20201220233543.GB3107610@lunn.ch> References: <20201217075134.919699-1-steen.hegelund@microchip.com> <20201217075134.919699-5-steen.hegelund@microchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201217075134.919699-5-steen.hegelund@microchip.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > + /* Aneg complete provides more information */ > + if (DEV2G5_PCS1G_ANEG_STATUS_ANEG_COMPLETE_GET(value)) { > + if (port->conf.portmode == PHY_INTERFACE_MODE_SGMII) { > + /* SGMII cisco aneg */ > + u32 spdvalue = ((lp_abil >> 10) & 3); u32 spdvalue = lp_abil & LPA_SGMII_SPD_MASK; > + > + status->link = !!((lp_abil >> 15) == 1) && status->link; Maybe status->link = !!((lp_abil & LPA_SGMII_LINK) && status->link; > + status->an_complete = true; > + status->duplex = (lp_abil >> 12) & 0x1 ? DUPLEX_FULL : DUPLEX_HALF; status->duplex = (lp_abil & LPA_SGMII_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF; > + if (spdvalue == LPA_SGMII_10) > + status->speed = SPEED_10; > + else if (spdvalue == LPA_SGMII_100) > + status->speed = SPEED_100; > + else > + status->speed = SPEED_1000; I wonder if there is a helper for this? > + } else { > + /* Clause 37 Aneg */ > + status->link = !((lp_abil >> 12) & 3) && status->link; > + status->an_complete = true; > + status->duplex = ((lp_abil >> 5) & 1) ? DUPLEX_FULL : DUPLEX_UNKNOWN; > + if ((lp_abil >> 8) & 1) /* symmetric pause */ > + status->pause = MLO_PAUSE_RX | MLO_PAUSE_TX; > + if (lp_abil & (1 << 7)) /* asymmetric pause */ > + status->pause |= MLO_PAUSE_RX; > + } Please check if there are any standard #defines you can use for this. Russell King has done some work for clause 37. Maybe there is some code in phy_driver.c you can use? phylink_decode_sgmii_word() > +static int sparx5_port_verify_speed(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + case PHY_INTERFACE_MODE_SGMII: > + if (conf->speed != SPEED_1000 && > + conf->speed != SPEED_100 && > + conf->speed != SPEED_10 && > + conf->speed != SPEED_2500) > + return sparx5_port_error(port, conf, SPX5_PERR_SPEED); Is it really SGMII over clocked at 2500? Or 2500BaseX? > +static int sparx5_port_fifo_sz(struct sparx5 *sparx5, > + u32 portno, u32 speed) > +{ > + u32 sys_clk = sparx5_clk_period(sparx5->coreclock); > + u32 mac_width = 8; > + u32 fifo_width = 16; > + u32 addition = 0; > + u32 mac_per = 6400, tmp1, tmp2, tmp3; > + u32 taxi_dist[SPX5_PORTS_ALL] = { const. As it is at the moment, it gets copied onto the stack, so it can be modified. Const i guess prevents that copy? > + 6, 8, 10, 6, 8, 10, 6, 8, 10, 6, 8, 10, > + 4, 4, 4, 4, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 4, 6, 8, 4, 6, 8, 6, 8, > + 2, 2, 2, 2, 2, 2, 2, 4, 2 > + }; > +static int sparx5_port_fwd_urg(struct sparx5 *sparx5, u32 speed) What is urg? > +static u16 sparx5_get_aneg_word(struct sparx5_port_config *conf) > +{ > + if (conf->portmode == PHY_INTERFACE_MODE_1000BASEX) /* cl-37 aneg */ > + return ((1 << 14) | /* ack */ > + ((conf->pause ? 1 : 0) << 8) | /* asymmetric pause */ > + ((conf->pause ? 1 : 0) << 7) | /* symmetric pause */ > + (1 << 5)); /* FDX only */ ADVERTISE_LPACK, ADVERTISE_PAUSE_ASYM, ADVERTISE_PAUSE_CAP, ADVERTISE_1000XFULL? > +int sparx5_port_config(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + bool high_speed_dev = sparx5_is_high_speed_device(conf); > + int err, urgency, stop_wm; > + > + err = sparx5_port_verify_speed(sparx5, port, conf); > + if (err) > + return err; > + > + /* high speed device is already configured */ > + if (!high_speed_dev) > + sparx5_port_config_low_set(sparx5, port, conf); > + > + /* Configure flow control */ > + err = sparx5_port_fc_setup(sparx5, port, conf); > + if (err) > + return err; > + > + /* Set the DSM stop watermark */ > + stop_wm = sparx5_port_fifo_sz(sparx5, port->portno, conf->speed); > + spx5_rmw(DSM_DEV_TX_STOP_WM_CFG_DEV_TX_STOP_WM_SET(stop_wm), > + DSM_DEV_TX_STOP_WM_CFG_DEV_TX_STOP_WM, > + sparx5, > + DSM_DEV_TX_STOP_WM_CFG(port->portno)); > + > + /* Enable port forwarding */ > + urgency = sparx5_port_fwd_urg(sparx5, conf->speed); > + spx5_rmw(QFWD_SWITCH_PORT_MODE_PORT_ENA_SET(1) | > + QFWD_SWITCH_PORT_MODE_FWD_URGENCY_SET(urgency), > + QFWD_SWITCH_PORT_MODE_PORT_ENA | > + QFWD_SWITCH_PORT_MODE_FWD_URGENCY, > + sparx5, > + QFWD_SWITCH_PORT_MODE(port->portno)); What does it mean by port forwarding? By default, packets should only go to the CPU, until the port is added to a bridge. I've not thought much about L3, since DSA so far only has L2 switches, but i guess you don't need to enable L3 forwarding until a route out the port has been added? > +/* Initialize port config to default */ > +int sparx5_port_init(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + /* Discard pause frame 01-80-C2-00-00-01 */ > + spx5_wr(0xC, sparx5, ANA_CL_CAPTURE_BPDU_CFG(port->portno)); The comment is about pause frames, but the macro contain BPDU? 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 32B0EC433DB for ; Sun, 20 Dec 2020 23:38:41 +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 C80EF21D95 for ; Sun, 20 Dec 2020 23:38:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C80EF21D95 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=dPOfNmX23Rz32A2c+58gkwpOGwv3kF30mZ6Ljh9sc3I=; b=RDzVE7Xy6A1PziMkFZ0WbPEXB FhOzwFdegzH/GaX1STLRuk6Ho9qGizo2ybsdKT24a2cos9H9UmUol00VIFxtr6OzFtiA3QaJdB6p3 a+xd7iSgq405Jjm4y+JMLSJmb/sMDFrzHO0guj3ZNxHvV9Hb9GO8Nm7bDYyVcxMK1eit5e/YQ5yRj UgmU8tPjAFbySbrRIfrhPOOK7f0xHL2GtHC1s4lg/EMW+wR2tAZolOUq5YtgJOj4TeCFIBz3zN1HP 1jyGoDBcXXvT87JZmFAK7NazJBzuw//PWo/tJ1hxzWB8D8RH5780W6we/Hxate3H1Bb/V92OQkFrO uLi54k90A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kr8Fy-0006vX-EB; Sun, 20 Dec 2020 23:36:54 +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 1kr8Fv-0006v8-Jb for linux-arm-kernel@lists.infradead.org; Sun, 20 Dec 2020 23:36:52 +0000 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1kr8Ep-00D2nd-74; Mon, 21 Dec 2020 00:35:43 +0100 Date: Mon, 21 Dec 2020 00:35:43 +0100 From: Andrew Lunn To: Steen Hegelund Subject: Re: [RFC PATCH v2 4/8] net: sparx5: add port module support Message-ID: <20201220233543.GB3107610@lunn.ch> References: <20201217075134.919699-1-steen.hegelund@microchip.com> <20201217075134.919699-5-steen.hegelund@microchip.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201217075134.919699-5-steen.hegelund@microchip.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201220_183651_827335_E158AC8F X-CRM114-Status: GOOD ( 22.52 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > + /* Aneg complete provides more information */ > + if (DEV2G5_PCS1G_ANEG_STATUS_ANEG_COMPLETE_GET(value)) { > + if (port->conf.portmode == PHY_INTERFACE_MODE_SGMII) { > + /* SGMII cisco aneg */ > + u32 spdvalue = ((lp_abil >> 10) & 3); u32 spdvalue = lp_abil & LPA_SGMII_SPD_MASK; > + > + status->link = !!((lp_abil >> 15) == 1) && status->link; Maybe status->link = !!((lp_abil & LPA_SGMII_LINK) && status->link; > + status->an_complete = true; > + status->duplex = (lp_abil >> 12) & 0x1 ? DUPLEX_FULL : DUPLEX_HALF; status->duplex = (lp_abil & LPA_SGMII_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF; > + if (spdvalue == LPA_SGMII_10) > + status->speed = SPEED_10; > + else if (spdvalue == LPA_SGMII_100) > + status->speed = SPEED_100; > + else > + status->speed = SPEED_1000; I wonder if there is a helper for this? > + } else { > + /* Clause 37 Aneg */ > + status->link = !((lp_abil >> 12) & 3) && status->link; > + status->an_complete = true; > + status->duplex = ((lp_abil >> 5) & 1) ? DUPLEX_FULL : DUPLEX_UNKNOWN; > + if ((lp_abil >> 8) & 1) /* symmetric pause */ > + status->pause = MLO_PAUSE_RX | MLO_PAUSE_TX; > + if (lp_abil & (1 << 7)) /* asymmetric pause */ > + status->pause |= MLO_PAUSE_RX; > + } Please check if there are any standard #defines you can use for this. Russell King has done some work for clause 37. Maybe there is some code in phy_driver.c you can use? phylink_decode_sgmii_word() > +static int sparx5_port_verify_speed(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + case PHY_INTERFACE_MODE_SGMII: > + if (conf->speed != SPEED_1000 && > + conf->speed != SPEED_100 && > + conf->speed != SPEED_10 && > + conf->speed != SPEED_2500) > + return sparx5_port_error(port, conf, SPX5_PERR_SPEED); Is it really SGMII over clocked at 2500? Or 2500BaseX? > +static int sparx5_port_fifo_sz(struct sparx5 *sparx5, > + u32 portno, u32 speed) > +{ > + u32 sys_clk = sparx5_clk_period(sparx5->coreclock); > + u32 mac_width = 8; > + u32 fifo_width = 16; > + u32 addition = 0; > + u32 mac_per = 6400, tmp1, tmp2, tmp3; > + u32 taxi_dist[SPX5_PORTS_ALL] = { const. As it is at the moment, it gets copied onto the stack, so it can be modified. Const i guess prevents that copy? > + 6, 8, 10, 6, 8, 10, 6, 8, 10, 6, 8, 10, > + 4, 4, 4, 4, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 11, 12, 13, 14, 15, 16, 17, 18, > + 4, 6, 8, 4, 6, 8, 6, 8, > + 2, 2, 2, 2, 2, 2, 2, 4, 2 > + }; > +static int sparx5_port_fwd_urg(struct sparx5 *sparx5, u32 speed) What is urg? > +static u16 sparx5_get_aneg_word(struct sparx5_port_config *conf) > +{ > + if (conf->portmode == PHY_INTERFACE_MODE_1000BASEX) /* cl-37 aneg */ > + return ((1 << 14) | /* ack */ > + ((conf->pause ? 1 : 0) << 8) | /* asymmetric pause */ > + ((conf->pause ? 1 : 0) << 7) | /* symmetric pause */ > + (1 << 5)); /* FDX only */ ADVERTISE_LPACK, ADVERTISE_PAUSE_ASYM, ADVERTISE_PAUSE_CAP, ADVERTISE_1000XFULL? > +int sparx5_port_config(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + bool high_speed_dev = sparx5_is_high_speed_device(conf); > + int err, urgency, stop_wm; > + > + err = sparx5_port_verify_speed(sparx5, port, conf); > + if (err) > + return err; > + > + /* high speed device is already configured */ > + if (!high_speed_dev) > + sparx5_port_config_low_set(sparx5, port, conf); > + > + /* Configure flow control */ > + err = sparx5_port_fc_setup(sparx5, port, conf); > + if (err) > + return err; > + > + /* Set the DSM stop watermark */ > + stop_wm = sparx5_port_fifo_sz(sparx5, port->portno, conf->speed); > + spx5_rmw(DSM_DEV_TX_STOP_WM_CFG_DEV_TX_STOP_WM_SET(stop_wm), > + DSM_DEV_TX_STOP_WM_CFG_DEV_TX_STOP_WM, > + sparx5, > + DSM_DEV_TX_STOP_WM_CFG(port->portno)); > + > + /* Enable port forwarding */ > + urgency = sparx5_port_fwd_urg(sparx5, conf->speed); > + spx5_rmw(QFWD_SWITCH_PORT_MODE_PORT_ENA_SET(1) | > + QFWD_SWITCH_PORT_MODE_FWD_URGENCY_SET(urgency), > + QFWD_SWITCH_PORT_MODE_PORT_ENA | > + QFWD_SWITCH_PORT_MODE_FWD_URGENCY, > + sparx5, > + QFWD_SWITCH_PORT_MODE(port->portno)); What does it mean by port forwarding? By default, packets should only go to the CPU, until the port is added to a bridge. I've not thought much about L3, since DSA so far only has L2 switches, but i guess you don't need to enable L3 forwarding until a route out the port has been added? > +/* Initialize port config to default */ > +int sparx5_port_init(struct sparx5 *sparx5, > + struct sparx5_port *port, > + struct sparx5_port_config *conf) > +{ > + /* Discard pause frame 01-80-C2-00-00-01 */ > + spx5_wr(0xC, sparx5, ANA_CL_CAPTURE_BPDU_CFG(port->portno)); The comment is about pause frames, but the macro contain BPDU? Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel