* [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
@ 2023-06-22 10:31 Florian Fainelli
2023-06-22 15:36 ` Andrew Lunn
2023-06-24 2:00 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 5+ messages in thread
From: Florian Fainelli @ 2023-06-22 10:31 UTC (permalink / raw)
To: netdev
Cc: andrew, hkallweit1, ansuelsmth, rmk+kernel, Florian Fainelli,
Doug Berger, Broadcom internal kernel review list,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
open list
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]
With support for Ethernet PHY LEDs having been added, while
unregistering a MDIO bus and its child device liks PHYs there may be
"late" accesses to the MDIO bus. One typical use case is setting the PHY
LEDs brightness to OFF for instance.
We need to ensure that the MDIO bus controller remains entirely
functional since it runs off the main GENET adapter clock.
Link: https://lore.kernel.org/all/20230617155500.4005881-1-andrew@lunn.ch/
Fixes: 9a4e79697009 ("net: bcmgenet: utilize generic Broadcom UniMAC MDIO controller driver")
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index be042905ada2..c1d670c92958 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -668,5 +668,7 @@ void bcmgenet_mii_exit(struct net_device *dev)
if (of_phy_is_fixed_link(dn))
of_phy_deregister_fixed_link(dn);
of_node_put(priv->phy_dn);
+ clk_prepare_enable(priv->clk);
platform_device_unregister(priv->mii_pdev);
+ clk_disable_unprepare(priv->clk);
}
--
2.34.1
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
2023-06-22 10:31 [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled Florian Fainelli
@ 2023-06-22 15:36 ` Andrew Lunn
2023-06-22 15:50 ` Florian Fainelli
2023-06-24 2:00 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2023-06-22 15:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, hkallweit1, ansuelsmth, rmk+kernel, Doug Berger,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, open list
On Thu, Jun 22, 2023 at 03:31:07AM -0700, Florian Fainelli wrote:
> With support for Ethernet PHY LEDs having been added, while
> unregistering a MDIO bus and its child device liks PHYs there may be
> "late" accesses to the MDIO bus. One typical use case is setting the PHY
> LEDs brightness to OFF for instance.
>
> We need to ensure that the MDIO bus controller remains entirely
> functional since it runs off the main GENET adapter clock.
So this clock is enabled in bcmgenet_open() and disabled in
bcmgenet_close(). The assumption being, the MDIO bus is only used when
the interface is up.
How does this work when there is an MDIO based switch attached? I had
similar problems with the FEC and mv88e6xxx. DSA would try to talk to
the switch with the master interface down, and MDIO would time out. I
needed to add runtime PM support to the MDIO bus ops.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
2023-06-22 15:36 ` Andrew Lunn
@ 2023-06-22 15:50 ` Florian Fainelli
2023-06-22 19:14 ` Andrew Lunn
0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2023-06-22 15:50 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, hkallweit1, ansuelsmth, rmk+kernel, Doug Berger,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, open list
[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]
On 6/22/2023 4:36 PM, Andrew Lunn wrote:
> On Thu, Jun 22, 2023 at 03:31:07AM -0700, Florian Fainelli wrote:
>> With support for Ethernet PHY LEDs having been added, while
>> unregistering a MDIO bus and its child device liks PHYs there may be
>> "late" accesses to the MDIO bus. One typical use case is setting the PHY
>> LEDs brightness to OFF for instance.
>>
>> We need to ensure that the MDIO bus controller remains entirely
>> functional since it runs off the main GENET adapter clock.
>
> So this clock is enabled in bcmgenet_open() and disabled in
> bcmgenet_close(). The assumption being, the MDIO bus is only used when
> the interface is up.
>
> How does this work when there is an MDIO based switch attached? I had
> similar problems with the FEC and mv88e6xxx. DSA would try to talk to
> the switch with the master interface down, and MDIO would time out. I
> needed to add runtime PM support to the MDIO bus ops.
We do not have that configuration to support today, and given the way
that we do register the MDIO bus, it could actually be a bit challenging
to support DSA here, I might still have a board around to test, one day.
Passing the clock to the MDIO driver does require quite a bit of
restructuring in the driver such that the clock is only acquired around
mii_bus::write and read operations, otherwise the clock remains
constantly enabled, even if the network device is brought down, which
burns power unnecessarily.
Since this is a fix, I went with the more targeted approach here.
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
2023-06-22 15:50 ` Florian Fainelli
@ 2023-06-22 19:14 ` Andrew Lunn
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2023-06-22 19:14 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, hkallweit1, ansuelsmth, rmk+kernel, Doug Berger,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, open list
> Since this is a fix, I went with the more targeted approach here.
Yes, this is fine for stable.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
2023-06-22 10:31 [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled Florian Fainelli
2023-06-22 15:36 ` Andrew Lunn
@ 2023-06-24 2:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-24 2:00 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, andrew, hkallweit1, ansuelsmth, rmk+kernel, opendmb,
bcm-kernel-feedback-list, davem, edumazet, kuba, pabeni,
linux-kernel
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 22 Jun 2023 03:31:07 -0700 you wrote:
> With support for Ethernet PHY LEDs having been added, while
> unregistering a MDIO bus and its child device liks PHYs there may be
> "late" accesses to the MDIO bus. One typical use case is setting the PHY
> LEDs brightness to OFF for instance.
>
> We need to ensure that the MDIO bus controller remains entirely
> functional since it runs off the main GENET adapter clock.
>
> [...]
Here is the summary with links:
- [net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled
https://git.kernel.org/netdev/net/c/1b5ea7ffb7a3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-24 2:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-22 10:31 [PATCH net] net: bcmgenet: Ensure MDIO unregistration has clocks enabled Florian Fainelli
2023-06-22 15:36 ` Andrew Lunn
2023-06-22 15:50 ` Florian Fainelli
2023-06-22 19:14 ` Andrew Lunn
2023-06-24 2:00 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.