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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E8B1C433EF for ; Fri, 10 Jun 2022 07:27:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346714AbiFJH1s (ORCPT ); Fri, 10 Jun 2022 03:27:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59652 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343879AbiFJH1q (ORCPT ); Fri, 10 Jun 2022 03:27:46 -0400 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D01C5766E; Fri, 10 Jun 2022 00:27:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=H0Po1e85toAXw3RbuejcHeMli/OCRToQR1vDolHiUUU=; b=rx0Zw5yu5PhdYdeuwAbaJfgswf tPS/HuOnRprB6tVdAKCZlnv8lAxe10w7a8wsYkCaPSwJwNoQ+XQDHIXYz6/f7vpbf+YqOxdzmQmva f91uWsQpJWTIlwP3zmz0kkAWH9pZnKcDAyKv45sjjmepDFUUVpvVZesCONMTg+RQVLmAyqVkbf9I4 nxMbpa/DWNFebz5u/sM5zjEdC4WdyU2isa5dNlDh78sOAyqwSgvQ2TZ8Bpf9vo5Ad+0/1XGvkIbVR JN+HhO8r6b2S+x1oEd5wSyOfJtPeypbRo1jdfmSKlUmU0ULO1MUhWTWKQi4njI/gMNuKxNIMEsguQ 9PjnC1eA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:32812) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nzZ2z-0007Dy-Ap; Fri, 10 Jun 2022 08:27:09 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1nzZ2l-0003Ut-L7; Fri, 10 Jun 2022 08:26:55 +0100 Date: Fri, 10 Jun 2022 08:26:55 +0100 From: "Russell King (Oracle)" To: Ong Boon Leong Cc: Alexandre Torgue , Jose Abreu , Andrew Lunn , Heiner Kallweit , Paolo Abeni , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Vladimir Oltean , Vivien Didelot , Florian Fainelli , Maxime Coquelin , Giuseppe Cavallaro , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Emilio Riva Subject: Re: [PATCH net-next v2 3/6] net: pcs: xpcs: add CL37 1000BASE-X AN support Message-ID: References: <20220610032941.113690-1-boon.leong.ong@intel.com> <20220610032941.113690-4-boon.leong.ong@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220610032941.113690-4-boon.leong.ong@intel.com> Sender: Russell King (Oracle) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Jun 10, 2022 at 11:29:38AM +0800, Ong Boon Leong wrote: > +static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs, > + struct phylink_link_state *state) > +{ > + int lpa, adv; > + int ret; > + > + if (state->an_enabled) { > + /* Reset link state */ > + state->link = false; > + > + lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_LPA); > + if (lpa < 0 || lpa & LPA_RFAULT) > + return lpa; > + > + adv = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE); > + if (adv < 0) > + return adv; > + > + if (lpa & ADVERTISE_1000XFULL && > + adv & ADVERTISE_1000XFULL) { > + state->link = true; > + state->speed = SPEED_1000; > + state->duplex = DUPLEX_FULL; > + } phylink_mii_c22_pcs_decode_state() is your friend here, will implement this correctly, and will set lp_advertising correctly as well. > + > + /* Clear CL37 AN complete status */ > + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0); > + if (ret < 0) > + return ret; Why do you need to clear the interrupt status here? This function will be called from a work queue sometime later after an interrupt has fired. It will also be called at random times when userspace enquires what the link parameters are, so clearing the interrupt here can result in lost link changes. > +static void xpcs_link_up_1000basex(struct dw_xpcs *xpcs, int speed, > + int duplex) > +{ > + int val, ret; > + > + switch (speed) { > + case SPEED_1000: > + val = BMCR_SPEED1000; > + break; > + case SPEED_100: > + case SPEED_10: > + default: > + pr_err("%s: speed = %d\n", __func__, speed); > + return; > + } > + > + if (duplex == DUPLEX_FULL) > + val |= BMCR_FULLDPLX; > + else > + pr_err("%s: half duplex not supported\n", __func__); > + > + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, val); > + if (ret) > + pr_err("%s: xpcs_write returned %pe\n", __func__, ERR_PTR(ret)); Does this need to be done even when AN is enabled? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!