Re: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control messages

[Leon Romanovsky]

On Wed, Nov 30, 2022 at 03:44:30PM +0000, Veerasenareddy Burru wrote:
> 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <leon@xxxxxxxxxx>
> > Sent: Wednesday, November 30, 2022 1:30 AM
> > To: Veerasenareddy Burru <vburru@xxxxxxxxxxx>
> > Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Liron Himi
> > <lironh@xxxxxxxxxxx>; Abhijit Ayarekar <aayarekar@xxxxxxxxxxx>; Sathesh
> > B Edara <sedara@xxxxxxxxxxx>; Satananda Burla <sburla@xxxxxxxxxxx>;
> > linux-doc@xxxxxxxxxxxxxxx; David S. Miller <davem@xxxxxxxxxxxxx>; Eric
> > Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> > Paolo Abeni <pabeni@xxxxxxxxxx>
> > Subject: [EXT] Re: [PATCH net-next v2 2/9] octeon_ep: poll for control
> > messages
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Tue, Nov 29, 2022 at 05:09:25AM -0800, Veerasenareddy Burru wrote:
> > > Poll for control messages until interrupts are enabled.
> > > All the interrupts are enabled in ndo_open().
> > 
> > So what are you saying if I have your device and didn't enable network
> > device, you will poll forever?
> Yes, Leon. It will poll periodically until network interface is enabled.

I don't know if it is acceptable behaviour in netdev, but it doesn't
sound right to me. What type of control messages will be sent by FW,
which PF should listen to them?

> > 
> > > 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@xxxxxxxxxxx>
> > > Signed-off-by: Abhijit Ayarekar <aayarekar@xxxxxxxxxxx>
> > > ---
> > > 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..ace2dfd1e918 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;
> > > +	u64 reg0;
> > > +	int handled = 0;
> > 
> > Reversed Christmas tree.
> Thanks for the feedback. Will revise the patch.

It is applicable to all patches.

And please fix your email client to properly add blank lines between
replies.

Thanks

> > 
> > Thanks