Message ID | 20230214051422.13705-3-vburru@marvell.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | octeon_ep: deferred probe and mailbox | 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 7 of 7 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/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | warning | WARNING: line length of 84 exceeds 80 columns |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
On Mon, Feb 13, 2023 at 09:14:17PM -0800, Veerasenareddy Burru wrote: > Poll for control messages until interrupts are enabled. > All the interrupts are enabled in ndo_open(). > Add ability to listen for notifications from firmware before ndo_open(). > Once interrupts are enabled, this polling is disabled and all the > messages are processed by bottom half of interrupt handler. > > Signed-off-by: Veerasenareddy Burru <vburru@marvell.com> > Signed-off-by: Abhijit Ayarekar <aayarekar@marvell.com> small two nits > --- > v2-> v3: > * resovled review comment; fixed reverse christmas tree. > > v1 -> v2: > * removed device status oct->status, as it is not required with the > modified implementation in 0001-xxxx.patch > > .../marvell/octeon_ep/octep_cn9k_pf.c | 49 +++++++++---------- > .../ethernet/marvell/octeon_ep/octep_main.c | 35 +++++++++++++ > .../ethernet/marvell/octeon_ep/octep_main.h | 11 ++++- > .../marvell/octeon_ep/octep_regs_cn9k_pf.h | 4 ++ > 4 files changed, 71 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > index 6ad88d0fe43f..f40ebac15a79 100644 > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > @@ -352,27 +352,36 @@ static void octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no) > mbox->mbox_read_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_VF_PF_DATA(q_no); > } > > -/* Mailbox Interrupt handler */ > -static void cn93_handle_pf_mbox_intr(struct octep_device *oct) > +/* Process non-ioq interrupts required to keep pf interface running. > + * OEI_RINT is needed for control mailbox > + */ > +static int octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct) return bool? > { > - u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL; > + int handled = 0; > + u64 reg0; > > - mbox_int_val = readq(oct->mbox[0]->mbox_int_reg); > - for (qno = 0; qno < OCTEP_MAX_VF; qno++) { > - val = readq(oct->mbox[qno]->mbox_read_reg); > - dev_dbg(&oct->pdev->dev, > - "PF MBOX READ: val:%llx from VF:%llx\n", val, qno); > + /* Check for OEI INTR */ > + reg0 = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); > + if (reg0) { > + dev_info(&oct->pdev->dev, > + "Received OEI_RINT intr: 0x%llx\n", > + reg0); > + octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg0); > + if (reg0 & CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX) > + queue_work(octep_wq, &oct->ctrl_mbox_task); > + > + handled = 1; > } > > - writeq(mbox_int_val, oct->mbox[0]->mbox_int_reg); > + return handled; > } > > /* Interrupts handler for all non-queue generic interrupts. */ > static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev) > { > struct octep_device *oct = (struct octep_device *)dev; > - struct pci_dev *pdev = oct->pdev; > u64 reg_val = 0; > + struct pci_dev *pdev = oct->pdev; why this move of var and rct breakage? > int i = 0; > > /* Check for IRERR INTR */ > @@ -434,24 +443,9 @@ static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev) > goto irq_handled; > } > > - /* Check for MBOX INTR */ > - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0)); > - if (reg_val) { > - dev_info(&pdev->dev, > - "Received MBOX_RINT intr: 0x%llx\n", reg_val); > - cn93_handle_pf_mbox_intr(oct); > + /* Check for MBOX INTR and OEI INTR */ > + if (octep_poll_non_ioq_interrupts_cn93_pf(oct)) > goto irq_handled; > - } > - > - /* Check for OEI INTR */ > - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); > - if (reg_val) { > - dev_info(&pdev->dev, > - "Received OEI_EINT intr: 0x%llx\n", reg_val); > - octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg_val); > - queue_work(octep_wq, &oct->ctrl_mbox_task); > - goto irq_handled; > - } > > /* Check for DMA INTR */ > reg_val = octep_read_csr64(oct, CN93_SDP_EPF_DMA_RINT); > @@ -712,6 +706,7 @@ void octep_device_setup_cn93_pf(struct octep_device *oct) > > oct->hw_ops.enable_interrupts = octep_enable_interrupts_cn93_pf; > oct->hw_ops.disable_interrupts = octep_disable_interrupts_cn93_pf; > + oct->hw_ops.poll_non_ioq_interrupts = octep_poll_non_ioq_interrupts_cn93_pf; > > oct->hw_ops.update_iq_read_idx = octep_update_iq_read_index_cn93_pf;
> -----Original Message----- > From: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Sent: Tuesday, February 14, 2023 9:43 AM > To: Veerasenareddy Burru <vburru@marvell.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Abhijit Ayarekar > <aayarekar@marvell.com>; Sathesh B Edara <sedara@marvell.com>; > Satananda Burla <sburla@marvell.com>; linux-doc@vger.kernel.org; David S. > Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; > Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com> > Subject: [EXT] Re: [PATCH net-next v3 2/7] octeon_ep: poll for control > messages > > External Email > > ---------------------------------------------------------------------- > On Mon, Feb 13, 2023 at 09:14:17PM -0800, Veerasenareddy Burru wrote: > > Poll for control messages until interrupts are enabled. > > All the interrupts are enabled in ndo_open(). > > Add ability to listen for notifications from firmware before ndo_open(). > > Once interrupts are enabled, this polling is disabled and all the > > messages are processed by bottom half of interrupt handler. > > > > Signed-off-by: Veerasenareddy Burru <vburru@marvell.com> > > Signed-off-by: Abhijit Ayarekar <aayarekar@marvell.com> > > small two nits > > > --- > > v2-> v3: > > * resovled review comment; fixed reverse christmas tree. > > > > v1 -> v2: > > * removed device status oct->status, as it is not required with the > > modified implementation in 0001-xxxx.patch > > > > .../marvell/octeon_ep/octep_cn9k_pf.c | 49 +++++++++---------- > > .../ethernet/marvell/octeon_ep/octep_main.c | 35 +++++++++++++ > > .../ethernet/marvell/octeon_ep/octep_main.h | 11 ++++- > > .../marvell/octeon_ep/octep_regs_cn9k_pf.h | 4 ++ > > 4 files changed, 71 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > > b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > > index 6ad88d0fe43f..f40ebac15a79 100644 > > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c > > @@ -352,27 +352,36 @@ static void > octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no) > > mbox->mbox_read_reg = oct->mmio[0].hw_addr + > > CN93_SDP_R_MBOX_VF_PF_DATA(q_no); } > > > > -/* Mailbox Interrupt handler */ > > -static void cn93_handle_pf_mbox_intr(struct octep_device *oct) > > +/* Process non-ioq interrupts required to keep pf interface running. > > + * OEI_RINT is needed for control mailbox */ static int > > +octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct) > > return bool? > Yes, bool is sufficient. Will make the change in next revision. > > { > > - u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL; > > + int handled = 0; > > + u64 reg0; > > > > - mbox_int_val = readq(oct->mbox[0]->mbox_int_reg); > > - for (qno = 0; qno < OCTEP_MAX_VF; qno++) { > > - val = readq(oct->mbox[qno]->mbox_read_reg); > > - dev_dbg(&oct->pdev->dev, > > - "PF MBOX READ: val:%llx from VF:%llx\n", val, qno); > > + /* Check for OEI INTR */ > > + reg0 = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); > > + if (reg0) { > > + dev_info(&oct->pdev->dev, > > + "Received OEI_RINT intr: 0x%llx\n", > > + reg0); > > + octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg0); > > + if (reg0 & CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX) > > + queue_work(octep_wq, &oct->ctrl_mbox_task); > > + > > + handled = 1; > > } > > > > - writeq(mbox_int_val, oct->mbox[0]->mbox_int_reg); > > + return handled; > > } > > > > /* Interrupts handler for all non-queue generic interrupts. */ > > static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev) { > > struct octep_device *oct = (struct octep_device *)dev; > > - struct pci_dev *pdev = oct->pdev; > > u64 reg_val = 0; > > + struct pci_dev *pdev = oct->pdev; > > why this move of var and rct breakage? > Thank you for the feedback. This change was not necessary. Will revert this change. I will recheck whole patchset for RCT breakage and fix it in next revision. > > int i = 0; > > > > /* Check for IRERR INTR */ > > @@ -434,24 +443,9 @@ static irqreturn_t > octep_non_ioq_intr_handler_cn93_pf(void *dev) > > goto irq_handled; > > } > > > > - /* Check for MBOX INTR */ > > - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0)); > > - if (reg_val) { > > - dev_info(&pdev->dev, > > - "Received MBOX_RINT intr: 0x%llx\n", reg_val); > > - cn93_handle_pf_mbox_intr(oct); > > + /* Check for MBOX INTR and OEI INTR */ > > + if (octep_poll_non_ioq_interrupts_cn93_pf(oct)) > > goto irq_handled; > > - } > > - > > - /* Check for OEI INTR */ > > - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); > > - if (reg_val) { > > - dev_info(&pdev->dev, > > - "Received OEI_EINT intr: 0x%llx\n", reg_val); > > - octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg_val); > > - queue_work(octep_wq, &oct->ctrl_mbox_task); > > - goto irq_handled; > > - } > > > > /* Check for DMA INTR */ > > reg_val = octep_read_csr64(oct, CN93_SDP_EPF_DMA_RINT); > > @@ -712,6 +706,7 @@ void octep_device_setup_cn93_pf(struct > octep_device *oct) > > > > oct->hw_ops.enable_interrupts = > octep_enable_interrupts_cn93_pf; > > oct->hw_ops.disable_interrupts = > octep_disable_interrupts_cn93_pf; > > + oct->hw_ops.poll_non_ioq_interrupts = > octep_poll_non_ioq_interrupts_cn93_pf; > > > > oct->hw_ops.update_iq_read_idx = > octep_update_iq_read_index_cn93_pf;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c index 6ad88d0fe43f..f40ebac15a79 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c @@ -352,27 +352,36 @@ static void octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no) mbox->mbox_read_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_VF_PF_DATA(q_no); } -/* Mailbox Interrupt handler */ -static void cn93_handle_pf_mbox_intr(struct octep_device *oct) +/* Process non-ioq interrupts required to keep pf interface running. + * OEI_RINT is needed for control mailbox + */ +static int octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct) { - u64 mbox_int_val = 0ULL, val = 0ULL, qno = 0ULL; + int handled = 0; + u64 reg0; - mbox_int_val = readq(oct->mbox[0]->mbox_int_reg); - for (qno = 0; qno < OCTEP_MAX_VF; qno++) { - val = readq(oct->mbox[qno]->mbox_read_reg); - dev_dbg(&oct->pdev->dev, - "PF MBOX READ: val:%llx from VF:%llx\n", val, qno); + /* Check for OEI INTR */ + reg0 = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); + if (reg0) { + dev_info(&oct->pdev->dev, + "Received OEI_RINT intr: 0x%llx\n", + reg0); + octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg0); + if (reg0 & CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX) + queue_work(octep_wq, &oct->ctrl_mbox_task); + + handled = 1; } - writeq(mbox_int_val, oct->mbox[0]->mbox_int_reg); + return handled; } /* Interrupts handler for all non-queue generic interrupts. */ static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev) { struct octep_device *oct = (struct octep_device *)dev; - struct pci_dev *pdev = oct->pdev; u64 reg_val = 0; + struct pci_dev *pdev = oct->pdev; int i = 0; /* Check for IRERR INTR */ @@ -434,24 +443,9 @@ static irqreturn_t octep_non_ioq_intr_handler_cn93_pf(void *dev) goto irq_handled; } - /* Check for MBOX INTR */ - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0)); - if (reg_val) { - dev_info(&pdev->dev, - "Received MBOX_RINT intr: 0x%llx\n", reg_val); - cn93_handle_pf_mbox_intr(oct); + /* Check for MBOX INTR and OEI INTR */ + if (octep_poll_non_ioq_interrupts_cn93_pf(oct)) goto irq_handled; - } - - /* Check for OEI INTR */ - reg_val = octep_read_csr64(oct, CN93_SDP_EPF_OEI_RINT); - if (reg_val) { - dev_info(&pdev->dev, - "Received OEI_EINT intr: 0x%llx\n", reg_val); - octep_write_csr64(oct, CN93_SDP_EPF_OEI_RINT, reg_val); - queue_work(octep_wq, &oct->ctrl_mbox_task); - goto irq_handled; - } /* Check for DMA INTR */ reg_val = octep_read_csr64(oct, CN93_SDP_EPF_DMA_RINT); @@ -712,6 +706,7 @@ void octep_device_setup_cn93_pf(struct octep_device *oct) oct->hw_ops.enable_interrupts = octep_enable_interrupts_cn93_pf; oct->hw_ops.disable_interrupts = octep_disable_interrupts_cn93_pf; + oct->hw_ops.poll_non_ioq_interrupts = octep_poll_non_ioq_interrupts_cn93_pf; oct->hw_ops.update_iq_read_idx = octep_update_iq_read_index_cn93_pf; diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c index 5620df4c6d55..565a383f0c68 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c @@ -18,6 +18,7 @@ #include "octep_main.h" #include "octep_ctrl_net.h" +#define OCTEP_INTR_POLL_TIME_MSECS 100 struct workqueue_struct *octep_wq; /* Supported Devices */ @@ -512,6 +513,7 @@ static int octep_open(struct net_device *netdev) ret = octep_get_link_status(oct); if (!ret) octep_set_link_status(oct, true); + oct->poll_non_ioq_intr = false; /* Enable the input and output queues for this Octeon device */ oct->hw_ops.enable_io_queues(oct); @@ -573,6 +575,11 @@ static int octep_stop(struct net_device *netdev) oct->hw_ops.reset_io_queues(oct); octep_free_oqs(oct); octep_free_iqs(oct); + + oct->poll_non_ioq_intr = true; + queue_delayed_work(octep_wq, &oct->intr_poll_task, + msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS)); + netdev_info(netdev, "Device stopped !!\n"); return 0; } @@ -865,6 +872,28 @@ static const struct net_device_ops octep_netdev_ops = { .ndo_change_mtu = octep_change_mtu, }; +/** + * octep_intr_poll_task - work queue task to process non-ioq interrupts. + * + * @work: pointer to mbox work_struct + * + * Process non-ioq interrupts to handle control mailbox, pfvf mailbox. + **/ +static void octep_intr_poll_task(struct work_struct *work) +{ + struct octep_device *oct = container_of(work, struct octep_device, + intr_poll_task.work); + + if (!oct->poll_non_ioq_intr) { + dev_info(&oct->pdev->dev, "Interrupt poll task stopped.\n"); + return; + } + + oct->hw_ops.poll_non_ioq_interrupts(oct); + queue_delayed_work(octep_wq, &oct->intr_poll_task, + msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS)); +} + /** * octep_ctrl_mbox_task - work queue task to handle ctrl mbox messages. * @@ -1101,6 +1130,10 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent) } 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); @@ -1162,6 +1195,8 @@ static void octep_remove(struct pci_dev *pdev) if (netdev->reg_state == NETREG_REGISTERED) unregister_netdev(netdev); + oct->poll_non_ioq_intr = false; + cancel_delayed_work_sync(&oct->intr_poll_task); octep_device_cleanup(oct); pci_release_mem_regions(pdev); free_netdev(netdev); diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h index 123ffc13754d..70cc3e236cb4 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h @@ -73,6 +73,7 @@ struct octep_hw_ops { void (*enable_interrupts)(struct octep_device *oct); void (*disable_interrupts)(struct octep_device *oct); + int (*poll_non_ioq_interrupts)(struct octep_device *oct); void (*enable_io_queues)(struct octep_device *oct); void (*disable_io_queues)(struct octep_device *oct); @@ -270,7 +271,15 @@ struct octep_device { /* Work entry to handle ctrl mbox interrupt */ struct work_struct ctrl_mbox_task; - + /* Wait queue for host to firmware requests */ + wait_queue_head_t ctrl_req_wait_q; + /* List of objects waiting for h2f response */ + struct list_head ctrl_req_wait_list; + + /* Enable non-ioq interrupt polling */ + bool poll_non_ioq_intr; + /* Work entry to poll non-ioq interrupts */ + struct delayed_work intr_poll_task; }; static inline u16 OCTEP_MAJOR_REV(struct octep_device *oct) diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h b/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h index 3d5d39a52fe6..0466fd9a002d 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_regs_cn9k_pf.h @@ -364,4 +364,8 @@ /* Number of non-queue interrupts in CN93xx */ #define CN93_NUM_NON_IOQ_INTR 16 + +/* bit 0 for control mbox interrupt */ +#define CN93_SDP_EPF_OEI_RINT_DATA_BIT_MBOX BIT_ULL(0) + #endif /* _OCTEP_REGS_CN9K_PF_H_ */