| Message ID | 20241106141152.1943-1-liangwentao@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: miss media cleanup behavior | expand |
Wed, Nov 06, 2024 at 03:11:52PM CET, liangwentao@iscas.ac.cn wrote: >In the de21041_media_timer(), line 1081, when media type is locked, >the code jumps to line 1136 to perform cleanup operations. However, >in the de21040_media_timer(), line 991, the same condition leads to >an immediate return without any cleanup. Don't use line numbers please. > >To address this inconsistency, we have added a jump statement to the Who's "we"? Just tell the codebase what to do, what to add, what to change, etc. >de21040_media_timer() to ensure that cleanup operations are executed >before the function returns. > >Signed-off-by: Wentao Liang <liangwentao@iscas.ac.cn> You are missing "Fixes" tag blaming the commit that introduced the issue. Actually, just to make things in the same way de21041_media_timer() has it is not a good reason to do so. What are you trying to fix? What issue you see. Why the code change is ok? The code itself looks okay to me. >--- > drivers/net/ethernet/dec/tulip/de2104x.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c >index 0a161a4db242..724c0b3b3966 100644 >--- a/drivers/net/ethernet/dec/tulip/de2104x.c >+++ b/drivers/net/ethernet/dec/tulip/de2104x.c >@@ -988,7 +988,7 @@ static void de21040_media_timer (struct timer_list *t) > de_link_down(de); > > if (de->media_lock) >- return; >+ goto set_media; > > if (de->media_type == DE_MEDIA_AUI) { > static const u32 next_state = DE_MEDIA_TP; >@@ -998,6 +998,7 @@ static void de21040_media_timer (struct timer_list *t) > de_next_media(de, &next_state, 1); > } > >+set_media: > spin_lock_irqsave(&de->lock, flags); > de_stop_rxtx(de); > spin_unlock_irqrestore(&de->lock, flags); >-- >2.42.0.windows.2 > >
diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 0a161a4db242..724c0b3b3966 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -988,7 +988,7 @@ static void de21040_media_timer (struct timer_list *t) de_link_down(de); if (de->media_lock) - return; + goto set_media; if (de->media_type == DE_MEDIA_AUI) { static const u32 next_state = DE_MEDIA_TP; @@ -998,6 +998,7 @@ static void de21040_media_timer (struct timer_list *t) de_next_media(de, &next_state, 1); } +set_media: spin_lock_irqsave(&de->lock, flags); de_stop_rxtx(de); spin_unlock_irqrestore(&de->lock, flags);
In the de21041_media_timer(), line 1081, when media type is locked, the code jumps to line 1136 to perform cleanup operations. However, in the de21040_media_timer(), line 991, the same condition leads to an immediate return without any cleanup. To address this inconsistency, we have added a jump statement to the de21040_media_timer() to ensure that cleanup operations are executed before the function returns. Signed-off-by: Wentao Liang <liangwentao@iscas.ac.cn> --- drivers/net/ethernet/dec/tulip/de2104x.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)