From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH net] net: phy: sfp: Fix unregistering of HWMON SFP device Date: Mon, 24 Sep 2018 16:50:19 -0700 Message-ID: References: <1537828692-22861-1-git-send-email-andrew@lunn.ch> <20180924234644.GE17444@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Miller , netdev , Russell King To: Andrew Lunn Return-path: Received: from mail-ed1-f66.google.com ([209.85.208.66]:35295 "EHLO mail-ed1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725745AbeIYFzN (ORCPT ); Tue, 25 Sep 2018 01:55:13 -0400 Received: by mail-ed1-f66.google.com with SMTP id y21-v6so5516281edr.2 for ; Mon, 24 Sep 2018 16:50:30 -0700 (PDT) In-Reply-To: <20180924234644.GE17444@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 09/24/2018 04:46 PM, Andrew Lunn wrote: > On Mon, Sep 24, 2018 at 03:43:33PM -0700, Florian Fainelli wrote: >> On 09/24/2018 03:38 PM, Andrew Lunn wrote: >>> A HWMON device is only registered is the SFP module supports the >>> diagnostic page and is complient to SFF8472. Don't unconditionally >>> unregister the hwmon device when the SFP module is remove, otherwise >>> we access data structures which don't exist. >>> >>> Reported-by: Florian Fainelli >>> Fixes: 1323061a018a ("net: phy: sfp: Add HWMON support for module sensors") >>> Signed-off-by: Andrew Lunn >>> --- >>> drivers/net/phy/sfp.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c >>> index 52fffb98fde9..23705ffae6bb 100644 >>> --- a/drivers/net/phy/sfp.c >>> +++ b/drivers/net/phy/sfp.c >>> @@ -1098,8 +1098,11 @@ static int sfp_hwmon_insert(struct sfp *sfp) >>> >>> static void sfp_hwmon_remove(struct sfp *sfp) >>> { >>> - hwmon_device_unregister(sfp->hwmon_dev); >>> - kfree(sfp->hwmon_name); >>> + if (!PTR_ERR_OR_NULL(sfp->hwmon_dev)) { >> >> I believe this should be IS_ERR_OR_NULL(), there is no PTR_ERR_OR_NULL() >> in the Linux tree AFAICT. > > Yep. > > Please ignore this patch. After fixing this locally, this worked, feel free to add a Tested-by: Florian Fainelli to your v2. Thanks for the fix! -- Florian