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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3ADF0C433F5 for ; Thu, 30 Sep 2021 13:54:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1BB1660E75 for ; Thu, 30 Sep 2021 13:54:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351532AbhI3N4B (ORCPT ); Thu, 30 Sep 2021 09:56:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55868 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351468AbhI3N4A (ORCPT ); Thu, 30 Sep 2021 09:56:00 -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 0AD01C06176A for ; Thu, 30 Sep 2021 06:54:18 -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-Transfer-Encoding:Content-Type: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-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=qDLAtsiT33ruB17Tbww9QwVKnCmJB/88Z6LJsxX0VDY=; b=nJ5OztL8Wwm26pKkWZ0EY8i9X6 aXB5Py39fMBWyoutWjE306bIU1viCB+WRdDinc4iOlDplxd48prG3roGumbfh2yeg4Tkcizd9lJiA +ejHonZgkT7CyQ6wT0jU/jIXN8gJwQPitDBq4t7YNB2DYZehj6xOCt+WOZ083ZYDuD0c3CYL2R3I1 6MTfkRsUCqNgY+IfbVBmf8fDm+wojIhqZhv/EBU4j92MSIwWKJPlK3lflxL/ISc00ER9NDpWfvCB9 yG/WKna0rq7bmTDpKnIduIcsr7b23oNIQoMffosFBl/NySB0ObM49xtqWyIO8L6nk8u8FzfLC19an AWC1nNgg==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:54874) 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 1mVwVs-0003Z1-B7; Thu, 30 Sep 2021 14:54:16 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1mVwVr-0003hr-QJ; Thu, 30 Sep 2021 14:54:15 +0100 Date: Thu, 30 Sep 2021 14:54:15 +0100 From: "Russell King (Oracle)" To: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: Andrew Lunn , Heiner Kallweit , Network Development , Florian Fainelli , BCM Kernel Feedback , Vivek Unune Subject: Re: Lockup in phy_probe() for MDIO device (Broadcom's switch) Message-ID: References: <5715f818-a279-d514-dcac-73a94c1d30ef@gmail.com> <1e4e40ba-23b8-65b4-0b53-1c8393d9a834@gmail.com> <955416fe-4da4-b1ec-aadb-9b816f02d7f2@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: Russell King (Oracle) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Sep 30, 2021 at 03:42:38PM +0200, Rafał Miłecki wrote: > On 30.09.2021 15:07, Russell King (Oracle) wrote: > > On Thu, Sep 30, 2021 at 02:51:40PM +0200, Rafał Miłecki wrote: > > > On 30.09.2021 14:30, Russell King (Oracle) wrote: > > > > > It's actually OpenWrt's downstream swconfig-based b53 driver that > > > > > matches this device. > > > > > > > > > > I'm confused as downstream b53_mdio.c calls phy_driver_register(). Why > > > > > does it match MDIO device then? I thought MDIO devices should be > > > > > matches only with drivers using mdio_driver_register(). > > > > > > > > Note that I've no idea what he swconfig-based b53 driver looks like, > > > > I don't have the source for that to hand. > > > > > > > > If it calls phy_driver_register(), then it is registering a driver for > > > > a MDIO device wrapped in a struct phy_device. If this driver has a > > > > .of_match_table member set, then this is wrong - the basic rule is > > > > > > > > PHY drivers must never match using DT compatibles. > > > > > > > > because this is exactly what will occur - it bypasses the check that > > > > the mdio_device being matched is in fact wrapped by a struct phy_device, > > > > and we will access members of the non-existent phy_device, including > > > > the "uninitialised" mutex. > > > > > > > > If the swconfig-based b53 driver does want to bind to a phy_device based > > > > DT node, then it needs to match using either a custom .match_phy_device > > > > method in the PHY driver, or it needs to match using the PHY IDs. > > > > > > Sorry, I should have linked downstream b53_mdio.c . It's: > > > https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/files/drivers/net/phy/b53/b53_mdio.c;h=98cdbffe73c7354f4401389dfcc96014bff62588;hb=HEAD > > > > Yes, I just found a version of the driver > > > > > You can see that is *uses* of_match_table. > > > > > > What about refusing bugged drivers like above b53 with something like: > > > > That will break all the MDIO based DSA and other non-PHY drivers, > > sorry. > > > > I suppose we could detect if the driver has the MDIO_DEVICE_IS_PHY flag > > set, and reject any device that does not have MDIO_DEVICE_IS_PHY set: > > > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > > index 53f034fc2ef7..7bc06126ce10 100644 > > --- a/drivers/net/phy/mdio_bus.c > > +++ b/drivers/net/phy/mdio_bus.c > > @@ -939,6 +939,12 @@ EXPORT_SYMBOL_GPL(mdiobus_modify); > > static int mdio_bus_match(struct device *dev, struct device_driver *drv) > > { > > struct mdio_device *mdio = to_mdio_device(dev); > > + struct mdio_driver *mdiodrv = to_mdio_driver(drv); > > + > > + /* Both the driver and device must type-match */ > > + if (!(mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY) == > > + !(mdio->flags & MDIO_DEVICE_FLAG_PHY)) > > + return 0; > > if (of_driver_match_device(dev, drv)) > > return 1; > > > > In OpenWrt & bugged b53 case we have: > 1. Device without MDIO_DEVICE_FLAG_PHY > 2. Driver with MDIO_DEVICE_IS_PHY > > I think the logic should be to return 0 on mismatch (reverted). I assume you mean the test should've been != not == - then yes, you are absolutely correct. Sorry, I'm still not over a cold. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!