Message ID | 20231220045228.27079-2-luizluca@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:24d3:b0:fb:cd0c:d3e with SMTP id r19csp2420442dyi; Tue, 19 Dec 2023 20:53:53 -0800 (PST) X-Google-Smtp-Source: AGHT+IERvWNQyCS/K8m6i02uX+3Md0FFR2TnFwGerb3BLV3YaP/a29Irmwuj7aJzUv+frba2tCtC X-Received: by 2002:a05:651c:2129:b0:2cc:6fa0:106a with SMTP id a41-20020a05651c212900b002cc6fa0106amr2958534ljq.24.1703048032864; Tue, 19 Dec 2023 20:53:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703048032; cv=none; d=google.com; s=arc-20160816; b=D+Py9Y3X6X03fblX4MhHZzD9onuXeCujqiUspgqSnsnad+1o1L8my8LZBMyDStkdIQ ipOmM91x/CfCPuP5ADQ/LmmPING3FykKlHuT/4tBzMGmjED2XfcUZjxGYoLuhzmDUJZh xt4VUIZhhlUbusNiWLDn7iWDpOVTLc3fHNgsdxAtJGZMzzjlkGVlIWjVUJPJvZ25hQaV GTeXhc4EMjQrU3Fa3KJ4J4Goy/wJeDOTNxNcwevhZN3TPPkCCQQBWPGKmUu/g3Z7bLxQ TksYOLjEaU6guXGbhHGBgcX2UEVPoVsoOpu8e6KodpVveUq/BwvIA1L5ivph02uKpvA1 zRag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=oCdzME/wc/7u1TobMBsXQV40LzKB+51jmYS/oXHHLXc=; fh=yDVdxtMybEcgRpZHCHYokJY/pmGNNOaH/RP4uiEfktY=; b=jAkCC7b3h6lq5dXdMaVw6Vxoo+th8lHcx+bupquC6r/M7rt3NrUrp8LmSvlzKmhxeL HNIW9plKHcmh2jwRuUzYGo+lTCVadMkFRspB7DsBUciZmDveMusovWiKTLFJpLG6dbhZ 327Xyod2nFv/zlOIyn1jm34F/D9s9QO3w1EzysfMFKmCBMRcaqY8n7VObYlMd1pJlhbx 9/Wd9d7qZlrvcdv8R7Go2P3J8FHazw7vEg0rt2z0gdsnFqU115gBIhHXcnPsqc37uB/j wPzUPoTvdTlH0FC+D7ytE8t1lUppH4X2dbIpB+sOGsYeLPP3bAN1aUrxKo6NwlFG+scV FpAQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=U1yXN7tr; spf=pass (google.com: domain of linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id s23-20020a50d497000000b00552ee29e05bsi3909550edi.308.2023.12.19.20.53.52 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Dec 2023 20:53:52 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=U1yXN7tr; spf=pass (google.com: domain of linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-6376-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 4E0E61F261A3 for <ouuuleilei@gmail.com>; Wed, 20 Dec 2023 04:53:52 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2ADC411C84; Wed, 20 Dec 2023 04:53:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="U1yXN7tr" X-Original-To: linux-kernel@vger.kernel.org Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 38FAAC2C3; Wed, 20 Dec 2023 04:53:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-1d3ec3db764so1714905ad.2; Tue, 19 Dec 2023 20:53:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1703048006; x=1703652806; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=oCdzME/wc/7u1TobMBsXQV40LzKB+51jmYS/oXHHLXc=; b=U1yXN7trYxo5D5Cxn2wMtvdgBW3I5iibYfDFs+EFVSw5RrbfgW0V2JnepSR/eknnuU EpsL1gq9zU6faocDbEF/Ti5rt/BJZ6sCf9LKtG6rp+EtS/AXtm154DOsdw1KiJGz/8+Q 90xmBL1sHW2UxzNBmw3aYdpGUwgpof+FhbTMA/Fl+6aZVcqpgnRmy0MKJIgc5xP97YJy F1wF+OcWP3zrw+0a0dThfuDTzdTIACAlYBAzUuGY2hZu+AvFIkVRkEnYjU9TMWyxGUmW k1MbWzy4dpmD0LefHPRdQvz5G56rv0fzHrQSKfEr2fLUcwx+zoBHRPiuohAygutq9bhC Nq5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703048006; x=1703652806; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oCdzME/wc/7u1TobMBsXQV40LzKB+51jmYS/oXHHLXc=; b=rE5XCdMQRrA3mriUyyQ+b2wzaQK1WiBM2T0NBYltjj3Z5GidlMHsjofugLiTpWnkM/ xgVKCwzyFXOgb6yYWrKEz9LPv43NDWG2emKDWMxj2ZsmyX4UBjq/piVFuho4XkI6+dNK cVvNbI5Xjb3NZpTonZ6QdPhqz/4MygRSNih8eYQVpwgAacpkTOYEgB2io7akENIKfbuY G/Is3ERQEP8KcC+x2DNaHQhgI/bhzcQrBdWpOGnGtCTFcxAZdYDoqKXAN+xpDfet5aeZ h4ebF38e6FMGTD3rLE444TNrJOHQEqKJx/0dVZtQba80iHEtAtFg4Ac7ga5I3n9sLA1E rLEA== X-Gm-Message-State: AOJu0YwmqkWB0eNs26ofu06VAp536AlnUby5Arj7eERb6aYSvKIk4thQ hFwqaAQT0BOK5nMhzSSkqCctq/ZqFT+DPimB X-Received: by 2002:a17:902:a58b:b0:1d0:8d57:482 with SMTP id az11-20020a170902a58b00b001d08d570482mr9234611plb.50.1703048006398; Tue, 19 Dec 2023 20:53:26 -0800 (PST) Received: from tresc054937.tre-sc.gov.br ([187.94.103.218]) by smtp.gmail.com with ESMTPSA id u7-20020a170902b28700b001cfcd2fb7b0sm3049318plr.285.2023.12.19.20.53.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Dec 2023 20:53:25 -0800 (PST) From: Luiz Angelo Daros de Luca <luizluca@gmail.com> To: netdev@vger.kernel.org Cc: andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, linux-kernel@vger.kernel.org, Luiz Angelo Daros de Luca <luizluca@gmail.com> Subject: [PATCH net-next] net: mdio: get/put device node during (un)registration Date: Wed, 20 Dec 2023 01:52:29 -0300 Message-ID: <20231220045228.27079-2-luizluca@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785775294468766996 X-GMAIL-MSGID: 1785775294468766996 |
Series |
[net-next] net: mdio: get/put device node during (un)registration
|
|
Commit Message
Luiz Angelo Daros de Luca
Dec. 20, 2023, 4:52 a.m. UTC
The __of_mdiobus_register() function was storing the device node in
dev.of_node without increasing its reference count. It implicitly relied
on the caller to maintain the allocated node until the mdiobus was
unregistered.
Now, __of_mdiobus_register() will acquire the node before assigning it,
and of_mdiobus_unregister_callback() will be called at the end of
mdio_unregister().
Drivers can now release the node immediately after MDIO registration.
Some of them are already doing that even before this patch.
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
drivers/net/mdio/of_mdio.c | 12 +++++++++++-
drivers/net/phy/mdio_bus.c | 3 +++
include/linux/phy.h | 3 +++
3 files changed, 17 insertions(+), 1 deletion(-)
Comments
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Wed, 20 Dec 2023 01:52:29 -0300 you wrote: > The __of_mdiobus_register() function was storing the device node in > dev.of_node without increasing its reference count. It implicitly relied > on the caller to maintain the allocated node until the mdiobus was > unregistered. > > Now, __of_mdiobus_register() will acquire the node before assigning it, > and of_mdiobus_unregister_callback() will be called at the end of > mdio_unregister(). > > [...] Here is the summary with links: - [net-next] net: mdio: get/put device node during (un)registration https://git.kernel.org/netdev/net-next/c/cff9c565e65f You are awesome, thank you!
On Wed, Dec 20, 2023 at 01:52:29AM -0300, Luiz Angelo Daros de Luca wrote: > The __of_mdiobus_register() function was storing the device node in > dev.of_node without increasing its reference count. It implicitly relied > on the caller to maintain the allocated node until the mdiobus was > unregistered. > > Now, __of_mdiobus_register() will acquire the node before assigning it, > and of_mdiobus_unregister_callback() will be called at the end of > mdio_unregister(). > > Drivers can now release the node immediately after MDIO registration. > Some of them are already doing that even before this patch. > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> I don't like this, certainly not the use of a method prefixed by a double-underscore, and neither the conditional nature of "putting" this. That alone seems to point to there being more issues. I also notice that netdev have applied this without *any* review from phylib maintainers. Grr. Indeed there are more issues with the refcounting here. If one looks at drivers/net/phy/mdio_bus.c::of_mdiobus_link_mdiodev(), we find this: if (addr == mdiodev->addr) { device_set_node(dev, of_fwnode_handle(child)); /* The refcount on "child" is passed to the mdio * device. Do _not_ use of_node_put(child) here. */ return; but there is nowhere that this refcount is dropped. Really, the patch should be addressing the problem rather than putting a sticky-plaster over just one instance of it.
> On Wed, Dec 20, 2023 at 01:52:29AM -0300, Luiz Angelo Daros de Luca wrote: > > The __of_mdiobus_register() function was storing the device node in > > dev.of_node without increasing its reference count. It implicitly relied > > on the caller to maintain the allocated node until the mdiobus was > > unregistered. > > > > Now, __of_mdiobus_register() will acquire the node before assigning it, > > and of_mdiobus_unregister_callback() will be called at the end of > > mdio_unregister(). > > > > Drivers can now release the node immediately after MDIO registration. > > Some of them are already doing that even before this patch. > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > I don't like this, certainly not the use of a method prefixed by a > double-underscore, and neither the conditional nature of "putting" > this. That alone seems to point to there being more issues. Thanks Russel. At least one driver (bcm_sf2_mdio_register) is writing directly to the mii_bus->dev.of_node and not using of_mdiobus_register(). We should not put a node in the MDIO bus if the bus didn't get it before. That's the reason for the conditional putting the node. I wasn't sure about the names. What would be an appropriate name? The same without the prefix? In order to put the node only when the bus was registered by __of_mdiobus_register, I opted for a callback but it might be a better approach. > I also notice that netdev have applied this without *any* review from > phylib maintainers. Grr. Some reviews are required. Should we revert it? > Indeed there are more issues with the refcounting here. If one looks at > drivers/net/phy/mdio_bus.c::of_mdiobus_link_mdiodev(), we find this: > > if (addr == mdiodev->addr) { > device_set_node(dev, of_fwnode_handle(child)); > /* The refcount on "child" is passed to the mdio > * device. Do _not_ use of_node_put(child) here. > */ > return; > > but there is nowhere that this refcount is dropped. The same file where we have the get should also contain the put, ideally in a reverse function like register/unregister. It is too easy to miss a put that should happen in a different context. fixed_phy_unregister seems to be one case where it put that node after phy_device_remove() but I didn't investigate it further if that was related to a different of_node_get. mdiobus_unregister_device might be a nice place to fit that put but I'm not an expert in MDIO API. > Really, the patch should be addressing the problem rather than putting > a sticky-plaster over just one instance of it. I'm trying to address an issue I ran into while modifying a DSA driver. We have drivers putting the node passed to of_mdiobus_register just after it returns. In my option, it feels more natural and this patch fixes that scenario. Other drivers keep that reference until the driver is removed, which might still be too soon without this patch. I guess putting the node should happen between mdiobus_unregister and mdiobus_free. If the driver uses devm variants, it does not control the code between those two methods and it should just hope that it is enough to put the node as its last step. I issue that the child node you pointed to should also be addressed. However, I think they are two different but related issues. Any place we see a device_set_node(), we should see a of_node_get before and a of_node_put when the device is gone. Regards, Luiz
On Tue, 2 Jan 2024 18:57:35 -0300 Luiz Angelo Daros de Luca wrote: > > I also notice that netdev have applied this without *any* review from > > phylib maintainers. Grr. > > Some reviews are required. Should we revert it? Reverted.
On Tue, Jan 02, 2024 at 06:57:35PM -0300, Luiz Angelo Daros de Luca wrote: > > On Wed, Dec 20, 2023 at 01:52:29AM -0300, Luiz Angelo Daros de Luca wrote: > > > The __of_mdiobus_register() function was storing the device node in > > > dev.of_node without increasing its reference count. It implicitly relied > > > on the caller to maintain the allocated node until the mdiobus was > > > unregistered. > > > > > > Now, __of_mdiobus_register() will acquire the node before assigning it, > > > and of_mdiobus_unregister_callback() will be called at the end of > > > mdio_unregister(). > > > > > > Drivers can now release the node immediately after MDIO registration. > > > Some of them are already doing that even before this patch. > > > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > > > I don't like this, certainly not the use of a method prefixed by a > > double-underscore, and neither the conditional nature of "putting" > > this. That alone seems to point to there being more issues. > > Thanks Russel. Hi Lewis, > At least one driver (bcm_sf2_mdio_register) is writing directly to the > mii_bus->dev.of_node and not using of_mdiobus_register(). We should > not put a node in the MDIO bus if the bus didn't get it before. That's > the reason for the conditional putting the node. I agree with the idea that a node placed in a bus needs to have it's reference count increased before hand, _unless_ the reference is being passed from the code registering. What I don't agree with is the conditional putting of the node. What I think should have happened is a review of all the code, and either a justification needed to be put forward (and considered *before* this patch was merged) about why to do this conditionally, _or_ all the places where the refcounting is not correct get fixed at the same time. Adding this conditional mechanism adds more complexity which makes the situation more difficult to analyse and fix later. > I wasn't sure about the names. What would be an appropriate name? The > same without the prefix? In order to put the node only when the bus > was registered by __of_mdiobus_register, I opted for a callback but it > might be a better approach. Normally, the callback is just named "release". > > I also notice that netdev have applied this without *any* review from > > phylib maintainers. Grr. > > Some reviews are required. Should we revert it? Clearly reviews are needed, even more so as there is indeed an issue with this patch. Looking at __of_mdiobus_register(), let's assume __mdiobus_register() succeeds. While scanning the PHYs, we hit an error that calls us to head to the unregister label. This calls mdiobus_unregister(), which calls your bus->__unregister_callback function, which puts the node. When that returns, we continue past the "put_node" label, which does *another* of_node_put() on the same node. So, this patch has traded a lack-of-get for a double-put bug. Given that it wasn't reviewed before being applied, and I think we can do much better, I am definitely in the mindset that it should be reverted. > > Indeed there are more issues with the refcounting here. If one looks at > > drivers/net/phy/mdio_bus.c::of_mdiobus_link_mdiodev(), we find this: > > > > if (addr == mdiodev->addr) { > > device_set_node(dev, of_fwnode_handle(child)); > > /* The refcount on "child" is passed to the mdio > > * device. Do _not_ use of_node_put(child) here. > > */ > > return; > > > > but there is nowhere that this refcount is dropped. > > The same file where we have the get should also contain the put, > ideally in a reverse function like register/unregister. Not necessarily true. There are cases where we need the node to hang around until the device is actually released, so putting the node in the release callback for the device tends to be the best place. The rule for all devices of that class then becomes that the node must be "got" before assigning them to the device which then becomes easy to audit. > I'm trying to address an issue I ran into while modifying a DSA > driver. We have drivers putting the node passed to of_mdiobus_register > just after it returns. In my option, it feels more natural and this > patch fixes that scenario. I agree with that approach, but as you rightly point out, we need MDIO to behave correctly, and I don't think that patching just one bit of MDIO to fix this mess is the right approach. Jakub: please revert, if that's still possible.
On Wed, Jan 03, 2024 at 10:22:00AM +0000, Russell King (Oracle) wrote: > I agree with that approach, but as you rightly point out, we need MDIO > to behave correctly, and I don't think that patching just one bit of > MDIO to fix this mess is the right approach. This is probably a safer approach to ensuring that the firmware data reference count isn't dropped while the bus exists byensuring that we always take a reference at register time. It also likely fixes similar issues with ACPI and swnode based users as well. It doesn't deal with the excess-refcount problem, as with this approach the two issues are entirely independent of each other. Please test to check that this addresses your issue. Thanks. diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 6cf73c15635b..afbad1ad8683 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -193,6 +193,10 @@ static void mdiobus_release(struct device *d) bus->state != MDIOBUS_ALLOCATED, "%s: not in RELEASED or ALLOCATED state\n", bus->id); + + if (bus->state == MDIOBUS_RELEASED) + fwnode_handle_put(dev_fwnode(d)); + kfree(bus); } @@ -684,6 +688,15 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) bus->dev.groups = NULL; dev_set_name(&bus->dev, "%s", bus->id); + /* If the bus state is allocated, we're registering a fresh bus + * that may have a fwnode associated with it. Grab a reference + * to the fwnode. This will be dropped when the bus is released. + * If the bus was set to unregistered, it means that the bus was + * previously registered, and we've already grabbed a reference. + */ + if (bus->state == MDIOBUS_ALLOCATED) + fwnode_handle_get(dev_fwnode(&bus->dev)); + /* We need to set state to MDIOBUS_UNREGISTERED to correctly release * the device in mdiobus_free() *
> Please test to check that this addresses your issue. Thanks. > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 6cf73c15635b..afbad1ad8683 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -193,6 +193,10 @@ static void mdiobus_release(struct device *d) > bus->state != MDIOBUS_ALLOCATED, > "%s: not in RELEASED or ALLOCATED state\n", > bus->id); > + > + if (bus->state == MDIOBUS_RELEASED) > + fwnode_handle_put(dev_fwnode(d)); > + > kfree(bus); > } > > @@ -684,6 +688,15 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) > bus->dev.groups = NULL; > dev_set_name(&bus->dev, "%s", bus->id); > > + /* If the bus state is allocated, we're registering a fresh bus > + * that may have a fwnode associated with it. Grab a reference > + * to the fwnode. This will be dropped when the bus is released. > + * If the bus was set to unregistered, it means that the bus was > + * previously registered, and we've already grabbed a reference. > + */ > + if (bus->state == MDIOBUS_ALLOCATED) > + fwnode_handle_get(dev_fwnode(&bus->dev)); > + > /* We need to set state to MDIOBUS_UNREGISTERED to correctly release > * the device in mdiobus_free() > * > -- Thanks Russel. It is much better than my approach. You simply get/put during registration/unregistration when a node is defined, no matter who defined it (of_mdiobus_register or anything else). Clean and simple. Regards, Luiz
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c index 64ebcb6d235c..9b6cab6154e0 100644 --- a/drivers/net/mdio/of_mdio.c +++ b/drivers/net/mdio/of_mdio.c @@ -139,6 +139,11 @@ bool of_mdiobus_child_is_phy(struct device_node *child) } EXPORT_SYMBOL(of_mdiobus_child_is_phy); +static void __of_mdiobus_unregister_callback(struct mii_bus *mdio) +{ + of_node_put(mdio->dev.of_node); +} + /** * __of_mdiobus_register - Register mii_bus and create PHYs from the device tree * @mdio: pointer to mii_bus structure @@ -166,6 +171,8 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np, * the device tree are populated after the bus has been registered */ mdio->phy_mask = ~0; + mdio->__unregister_callback = __of_mdiobus_unregister_callback; + of_node_get(np); device_set_node(&mdio->dev, of_fwnode_handle(np)); /* Get bus level PHY reset GPIO details */ @@ -177,7 +184,7 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np, /* Register the MDIO bus */ rc = __mdiobus_register(mdio, owner); if (rc) - return rc; + goto put_node; /* Loop over the child nodes and register a phy_device for each phy */ for_each_available_child_of_node(np, child) { @@ -237,6 +244,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np, unregister: of_node_put(child); mdiobus_unregister(mdio); + +put_node: + of_node_put(np); return rc; } EXPORT_SYMBOL(__of_mdiobus_register); diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 25dcaa49ab8b..1229b8e4c53b 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -787,6 +787,9 @@ void mdiobus_unregister(struct mii_bus *bus) gpiod_set_value_cansleep(bus->reset_gpiod, 1); device_del(&bus->dev); + + if (bus->__unregister_callback) + bus->__unregister_callback(bus); } EXPORT_SYMBOL(mdiobus_unregister); diff --git a/include/linux/phy.h b/include/linux/phy.h index e5f1f41e399c..2b383da4d825 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -433,6 +433,9 @@ struct mii_bus { /** @shared: shared state across different PHYs */ struct phy_package_shared *shared[PHY_MAX_ADDR]; + + /** @__unregister_callback: called at the last step of unregistration */ + void (*__unregister_callback)(struct mii_bus *bus); }; #define to_mii_bus(d) container_of(d, struct mii_bus, dev)