[PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Nikita Kiryushin]

As of now, tg3_power_down_prepare always ends with success, but
the error handling code from former tg3_set_power_state call is still here.

This code became unreachable in commit c866b7eac073 ("tg3: Do not use
legacy PCI power management").

Remove (now unreachable) error handling code for simplification and change
tg3_power_down_prepare to a void function as its result is no more checked.

Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
---
v3:
  - Change commit message wording as
    Jakub Kicinski <kuba@kernel.org> requested
v2: https://lore.kernel.org/netdev/a6f3f931-17eb-4e53-9220-f81e7b311a8c@ancud.ru/
  - Change tg3_power_down_prepare() to a void function as
    Michael Chan <michael.chan@broadcom.com> suggested
v1: https://lore.kernel.org/netdev/4e7e11f8-03b5-4289-9475-d3b4e105d40a@ancud.ru/
 drivers/net/ethernet/broadcom/tg3.c | 30 ++++-------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 04964bbe08cf..bc36926a57cf 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4019,7 +4019,7 @@ static int tg3_power_up(struct tg3 *tp)
 
 static int tg3_setup_phy(struct tg3 *, bool);
 
-static int tg3_power_down_prepare(struct tg3 *tp)
+static void tg3_power_down_prepare(struct tg3 *tp)
 {
 	u32 misc_host_ctrl;
 	bool device_should_wake, do_low_power;
@@ -4263,7 +4263,7 @@ static int tg3_power_down_prepare(struct tg3 *tp)
 
 	tg3_ape_driver_state_change(tp, RESET_KIND_SHUTDOWN);
 
-	return 0;
+	return;
 }
 
 static void tg3_power_down(struct tg3 *tp)
@@ -18090,7 +18090,6 @@ static int tg3_suspend(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct tg3 *tp = netdev_priv(dev);
-	int err = 0;
 
 	rtnl_lock();
 
@@ -18114,32 +18113,11 @@ static int tg3_suspend(struct device *device)
 	tg3_flag_clear(tp, INIT_COMPLETE);
 	tg3_full_unlock(tp);
 
-	err = tg3_power_down_prepare(tp);
-	if (err) {
-		int err2;
-
-		tg3_full_lock(tp, 0);
-
-		tg3_flag_set(tp, INIT_COMPLETE);
-		err2 = tg3_restart_hw(tp, true);
-		if (err2)
-			goto out;
-
-		tg3_timer_start(tp);
-
-		netif_device_attach(dev);
-		tg3_netif_start(tp);
-
-out:
-		tg3_full_unlock(tp);
-
-		if (!err2)
-			tg3_phy_start(tp);
-	}
+	tg3_power_down_prepare(tp);
 
 unlock:
 	rtnl_unlock();
-	return err;
+	return 0;
 }
 
 static int tg3_resume(struct device *device)
-- 
2.34.1

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Simon Horman]

On Mon, Apr 01, 2024 at 10:14:18PM +0300, Nikita Kiryushin wrote:
> As of now, tg3_power_down_prepare always ends with success, but
> the error handling code from former tg3_set_power_state call is still here.
> 
> This code became unreachable in commit c866b7eac073 ("tg3: Do not use
> legacy PCI power management").
> 
> Remove (now unreachable) error handling code for simplification and change
> tg3_power_down_prepare to a void function as its result is no more checked.
> 
> Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru>
> Reviewed-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon,  1 Apr 2024 22:14:18 +0300 you wrote:
> As of now, tg3_power_down_prepare always ends with success, but
> the error handling code from former tg3_set_power_state call is still here.
> 
> This code became unreachable in commit c866b7eac073 ("tg3: Do not use
> legacy PCI power management").
> 
> Remove (now unreachable) error handling code for simplification and change
> tg3_power_down_prepare to a void function as its result is no more checked.
> 
> [...]

Here is the summary with links:
  - [net-next,v3] tg3: Remove residual error handling in tg3_suspend
    https://git.kernel.org/netdev/net/c/9ab4ad295622

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Paolo Abeni]

On Thu, 2024-04-04 at 08:50 +0000, patchwork-bot+netdevbpf@kernel.org
wrote:
> Hello:
> 
> This patch was applied to netdev/net.git (main)
                                   ^^^^^^^
Wrong tree, blame on me. I'm reverting it from net, I will re-apply to
net-next. Sorry for the noise.

Paolo

[PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Nikita Kiryushin]

As of now, tg3_power_down_prepare always ends with success, but
the error handling code from former tg3_set_power_state call is still here.

This code became unreachable in commit c866b7eac073 ("tg3: Do not use
legacy PCI power management").

Remove (now unreachable) error handling code for simplification and change
tg3_power_down_prepare to a void function as its result is no more checked.

Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
---
Resubmitted tha patch as it was applied to wrong tree and then
reverted in commit 72076fc9fe60
v3:
  - Change commit message wording as
    Jakub Kicinski <kuba@kernel.org> requested
v2: https://lore.kernel.org/netdev/a6f3f931-17eb-4e53-9220-f81e7b311a8c@ancud.ru/
  - Change tg3_power_down_prepare() to a void function as
    Michael Chan <michael.chan@broadcom.com> suggested
v1: https://lore.kernel.org/netdev/4e7e11f8-03b5-4289-9475-d3b4e105d40a@ancud.ru/
 drivers/net/ethernet/broadcom/tg3.c | 30 ++++-------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 04964bbe08cf..bc36926a57cf 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4019,7 +4019,7 @@ static int tg3_power_up(struct tg3 *tp)
 
 static int tg3_setup_phy(struct tg3 *, bool);
 
-static int tg3_power_down_prepare(struct tg3 *tp)
+static void tg3_power_down_prepare(struct tg3 *tp)
 {
 	u32 misc_host_ctrl;
 	bool device_should_wake, do_low_power;
@@ -4263,7 +4263,7 @@ static int tg3_power_down_prepare(struct tg3 *tp)
 
 	tg3_ape_driver_state_change(tp, RESET_KIND_SHUTDOWN);
 
-	return 0;
+	return;
 }
 
 static void tg3_power_down(struct tg3 *tp)
@@ -18090,7 +18090,6 @@ static int tg3_suspend(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct tg3 *tp = netdev_priv(dev);
-	int err = 0;
 
 	rtnl_lock();
 
@@ -18114,32 +18113,11 @@ static int tg3_suspend(struct device *device)
 	tg3_flag_clear(tp, INIT_COMPLETE);
 	tg3_full_unlock(tp);
 
-	err = tg3_power_down_prepare(tp);
-	if (err) {
-		int err2;
-
-		tg3_full_lock(tp, 0);
-
-		tg3_flag_set(tp, INIT_COMPLETE);
-		err2 = tg3_restart_hw(tp, true);
-		if (err2)
-			goto out;
-
-		tg3_timer_start(tp);
-
-		netif_device_attach(dev);
-		tg3_netif_start(tp);
-
-out:
-		tg3_full_unlock(tp);
-
-		if (!err2)
-			tg3_phy_start(tp);
-	}
+	tg3_power_down_prepare(tp);
 
 unlock:
 	rtnl_unlock();
-	return err;
+	return 0;
 }
 
 static int tg3_resume(struct device *device)
-- 
2.34.1

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

On Tue, Jul 9, 2024 at 10:24 PM Nikita Kiryushin <kiryushin@ancud.ru> wrote:
>
> As of now, tg3_power_down_prepare always ends with success, but
> the error handling code from former tg3_set_power_state call is still here.
>
> This code became unreachable in commit c866b7eac073 ("tg3: Do not use
> legacy PCI power management").
>
> Remove (now unreachable) error handling code for simplification and change
> tg3_power_down_prepare to a void function as its result is no more checked.
>
> Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru>
> Reviewed-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Resubmitted tha patch as it was applied to wrong tree and then
> reverted in commit 72076fc9fe60

I am not sure I understand this patch, commit d72b735712e65, which is
the same patch, is already applied, right?

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Nikita Kiryushin]

Right. Last time it was applied to a wrong tree (net instead of net-next) and then
reverted because of this. As I understand, it was meant to be re-applied to net-next (https://lore.kernel.org/lkml/4726fefd2a710cbee0d1a7fb15e361564915e955.camel@redhat.com/),
but it seemed to get lost in process, so I decided to resubmit.

On 7/10/24 07:53, Pavan Chebbi wrote:
> I am not sure I understand this patch, commit d72b735712e65, which is
> the same patch, is already applied, right?

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On Wed, Jul 10, 2024 at 7:03 PM Nikita Kiryushin <kiryushin@ancud.ru> wrote:
>
> Right. Last time it was applied to a wrong tree (net instead of net-next) and then
> reverted because of this. As I understand, it was meant to be re-applied to net-next (https://lore.kernel.org/lkml/4726fefd2a710cbee0d1a7fb15e361564915e955.camel@redhat.com/),
> but it seemed to get lost in process, so I decided to resubmit.

No, I do see the patch in net-next. (net->net-next merge takes care)
Please rebase to latest?

>
> On 7/10/24 07:53, Pavan Chebbi wrote:
> > I am not sure I understand this patch, commit d72b735712e65, which is
> > the same patch, is already applied, right?
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

Re: [PATCH net-next v3] tg3: Remove residual error handling in tg3_suspend

[Nikita Kiryushin]

Thank You! It was an outdated tree on my side, what a shame.
Sorry for the confusion.

On 7/11/24 07:02, Pavan Chebbi wrote:
> No, I do see the patch in net-next. (net->net-next merge takes care)
> Please rebase to latest?
>