Re: [EXT] Re: [net PATCH v4 09/10] octeontx2-af: Skip PFs if not enabled

From: Simon Horman
Date: Tue May 02 2023 - 07:55:20 EST


On Tue, May 02, 2023 at 03:10:53AM +0000, Ratheesh Kannoth wrote:
> Hi Simon,
>
> Thanks for your review. Please find replies inline.
>
> -Ratheesh
>
> > -----Original Message-----
> > From: Simon Horman <simon.horman@xxxxxxxxxxxx>
> > Sent: Wednesday, April 26, 2023 3:31 PM
> > To: Sai Krishna Gajula <saikrishnag@xxxxxxxxxxx>
> > Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> > pabeni@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx; leon@xxxxxxxxxx; Sunil Kovvuri Goutham
> > <sgoutham@xxxxxxxxxxx>; Geethasowjanya Akula <gakula@xxxxxxxxxxx>;
> > Linu Cherian <lcherian@xxxxxxxxxxx>; Jerin Jacob Kollanukkaran
> > <jerinj@xxxxxxxxxxx>; Hariprasad Kelam <hkelam@xxxxxxxxxxx>;
> > Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxxx>; Ratheesh Kannoth
> > <rkannoth@xxxxxxxxxxx>
> > Subject: [EXT] Re: [net PATCH v4 09/10] octeontx2-af: Skip PFs if not enabled
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Wed, Apr 26, 2023 at 01:13:44PM +0530, Sai Krishna wrote:
> > > From: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>
> > >
> > > Fiwmware enables set of PFs and allocate mbox resources based on the
> > > number of enabled PFs. Current driver tries to initialize the mbox
> > > resources even for disabled PFs, which may waste the resources.
> > > This patch fixes the issue by skipping the mbox initialization of
> > > disabled PFs.
> >
> > FWIIW, this feels more like an enhancement than a fix to me.
>
> [Ratheesh Kannoth] I agree, commit message convey this change as
> enhancement. But this Code change fixes a crash in driver. We will
> modify the commit message as below in next Patchset version. " Firmware
> enables PFs and allocate mbox resources for each of the PFs. Currently
> PF driver configures mbox resources without checking whether PF is
> enabled or not. This results in crash. This patch fixes this issue by
> skipping disabled PF's mbox initialization. "

Thanks, much appreciated.