| Message ID | 20220217220252.52293-2-snelson@pensando.io (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ionic: driver updates | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| 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 |
On Thu, 17 Feb 2022 14:02:49 -0800 Shannon Nelson wrote: > In some graceful updates that get initially triggered by the > RESET event, especially with older firmware, the fw_generation > bits don't change but the fw_status is seen to go to 0 then back > to 1. However, the driver didn't perform the restart, remained > waiting for fw_generation to change, and got left in limbo. > > This is because the clearing of idev->fw_status_ready to 0 > didn't happen correctly as it was buried in the transition > trigger: since the transition down was triggered not here > but in the RESET event handler, the clear to 0 didn't happen, > so the transition back to 1 wasn't detected. > > Fix this particular case by bringing the setting of > idev->fw_status_ready back out to where it was before. > > Fixes: 398d1e37f960 ("ionic: add FW_STOPPING state") > Signed-off-by: Shannon Nelson <snelson@pensando.io> This looks like a fix, and should go separately to net. Is there a reason behind posting together? The other patches don't even depend on this one.
On 2/17/22 8:12 PM, Jakub Kicinski wrote: > On Thu, 17 Feb 2022 14:02:49 -0800 Shannon Nelson wrote: >> In some graceful updates that get initially triggered by the >> RESET event, especially with older firmware, the fw_generation >> bits don't change but the fw_status is seen to go to 0 then back >> to 1. However, the driver didn't perform the restart, remained >> waiting for fw_generation to change, and got left in limbo. >> >> This is because the clearing of idev->fw_status_ready to 0 >> didn't happen correctly as it was buried in the transition >> trigger: since the transition down was triggered not here >> but in the RESET event handler, the clear to 0 didn't happen, >> so the transition back to 1 wasn't detected. >> >> Fix this particular case by bringing the setting of >> idev->fw_status_ready back out to where it was before. >> >> Fixes: 398d1e37f960 ("ionic: add FW_STOPPING state") >> Signed-off-by: Shannon Nelson <snelson@pensando.io> > This looks like a fix, and should go separately to net. > Is there a reason behind posting together? The other patches > don't even depend on this one. I posted it to net-next because the patch it is fixing is still in net-next and not in net or stable yet. sln
On Fri, 18 Feb 2022 09:30:45 -0800 Shannon Nelson wrote: > On 2/17/22 8:12 PM, Jakub Kicinski wrote: > > This looks like a fix, and should go separately to net. > > Is there a reason behind posting together? The other patches > > don't even depend on this one. > > I posted it to net-next because the patch it is fixing is still in > net-next and not in net or stable yet. Not sure how I missed that, sorry. Applied now, thanks!
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c index 52a1b5cfd8e7..faeedc8db6f4 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c @@ -206,6 +206,8 @@ int ionic_heartbeat_check(struct ionic *ionic) if (fw_status_ready != idev->fw_status_ready) { bool trigger = false; + idev->fw_status_ready = fw_status_ready; + if (!fw_status_ready && lif && !test_bit(IONIC_LIF_F_FW_RESET, lif->state) && !test_and_set_bit(IONIC_LIF_F_FW_STOPPING, lif->state)) { @@ -222,8 +224,6 @@ int ionic_heartbeat_check(struct ionic *ionic) if (trigger) { struct ionic_deferred_work *work; - idev->fw_status_ready = fw_status_ready; - work = kzalloc(sizeof(*work), GFP_ATOMIC); if (work) { work->type = IONIC_DW_TYPE_LIF_RESET;
In some graceful updates that get initially triggered by the RESET event, especially with older firmware, the fw_generation bits don't change but the fw_status is seen to go to 0 then back to 1. However, the driver didn't perform the restart, remained waiting for fw_generation to change, and got left in limbo. This is because the clearing of idev->fw_status_ready to 0 didn't happen correctly as it was buried in the transition trigger: since the transition down was triggered not here but in the RESET event handler, the clear to 0 didn't happen, so the transition back to 1 wasn't detected. Fix this particular case by bringing the setting of idev->fw_status_ready back out to where it was before. Fixes: 398d1e37f960 ("ionic: add FW_STOPPING state") Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic_dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)