On Wed, Jun 12, 2019 at 09:51:59AM +0100, John Garry wrote: > On 11/06/2019 06:56, Leon Romanovsky wrote: > > On Tue, Jun 11, 2019 at 10:37:48AM +0800, wangxi wrote: > > > > > > > > > 在 2019/6/10 21:27, Jason Gunthorpe 写道: > > > > On Sat, Jun 08, 2019 at 10:24:15AM +0800, wangxi wrote: > > > > > > > > > > Why is there an EXPROT_SYMBOL in a IB driver? I see many in > > > > > > hns. Please send a patch to remove all of them and respin this. > > > > > > > > > > > There are 2 modules in our ib driver, one is hns_roce.ko, another > > > > > is hns_roce_hw_v2.ko. all extern symbols are named like hns_roce_xxx, > > > > > this function defined in hns_roce.ko, and invoked in > > > > > hns_roce_hw_v2.ko. > > > > > > > > seems unnecessarily complicated > > > > > > > > Jason > > > > . > > > > > > > Hi,Jason, > > > > > > The hns ib driver was originally designed for the hip06. When designing the > > > driver for the new hardware hip08, in order to maximize the reuse of the > > > existing hip06 code, the common part of the code is separated into the > > > hns_roce.ko, and the hardware difference code is defined into hns_roce_hw_v1.ko > > > for hip06 and hns_roce_hw_v2.ko for hip08. > > > > > > The mtr code is designed as a public part in this patchset, so it is defined > > > in hns_roce.ko. It can be used for hi16xx series hardware with mixed mutihop > > > addressing feature. Currently, hip08 supports this feature, so it is be called > > > in hns_roce_hw_v2.ko. > > > > Combine v1 and v2 into one driver (.ko) and change initialization to > > call v1 or v2 accordingly. The rest is handled by ib_device_ops > > structure. > > > > Is there a rule which says that a driver cannot export symbols? Module > stacking is useful for more complex drivers, in that a hw-specific > implementation may plug into common driver. This helps code reuse. Generally we do not like to see leaf drivers be so complicated that they need to export symbols. A multi-module driver is generally an over engineered thing to do, few drivers would be so big for that to make any sense. > In addition to this, v1 hw is a platform device driver and depends on HNS, > while v2 hw is for a PCI device and depends on PCI && HNS3. Attempts to > combine into a single ko would introduce messy dependencies and ifdefs. I suspect it would not be any different from how it is today. Do everything the same, just have one module not three. module_init/etc already take care of conditional compilation of the entire .c file via Makefile Jason