On Mon, Sep 12, 2016 at 05:39:35PM +0000, Yuval Mintz wrote: > >>> include/linux/qed/common_hsi.h | 1 + > >>> include/linux/qed/qed_if.h | 9 +- > >>> include/linux/qed/qed_ll2_if.h | 140 + > >>> include/linux/qed/qed_roce_if.h | 604 ++++ > >>> include/linux/qed/qede_roce.h | 88 + > >> > include/linux/qed/rdma_common.h | 1 + > >> > >> Something not directly related to your patches, but they brought my > >> attention to the fact that all these new (and old) rdma<->net devices > >> are polluting include/linux > >> > > ocrdma driver includes be_roce.h located in net/ethernet/emulex/benet > > location instead of include/linux/. > > This file helps to bind rdma to net device or underlying hw device. > > > May be similar change can be done for rest of the drivers for > > rdma<-->net devices? > > By adding explicit inclusion paths in the Makefile, a la > ccflags-y := -Idrivers/net/ethernet/emulex/benet ? > > While this might work, I personally dislike it as I find it > counter-intuitive when going over the code - > I don't expect driver to locally modify the inclusion path. > Besides, we're going to [eventually] a whole suite of drivers based > on the qed module, some of which would reside under drivers/scsi; > Not sure it's best to have 3 or 4 different drivers privately include the > same directory under a different subsystem.
I agree with you that orcdma's way can be valuable for small drivers. Orcmda has small shared headers set and doesn't need to change them rapidly to support different devices. I thought to place them in similar directory to include/soc/* and remove from include/linux/. We have include/rdma/ and it looks like a good candidate. > > >> Filtered output: > >> ➜ linux-rdma git:(topic/fixes-for-4.8-2) ls -dl include/linux/*/ > >> drwxrwxr-x 2 leonro leonro 4096 Aug 30 16:27 include/linux/hsi/ > >> drwxrwxr-x 2 leonro leonro 4096 Sep 12 19:08 include/linux/mlx4/ > >> drwxrwxr-x 2 leonro leonro 4096 Sep 7 15:31 include/linux/mlx5/ > >> drwxrwxr-x 2 leonro leonro 4096 Sep 8 17:46 include/linux/qed/ > >> > >> Is this the right place for them? > > > > Thanks >
signature.asc
Description: PGP signature