Date:   Tue, 20 Dec 2016 09:28:43 -0500
From:   Doug Ledford <dledford@...hat.com>
To:     Selvin Xavier <selvin.xavier@...adcom.com>,
        linux-rdma@...r.kernel.org
Cc:     netdev@...r.kernel.org, michael.chan@...adcom.com
Subject: Re: [PATCH for bnxt_re V3 00/21] Broadcom RoCE Driver (bnxt_re)

On 12/20/2016 4:13 AM, Selvin Xavier wrote:
> This series introduces the RoCE driver for the Broadcom
> NetXtreme-E 10/25/40/50G RoCE HCAs.
> This driver is dependent on the bnxt_en NIC driver and is
> based on the bnxt_re branch in linux-rdma repository.
> bnxt_en changes required for this patch series are already
> available afore mentioned branch.
> 
> These changes are available for your reference in
> the bnxt_re_v3 branch of following repository.
> https://github.com/Broadcom/linux-rdma-nxt/
> 
> Doug,
> Please review and consider applying this to linux-rdma repository
> for 4.11 merge cycle.

I certainly won't get it done before the holiday break coming up (Red
Hat, and by extension, myself, have a 1 week shutdown over the holiday
season, so I'll be offline starting this Friday).

However, even though you changed the api filename, there is still a lot
of inconsistency in the naming of your files.  The module itself is
bnxt_re, not bnxtre or any other variant.  Please make all uses of
bnxtre or other variants match bnxt_re.  This includes the api file that
you just moved.  I also want the directory in drivers/infiniband/hw to
be bnxt_re not bnxtre.  Also, putting bnxt_re as part of the file name
for files already in the bnxt_re directory is redundant (all except for
bnxt_re.h, where it is appropriate).  Just name the files things like
main.c and ib_verbs.c.  And for the qplib files, drop the bnxt_ prefix
and just use qplib_*.

I'll make other comments as I sort through the files, but those are
things I would like to see changed so I wanted to get that feedback to
you sooner rather than later.

> Thanks,
> Selvin Xavier
> 
> v2->v3:
>   * Fix 0day build breakage
>   * Fix cocci, kbuild robot, sparse, smatch and checkpatch warnings
>   * Changed the filename bnxt_re_uverbs_abi.h  to bnxtre-abi.h
>   * Removed the __packed qualifier from the uverbs structure and adjusted
>     the structure alignment to 64bits.
>   * Added retry count to bail out in case of delayed or no response
>     to FW commands
>   * Removed the debugfs support from this patch series
>   * Changed some of the defines as inline functions based on Jason's comment
>   * Split two functions to get rid of switch within switch construct
>   * Removed bnxt_re_copy_to_udata as it is just a wrapper for ib_copy_to_udata
>   * Added maintainers information to MAINTAINERS file
> 
> v1-> v2:
>   * The license text in each file updated to reflect Dual license.
>   * Makefile and Kconfig changes are pushed to the last patch
>   * Moved bnxt_re_uverbs_abi.h to include/uapi/rdma folder
>   * Remove duplicate structure definitions from bnxt_re_hsi.h as
>     it is available in the corresponding bnxt_en header file (bnxt_hsi.h)
>   * Removed some unused code reported during code review.
>   * Fixed few sparse warnings
> 
> 
> Selvin Xavier (21):
>   bnxt_re: Add bnxt_re RoCE driver files
>   bnxt_re: Introducing autogenerated Host Software Interface(hsi) file
>   bnxt_re: register with the NIC driver
>   bnxt_re: Enabling RoCE control path
>   bnxt_re: Adding Notification Queue support
>   bnxt_re: Support for PD, ucontext and mmap verbs
>   bnxt_re: Support for query and modify device verbs
>   bnxt_re: Adding support for port related verbs
>   bnxt_re: Support for GID related verbs
>   bnxt_re: Support for CQ verbs
>   bnxt_re: Support for AH verbs
>   bnxt_re: Support memory registration verbs
>   bnxt_re: Support QP verbs
>   bnxt_re: Support post_send verb
>   bnxt_re: Support post_recv
>   bnxt_re: Support poll_cq verb
>   bnxt_re: Handling dispatching of events to IB stack
>   bnxt_re: Support for DCB
>   bnxt_re: Set uverbs command mask
>   bnxt_re: Add QP event handling
>   bnxt_re: Add bnxt_re driver build support
> 
>  MAINTAINERS                                     |   11 +
>  drivers/infiniband/Kconfig                      |    2 +
>  drivers/infiniband/hw/Makefile                  |    1 +
>  drivers/infiniband/hw/bnxtre/Kconfig            |    9 +
>  drivers/infiniband/hw/bnxtre/Makefile           |    6 +
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.c    | 2167 +++++++++++++++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.h    |  441 ++++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_rcfw.c  |  692 +++++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_rcfw.h  |  231 ++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_res.c   |  825 ++++++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_res.h   |  223 ++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_sp.c    |  838 ++++++
>  drivers/infiniband/hw/bnxtre/bnxt_qplib_sp.h    |  160 ++
>  drivers/infiniband/hw/bnxtre/bnxt_re.h          |  150 ++
>  drivers/infiniband/hw/bnxtre/bnxt_re_hsi.h      | 2821 ++++++++++++++++++++
>  drivers/infiniband/hw/bnxtre/bnxt_re_ib_verbs.c | 3206 +++++++++++++++++++++++
>  drivers/infiniband/hw/bnxtre/bnxt_re_ib_verbs.h |  196 ++
>  drivers/infiniband/hw/bnxtre/bnxt_re_main.c     | 1340 ++++++++++
>  include/uapi/rdma/bnxtre-abi.h                  |   89 +
>  19 files changed, 13408 insertions(+)
>  create mode 100644 drivers/infiniband/hw/bnxtre/Kconfig
>  create mode 100644 drivers/infiniband/hw/bnxtre/Makefile
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.c
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_rcfw.c
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_rcfw.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_res.c
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_res.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_sp.c
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_qplib_sp.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_re.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_re_hsi.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_re_ib_verbs.c
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_re_ib_verbs.h
>  create mode 100644 drivers/infiniband/hw/bnxtre/bnxt_re_main.c
>  create mode 100644 include/uapi/rdma/bnxtre-abi.h
> 


-- 
Doug Ledford <dledford@...hat.com>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



Download attachment "signature.asc" of type "application/pgp-signature" (885 bytes)

Powered by blists - more mailing lists