On Sat, Dec 8, 2018 at 2:57 AM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
>
> On Thu, Dec 06, 2018 at 10:49:46AM -0800, Selvin Xavier wrote:
> > From: Devesh Sharma <devesh.sharma@xxxxxxxxxxxx>
> >
> > Increasing the depth of control path command queue to 8K
> > entries to handle burst of commands. This feature needs
> > support from FW and the driver/fw compatibility is checked
> > from the interface version number.
> > -#define BNXT_QPLIB_CMDQE_MAX_CNT 256
> > +#define BNXT_QPLIB_CMDQE_MAX_CNT_256 256
> > +#define BNXT_QPLIB_CMDQE_MAX_CNT_8192 8192
> > #define BNXT_QPLIB_CMDQE_UNITS sizeof(struct bnxt_qplib_cmdqe)
> > -#define BNXT_QPLIB_CMDQE_CNT_PER_PG (PAGE_SIZE / BNXT_QPLIB_CMDQE_UNITS)
> > +#define BNXT_QPLIB_CMDQE_BYTES(depth) ((depth) * BNXT_QPLIB_CMDQE_UNITS)
> > +#define BNXT_QPLIB_CMDQE_NPAGES(depth) ((BNXT_QPLIB_CMDQE_BYTES(depth) %\
> > + PAGE_SIZE) ? \
> > + ((BNXT_QPLIB_CMDQE_BYTES(depth) /\
> > + PAGE_SIZE) + 1) : \
> > + (BNXT_QPLIB_CMDQE_BYTES(depth) /\
> > + PAGE_SIZE))
>
> Several of these look like they should be a static inline function to me, at
> least this last one should be.
sure. will do
>
> > +#define BNXT_QPLIB_CMDQE_PAGE_SIZE(depth) (BNXT_QPLIB_CMDQE_NPAGES(depth) * \
> > + PAGE_SIZE)
> > +
> > +#define BNXT_QPLIB_CMDQE_CNT_PER_PG(depth) (BNXT_QPLIB_CMDQE_PAGE_SIZE(depth) /\
> > + BNXT_QPLIB_CMDQE_UNITS)
> > +
> > +#define MAX_CMDQ_IDX(depth) ((depth) - 1)
> > +#define MAX_CMDQ_IDX_PER_PG(depth) (BNXT_QPLIB_CMDQE_CNT_PER_PG(depth) - 1)
> >
> > -#define MAX_CMDQ_IDX (BNXT_QPLIB_CMDQE_MAX_CNT - 1)
> > -#define MAX_CMDQ_IDX_PER_PG (BNXT_QPLIB_CMDQE_CNT_PER_PG - 1)
> > -
> > -#define RCFW_MAX_OUTSTANDING_CMD BNXT_QPLIB_CMDQE_MAX_CNT
> > #define RCFW_MAX_COOKIE_VALUE 0x7FFF
> > #define RCFW_CMD_IS_BLOCKING 0x8000
> > #define RCFW_BLOCKED_CMD_WAIT_COUNT 0x4E20
> >
> > +#define HWRM_VERSION_RCFW_CMDQ_DEPTH_CHECK 0x1000900020011ULL
> > +
> > /* Cmdq contains a fix number of a 16-Byte slots */
> > struct bnxt_qplib_cmdqe {
> > u8 data[16];
> > };
> >
> > -static inline u32 get_cmdq_pg(u32 val)
> > +static inline u32 get_cmdq_pg(u32 val, u32 depth)
> > {
> > - return (val & ~MAX_CMDQ_IDX_PER_PG) / BNXT_QPLIB_CMDQE_CNT_PER_PG;
> > + return (val & ~(MAX_CMDQ_IDX_PER_PG(depth))) /
> > + (BNXT_QPLIB_CMDQE_CNT_PER_PG(depth));
> > }
> >
> > -static inline u32 get_cmdq_idx(u32 val)
> > +static inline u32 get_cmdq_idx(u32 val, u32 depth)
> > {
> > - return val & MAX_CMDQ_IDX_PER_PG;
> > + return val & (MAX_CMDQ_IDX_PER_PG(depth));
> > }
>
> .. and if you are already going to have static inlines with trivial
> content this, then probably all of the above macros should be
> functions.
>
> Jason
