| Message ID | 20231127162135.2529363-2-srasheed@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | support OCTEON CN98 devices | expand |
On Mon, 27 Nov 2023 08:21:34 -0800 Shinas Rasheed wrote:
> Device unload control net function should inform firmware
What is "control net" again?
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Tuesday, November 28, 2023 8:13 AM > To: Shinas Rasheed <srasheed@marvell.com> > > On Mon, 27 Nov 2023 08:21:34 -0800 Shinas Rasheed wrote: > > Device unload control net function should inform firmware > > What is "control net" again? Control net is just a software layer which is used by the host driver as well as the firmware to communicate with each other, given in the source file octep_ctrl_net.c and the corresponding octep_ctrl_net.h interface, which is already part of upstreamed driver.
On Tue, 28 Nov 2023 04:22:11 +0000 Shinas Rasheed wrote: > > On Mon, 27 Nov 2023 08:21:34 -0800 Shinas Rasheed wrote: > > > Device unload control net function should inform firmware > > > > What is "control net" again? > > Control net is just a software layer which is used by the host driver > as well as the firmware to communicate with each other, given in the > source file octep_ctrl_net.c and the corresponding octep_ctrl_net.h > interface, which is already part of upstreamed driver. Yes, I think it went in before I had time to nack it. I'm strongly against using the IP stack to talk to FW, if you read the ML you would know it. No new patches to octep_ctrl_net will be accepted.
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Tuesday, November 28, 2023 9:57 PM > To: Shinas Rasheed <srasheed@marvell.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Haseeb Gani > On Tue, 28 Nov 2023 04:22:11 +0000 Shinas Rasheed wrote: > > > On Mon, 27 Nov 2023 08:21:34 -0800 Shinas Rasheed wrote: > > > > Device unload control net function should inform firmware > > > > > > What is "control net" again? > > > > Control net is just a software layer which is used by the host driver > > as well as the firmware to communicate with each other, given in the > > source file octep_ctrl_net.c and the corresponding octep_ctrl_net.h > > interface, which is already part of upstreamed driver. > > Yes, I think it went in before I had time to nack it. > I'm strongly against using the IP stack to talk to FW, > if you read the ML you would know it. > > No new patches to octep_ctrl_net will be accepted. Control net doesn't use the IP stack at all. It's a simple producer-consumer based ring mechanism using PCIe BAR4 memory. Sorry maybe the nomenclature suggests something of that nature.
On Tue, 28 Nov 2023 19:08:26 +0000 Shinas Rasheed wrote: > > Yes, I think it went in before I had time to nack it. > > I'm strongly against using the IP stack to talk to FW, > > if you read the ML you would know it. > > > > No new patches to octep_ctrl_net will be accepted. > > Control net doesn't use the IP stack at all. It's a simple > producer-consumer based ring mechanism using PCIe BAR4 memory. > Sorry maybe the nomenclature suggests something of that nature. Ah, got it. I read that as "separate netdev for control", my bad. Just one nit then: >+ dev_info(&oct->pdev->dev, "Sending dev_unload msg to fw\n"); Is it really necessary to print this at info level for each remove? Remove is a normal part of operation and we shouldn't spam logs unless we have a good reason..
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Wednesday, November 29, 2023 3:25 AM > To: Shinas Rasheed <srasheed@marvell.com> > >+ dev_info(&oct->pdev->dev, "Sending dev_unload msg to fw\n"); > > Is it really necessary to print this at info level for each remove? > Remove is a normal part of operation and we shouldn't spam logs > unless we have a good reason.. I suppose it's not a need other than maybe for debug purposes. I'll use dev_dbg and submit a second version
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c index 5fa596c674da..f0fdfda8894a 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c @@ -26,7 +26,7 @@ static atomic_t ctrl_net_msg_id; /* Control plane version in which OCTEP_CTRL_NET_H2F_CMD was added */ static const u32 octep_ctrl_net_h2f_cmd_versions[OCTEP_CTRL_NET_H2F_CMD_MAX] = { - [OCTEP_CTRL_NET_H2F_CMD_INVALID ... OCTEP_CTRL_NET_H2F_CMD_GET_INFO] = + [OCTEP_CTRL_NET_H2F_CMD_INVALID ... OCTEP_CTRL_NET_H2F_CMD_DEV_REMOVE] = OCTEP_CP_VERSION(1, 0, 0) }; @@ -393,10 +393,24 @@ int octep_ctrl_net_get_info(struct octep_device *oct, int vfid, return 0; } +int octep_ctrl_net_dev_remove(struct octep_device *oct, int vfid) +{ + struct octep_ctrl_net_wait_data d = {}; + struct octep_ctrl_net_h2f_req *req; + + req = &d.data.req; + dev_info(&oct->pdev->dev, "Sending dev_unload msg to fw\n"); + init_send_req(&d.msg, req, sizeof(int), vfid); + req->hdr.s.cmd = OCTEP_CTRL_NET_H2F_CMD_DEV_REMOVE; + + return octep_send_mbox_req(oct, &d, false); +} int octep_ctrl_net_uninit(struct octep_device *oct) { struct octep_ctrl_net_wait_data *pos, *n; + octep_ctrl_net_dev_remove(oct, OCTEP_CTRL_NET_INVALID_VFID); + list_for_each_entry_safe(pos, n, &oct->ctrl_req_wait_list, list) pos->done = 1; diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h index a2463b460ad9..0de4de2ceb8f 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.h @@ -42,6 +42,7 @@ enum octep_ctrl_net_h2f_cmd { OCTEP_CTRL_NET_H2F_CMD_RX_STATE, OCTEP_CTRL_NET_H2F_CMD_LINK_INFO, OCTEP_CTRL_NET_H2F_CMD_GET_INFO, + OCTEP_CTRL_NET_H2F_CMD_DEV_REMOVE, OCTEP_CTRL_NET_H2F_CMD_MAX }; @@ -370,6 +371,16 @@ void octep_ctrl_net_recv_fw_messages(struct octep_device *oct); int octep_ctrl_net_get_info(struct octep_device *oct, int vfid, struct octep_fw_info *info); +/** + * octep_ctrl_net_dev_remove() - Indicate to firmware that a device unload has happened. + * + * @oct: non-null pointer to struct octep_device. + * @vfid: Index of virtual function. + * + * return value: 0 on success, -errno on failure. + */ +int octep_ctrl_net_dev_remove(struct octep_device *oct, int vfid); + /** * octep_ctrl_net_uninit() - Uninitialize data for ctrl net. *
Device unload control net function should inform firmware of driver unload to let it take necessary actions to cleanup. Signed-off-by: Shinas Rasheed <srasheed@marvell.com> --- .../ethernet/marvell/octeon_ep/octep_ctrl_net.c | 16 +++++++++++++++- .../ethernet/marvell/octeon_ep/octep_ctrl_net.h | 11 +++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-)