On Sun, Aug 25, 2019 at 10:36 PM David Miller <da...@davemloft.net> wrote:
>
> From: Michael Chan <michael.c...@broadcom.com>
> Date: Sun, 25 Aug 2019 23:54:54 -0400
>
> > @@ -687,6 +687,32 @@ static int bnxt_func_cfg(struct bnxt *bp, int num_vfs)
> > return bnxt_hwrm_func_cfg(bp, num_vfs);
> > }
> >
> > +int bnxt_cfg_hw_sriov(struct bnxt *bp, int *num_vfs)
> > +{
> > + int rc;
> > +
> > + /* Register buffers for VFs */
> > + rc = bnxt_hwrm_func_buf_rgtr(bp);
> > + if (rc)
> > + return rc;
> > +
> > + /* Reserve resources for VFs */
> > + rc = bnxt_func_cfg(bp, *num_vfs);
> > + if (rc != *num_vfs) {
>
> I notice that these two operations are reversed here from where they were in
> the
> bnxt_sriov_enable() function. Does the BUF_RGTR operation have to be undone
> if
> the bnxt_func_cfg() fails?
>
> When it's not a straight extraction of code into a helper function one really
> should do one of two things in my opinion:
>
> 1) Explain the differences in the commit message.
>
> 2) Do a straight extration in one commit, change the ordering in another.
OK. Will break it up into 2 commits with explanations. The reason is
that during normal init, the PF is always initialized first and the
exact bring-up sequence does not matter too much. During error
recovery, the PF and VFs can be all trying to recover at about the
same time and the sequence matters more. Will explain all of this
again in v2. Thanks.
