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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B815AC27C7A for ; Thu, 17 Aug 2023 13:06:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=YsyW7pWhTplEnsdTbWQF834eS1hSeNJTNVA8NBFmNZU=; b=hqNlQ/F81rXijG 62BCnBh39866jClZeoVo0tdes8cK0b+J6YAz1aTwm9hxigJb06gytqg5mFOjQ+Yt1SJIyraHx90NN nqNQpGN5ubHptDCQMz2yp8tUQvtwtCFdyNHdBZ0Xgo5YqtvKl5sJ3r6syWv9Vt8QYGmPLlit+qG1M bgd3OJWwEVn0pL+5kXjlIcerJWpp6DZzUpJY6Gi6BeELLW1d/b+nkMwHFtRATqiQ7xTfAg+A/TUAX PmPzl2jnsrFEGLN2s4dvZfcT6Cj3Nhl5XBMJBv0LR09hsGYjK5UNUNQWIbeJiNwNENnPUlCJN3ZTt phsj1y4nom77r5Qb5oeg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qWchQ-006QVY-2M; Thu, 17 Aug 2023 13:06:04 +0000 Received: from [2001:4d48:ad52:32c8:5054:ff:fe00:142] (helo=pandora.armlinux.org.uk) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qWchM-006QPe-2i; Thu, 17 Aug 2023 13:06:03 +0000 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=2O+oMCc4tCMfBiIfp7tXuumeCBqFXKjABtwXyZskEPw=; b=xGhRgbFnSSNHr2vcTGi9qICnPQ wnFJyD4mcsojgjl7m4A6TH6Vbd2KqJRj9YCbHhk1+a7ohdxeMUyW8J9EpgS8n/y+AOlxbHy//a3fq RzchlP+XTJE+uRAtv2yoyRwUu78eUUuoJ72uw3Qkd5/0DgZz8BVWZFtOs4+cpNSb8cNvwTrNoBYCX C/2YY8tb70oZjsIIoWwE1KUdu78y4obXNDEMxzsUvDr7ho1kzBUvfobuL3x4BwLR90wgq5GD+LYNp 2+smJFaErreGkcpezH0dGQJqyVnw3ADQIa1R3mZWW6YTQHZSI/RyvhVd5ZaWiFMJruaqfIkfZ1oFD P+z76Ftw==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:46006) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1qWcf7-0004CA-2Y; Thu, 17 Aug 2023 14:03:41 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1qWcf6-0000cG-FX; Thu, 17 Aug 2023 14:03:40 +0100 Date: Thu, 17 Aug 2023 14:03:40 +0100 From: "Russell King (Oracle)" To: Daniel Golle Cc: Alexander Couzens , Andrew Lunn , Heiner Kallweit , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Matthias Brugger , AngeloGioacchino Del Regno , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH net-next] net: pcs: lynxi: fully reconfigure if link is down Message-ID: References: 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-20230817_060600_903837_08DB0F57 X-CRM114-Status: GOOD ( 22.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Thu, Aug 17, 2023 at 01:04:06PM +0100, Daniel Golle wrote: > On MT7988 When switching from 10GBase-R/5GBase-R/USXGMII to one of the > interface modes provided by mtk-pcs-lynxi we need to make sure to > always perform a full configuration of the PHYA. > As the idea behind not doing that was mostly to prevent an existing link > going down without any need for it to do so. Hence we can just always > perform a full confinguration in case the link is down. And this is racy - because in the case with inband signalling, the link can come up between reading the status and acting on it. It could even be already up, but the link status indicates it is not. Lastly, reading the BMSR has side effects: the link status bit latches low until a read. Basically, do not read the BMSR here, it's buggy to read it any place other than pcs_get_state. I think what we need to do instead are: 1) mtk_mac_select_pcs() returns the SGMII PCS or NULL. Presumably this is the driver which supports 10GBase-R/5GBase-R/USXGMII, and thus this returns NULL for 10GBase-R/5GBase-R/USXGMII. Phylink doesn't cater for mac_select_pcs() returning non-NULL for some modes and NULL for others, mainly because the presence of a PCS _used_ to cause phylink to change its behaviour (see https://lore.kernel.org/netdev/YZRLQqLblRurUd4V@shell.armlinux.org.uk/). That has now changed (we've got rid of the legacy stuff at last!) so there is no technical reason not to now allow that. Vladimir did have some arguments for not allowing it when we had the phylink_set_pcs() interface: https://lore.kernel.org/netdev/20211123181515.qqo7e4xbuu2ntwgt@skbuf/ I'm assuming that your requirement now provides sufficient justification for allowing this. There is one bug that does need fixing first: phylink_change_inband_advert() checks pl->pcs->neg_mode without first checking whether pl->pcs is non-NULL. To allow this, phylink_major_config() needs: pcs_changed = pcs && pl->pcs != pcs; to become: pcs_changed = pl->pcs != pcs; 2) with (1) solved, there are a couple of callbacks that can be used to solve this - I think pcs_disable() is the one you want, which will be called when we switch to a mode where _this_ PCS will no longer be used (thus you can reset mpcs->interface to _NA, ready for when it is next brought into use.) Would that work for you? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel