diff mbox series

[net,v3] octeon_ep: initialise control mbox tasks before using APIs

Message ID 20231206135228.2591659-1-srasheed@marvell.com
State New
Headers
Series [net,v3] octeon_ep: initialise control mbox tasks before using APIs |

Commit Message

Shinas Rasheed Dec. 6, 2023, 1:52 p.m. UTC 
  Initialise various workqueue tasks and queue interrupt poll task
before the first invocation of any control net APIs. Since
octep_ctrl_net_get_info was called before the control net receive
work task was initialised or even the interrupt poll task was
queued, the function call wasn't returning actual firmware
info queried from Octeon.

Fixes: 8d6198a14e2b ("octeon_ep: support to fetch firmware info")
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V3:
  - Included Fixes line in commit log.
  - Corrected typo in print statement.

V2: https://lore.kernel.org/all/20231205130625.2586755-1-srasheed@marvell.com/
  - Updated changelog.
  - Handled error return for octep_ctrl_net_get_info

V1: https://lore.kernel.org/all/20231202150807.2571103-1-srasheed@marvell.com/

 .../ethernet/marvell/octeon_ep/octep_main.c   | 22 +++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Michal Schmidt Dec. 6, 2023, 1:55 p.m. UTC | #1 
On Wed, Dec 6, 2023 at 2:52 PM Shinas Rasheed <srasheed@marvell.com> wrote:
>
> Initialise various workqueue tasks and queue interrupt poll task
> before the first invocation of any control net APIs. Since
> octep_ctrl_net_get_info was called before the control net receive
> work task was initialised or even the interrupt poll task was
> queued, the function call wasn't returning actual firmware
> info queried from Octeon.
>
> Fixes: 8d6198a14e2b ("octeon_ep: support to fetch firmware info")
> Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
> ---
> V3:
>   - Included Fixes line in commit log.
>   - Corrected typo in print statement.
>
> V2: https://lore.kernel.org/all/20231205130625.2586755-1-srasheed@marvell.com/
>   - Updated changelog.
>   - Handled error return for octep_ctrl_net_get_info
>
> V1: https://lore.kernel.org/all/20231202150807.2571103-1-srasheed@marvell.com/
>
>  .../ethernet/marvell/octeon_ep/octep_main.c   | 22 +++++++++++--------
>  1 file changed, 13 insertions(+), 9 deletions(-)

Good timing. I was just going to write to you about the typo :)
Looks good now.

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
patchwork-bot+netdevbpf@kernel.org Dec. 9, 2023, 12:20 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 6 Dec 2023 05:52:27 -0800 you wrote:
> Initialise various workqueue tasks and queue interrupt poll task
> before the first invocation of any control net APIs. Since
> octep_ctrl_net_get_info was called before the control net receive
> work task was initialised or even the interrupt poll task was
> queued, the function call wasn't returning actual firmware
> info queried from Octeon.
> 
> [...]

Here is the summary with links:
  - [net,v3] octeon_ep: initialise control mbox tasks before using APIs
    https://git.kernel.org/netdev/net/c/a1664b991ac1

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index b8ae269f6f97..dbab878b4d76 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -1193,6 +1193,13 @@  int octep_device_setup(struct octep_device *oct)
 	if (ret)
 		return ret;
 
+	INIT_WORK(&oct->tx_timeout_task, octep_tx_timeout_task);
+	INIT_WORK(&oct->ctrl_mbox_task, octep_ctrl_mbox_task);
+	INIT_DELAYED_WORK(&oct->intr_poll_task, octep_intr_poll_task);
+	oct->poll_non_ioq_intr = true;
+	queue_delayed_work(octep_wq, &oct->intr_poll_task,
+			   msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS));
+
 	atomic_set(&oct->hb_miss_cnt, 0);
 	INIT_DELAYED_WORK(&oct->hb_task, octep_hb_timeout_task);
 
@@ -1326,21 +1333,18 @@  static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_octep_config;
 	}
 
-	octep_ctrl_net_get_info(octep_dev, OCTEP_CTRL_NET_INVALID_VFID,
-				&octep_dev->conf->fw_info);
+	err = octep_ctrl_net_get_info(octep_dev, OCTEP_CTRL_NET_INVALID_VFID,
+				      &octep_dev->conf->fw_info);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to get firmware info\n");
+		goto register_dev_err;
+	}
 	dev_info(&octep_dev->pdev->dev, "Heartbeat interval %u msecs Heartbeat miss count %u\n",
 		 octep_dev->conf->fw_info.hb_interval,
 		 octep_dev->conf->fw_info.hb_miss_count);
 	queue_delayed_work(octep_wq, &octep_dev->hb_task,
 			   msecs_to_jiffies(octep_dev->conf->fw_info.hb_interval));
 
-	INIT_WORK(&octep_dev->tx_timeout_task, octep_tx_timeout_task);
-	INIT_WORK(&octep_dev->ctrl_mbox_task, octep_ctrl_mbox_task);
-	INIT_DELAYED_WORK(&octep_dev->intr_poll_task, octep_intr_poll_task);
-	octep_dev->poll_non_ioq_intr = true;
-	queue_delayed_work(octep_wq, &octep_dev->intr_poll_task,
-			   msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS));
-
 	netdev->netdev_ops = &octep_netdev_ops;
 	octep_set_ethtool_ops(netdev);
 	netif_carrier_off(netdev);