Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932122AbdHVAqo (ORCPT ); Mon, 21 Aug 2017 20:46:44 -0400 Received: from mail-qt0-f195.google.com ([209.85.216.195]:37042 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754252AbdHVAqn (ORCPT ); Mon, 21 Aug 2017 20:46:43 -0400 Subject: Re: [PATCH v2 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print To: Andrew Lunn , Romain Perier Cc: Giuseppe Cavallaro , Alexandre Torgue , netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20170821114530.13706-1-romain.perier@collabora.com> <20170821114530.13706-3-romain.perier@collabora.com> <20170821142426.GF1703@lunn.ch> From: Florian Fainelli Message-ID: <0df9cbd4-30d8-2a30-07ee-bd0b38bd606b@gmail.com> Date: Mon, 21 Aug 2017 17:46:39 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170821142426.GF1703@lunn.ch> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1727 Lines: 47 On 08/21/2017 07:24 AM, Andrew Lunn wrote: > On Mon, Aug 21, 2017 at 01:45:30PM +0200, Romain Perier wrote: >> Currently, if this logging function is used prior the phy driver is >> bound to the phy device (that is usually done from .ndo_open), >> 'phydev->drv' might be NULL, resulting in a kernel crash. That is >> typically the case in the stmmac driver, info about the phy is displayed >> during the registration of the MDIO bus, and then genphy driver is bound >> to this phydev when .ndo_open is called. >> >> This commit fixes the issue by using the right genphy driver, when >> phydev->drv is NULL. >> >> Fixes: fbca164776e4 ("net: stmmac: Use the right logging functi") >> Signed-off-by: Romain Perier >> --- >> drivers/net/phy/phy_device.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index 1790f7fec125..6af6dc6dfeaf 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -864,15 +864,24 @@ EXPORT_SYMBOL(phy_attached_info); >> #define ATTACHED_FMT "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)" >> void phy_attached_print(struct phy_device *phydev, const char *fmt, ...) >> { >> + struct phy_driver *drv = phydev->drv; >> + >> + if (!drv) { >> + if (phydev->is_c45) >> + drv = &genphy_10g_driver; >> + else >> + drv = &genphy_driver; >> + } >> + > > As i said in my comment to v1, i don't like this. Agreed, just use Andrew's earlier suggestion of checking phydev->drv validity. We don't have an equivalent of "unregistered" in the PHY layer, but "unbound" seems like it could be what we want here. Thanks -- Florian