diff mbox series

[net.git] net: bgmac: postpone turning IRQs off to avoid SoC hangs

Message ID 20230707065325.11765-1-zajec5@gmail.com (mailing list archive)
State Accepted
Commit e7731194fdf085f46d58b1adccfddbd0dfee4873
Delegated to: Netdev Maintainers
Headers show
Series [net.git] net: bgmac: postpone turning IRQs off to avoid SoC hangs | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1341 this patch: 1341
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1364 this patch: 1364
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1364 this patch: 1364
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Rafał Miłecki July 7, 2023, 6:53 a.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

Turning IRQs off is done by accessing Ethernet controller registers.
That can't be done until device's clock is enabled. It results in a SoC
hang otherwise.

This bug remained unnoticed for years as most bootloaders keep all
Ethernet interfaces turned on. It seems to only affect a niche SoC
family BCM47189. It has two Ethernet controllers but CFE bootloader uses
only the first one.

Fixes: 34322615cbaa ("net: bgmac: Mask interrupts during probe")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/ethernet/broadcom/bgmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michal Kubiak July 7, 2023, 7:18 p.m. UTC | #1 
On Fri, Jul 07, 2023 at 08:53:25AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Turning IRQs off is done by accessing Ethernet controller registers.
> That can't be done until device's clock is enabled. It results in a SoC
> hang otherwise.
> 
> This bug remained unnoticed for years as most bootloaders keep all
> Ethernet interfaces turned on. It seems to only affect a niche SoC
> family BCM47189. It has two Ethernet controllers but CFE bootloader uses
> only the first one.
> 
> Fixes: 34322615cbaa ("net: bgmac: Mask interrupts during probe")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/ethernet/broadcom/bgmac.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 1761df8fb7f9..10c7c232cc4e 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1492,8 +1492,6 @@ int bgmac_enet_probe(struct bgmac *bgmac)
>  
>  	bgmac->in_init = true;
>  
> -	bgmac_chip_intrs_off(bgmac);
> -
>  	net_dev->irq = bgmac->irq;
>  	SET_NETDEV_DEV(net_dev, bgmac->dev);
>  	dev_set_drvdata(bgmac->dev, bgmac);
> @@ -1511,6 +1509,8 @@ int bgmac_enet_probe(struct bgmac *bgmac)
>  	 */
>  	bgmac_clk_enable(bgmac, 0);
>  
> +	bgmac_chip_intrs_off(bgmac);
> +
>  	/* This seems to be fixing IRQ by assigning OOB #6 to the core */
>  	if (!(bgmac->feature_flags & BGMAC_FEAT_IDM_MASK)) {
>  		if (bgmac->feature_flags & BGMAC_FEAT_IRQ_ID_OOB_6)
> -- 
> 2.35.3
> 
>

Looks like a good catch!

Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
patchwork-bot+netdevbpf@kernel.org July 8, 2023, 9:13 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  7 Jul 2023 08:53:25 +0200 you wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Turning IRQs off is done by accessing Ethernet controller registers.
> That can't be done until device's clock is enabled. It results in a SoC
> hang otherwise.
> 
> This bug remained unnoticed for years as most bootloaders keep all
> Ethernet interfaces turned on. It seems to only affect a niche SoC
> family BCM47189. It has two Ethernet controllers but CFE bootloader uses
> only the first one.
> 
> [...]

Here is the summary with links:
  - [net.git] net: bgmac: postpone turning IRQs off to avoid SoC hangs
    https://git.kernel.org/netdev/net/c/e7731194fdf0

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 1761df8fb7f9..10c7c232cc4e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1492,8 +1492,6 @@  int bgmac_enet_probe(struct bgmac *bgmac)
 
 	bgmac->in_init = true;
 
-	bgmac_chip_intrs_off(bgmac);
-
 	net_dev->irq = bgmac->irq;
 	SET_NETDEV_DEV(net_dev, bgmac->dev);
 	dev_set_drvdata(bgmac->dev, bgmac);
@@ -1511,6 +1509,8 @@  int bgmac_enet_probe(struct bgmac *bgmac)
 	 */
 	bgmac_clk_enable(bgmac, 0);
 
+	bgmac_chip_intrs_off(bgmac);
+
 	/* This seems to be fixing IRQ by assigning OOB #6 to the core */
 	if (!(bgmac->feature_flags & BGMAC_FEAT_IDM_MASK)) {
 		if (bgmac->feature_flags & BGMAC_FEAT_IRQ_ID_OOB_6)