From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 44AFE10947 for ; Wed, 2 Aug 2023 23:00:33 +0000 (UTC) Received: from pandora.armlinux.org.uk (unknown [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4900D1704; Wed, 2 Aug 2023 16:00:31 -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=GtHyUyRhmZC/ndCTfFZSi0G5xoy+tfRuoFtXp1YsIVM=; b=JtvkveQpSmLkLcNOVxyh/y4nHb eTYMyA6kStyZj5Yy/jFPb2xF7atWrkAKg1yr4gKtF8N8z1KdGkUi36hjqtQvmKvw6ONUqa/oTe+JD kt6/EsmDPjRYPCMRIJEGusOt7SGcjY0ijKhia2F2Xyix5ogILECyKSBVNJ10l8l70oMpqbX/LjLH1 GGp1uZbfwA8mYOp/WHGLLALFPNi1u+TpE5W80l9dyHQzokAsEjgfyZ6lgHuQBp4SrblStNuUiisxc D5qs27eB3aem//Yq4F4NcLvBO9yCrJASk7v3KcFR+aJfufpkez0Xb8sXT03tCjiNNDKzMUpiZyAot VZVb8eyg==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:42338) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1qRKp8-0005zV-0u; Thu, 03 Aug 2023 00:00:11 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1qRKp0-0002HP-CR; Thu, 03 Aug 2023 00:00:02 +0100 Date: Thu, 3 Aug 2023 00:00:02 +0100 From: "Russell King (Oracle)" To: Michael Walle Cc: Andrew Lunn , Heiner Kallweit , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Yisen Zhuang , Salil Mehta , Florian Fainelli , Broadcom internal kernel review list , Marek =?iso-8859-1?Q?Beh=FAn?= , Xu Liang , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Simon Horman Subject: Re: [PATCH net-next v3 02/11] net: phy: introduce phy_has_c45_registers() Message-ID: References: <20230620-feature-c45-over-c22-v3-2-9eb37edf7be0@kernel.org> <7be8b305-f287-4e99-bddd-55646285c427@lunn.ch> <867ae3cc05439599d63e4712bca79e27@kernel.org> <3fa8d14f0a989af971d61af01b13fd8b@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3fa8d14f0a989af971d61af01b13fd8b@kernel.org> Sender: Russell King (Oracle) X-Spam-Status: No, score=-1.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RDNS_NONE,SPF_HELO_NONE, SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net On Wed, Aug 02, 2023 at 07:11:27PM +0200, Michael Walle wrote: > I'm talking about > > u32 mmd_mask = MDIO_DEVS_PMAPMD | MDIO_DEVS_AN; > if (!phydev->is_c45 || > (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask) > return -ENODEV; > > How should that look like after this series? In the case of the marvell10g driver, at least the 88x3310 can be accessed via clause 45 bus cycles _or_ the clause 45 indirect register access via clause 22. I'm not sure what the clause 22 registers would contain, whether they contain valid values with the exception of the clause 45 indirect access registers because there's not enough information in the documentation, and I can't access the clause 22 registers on real hardware. Another issue is this PHY has two different ID values. One is 0x002b 0x09aa, the other is 0x0141 0x0daa. One or other of these values appears in the MMDs - in other words, they are not consistently used. This means it's impossible to guess what value may be in the clause 22 ID registers for this PHY. However, given the way phylib works, the above effectively ensures that we detected the PHY using clause 45 accesses, and then goes on to verify that we do indeed have the PMAPMD MMD and the AN MMD. Effectively, it ensures that get_phy_c45_ids() was used to detect the device. So, in effect, this code is ensuring that we discovered the PHY using clause 45 accesses, and that at a minimum the PHY also indicates that the PMAPMD and AN MMDs are implemented. For the bcm84881 driver, it's a similar situation - that's only ever been used with a bus that supports _only_ clause 45 accesses. Even less idea whether the PHY could respond to clause 22 accesses as there is no information available on the PHY. So, I'd suggest both of these are the same - returns -ENODEV if the bus doesn't support clause 45 transfers or if the two MMDs are not indicated. That said, it _can_ be simplified down to just testing the devices_in_package member, because that will only be non-zero if we successfully probed the PHY via some accessible way to the clause 45 registers. So: if ((phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask) return -ENODEV; is probably entirely sufficient in both cases. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!