Differential D14514
Microsemi smartpqi SCSI driver for PQI controllers Authored by deepak.ukey_microsemi.com on Feb 26 2018, 8:58 AM. Tags None Referenced Files
Details
The driver provides support for the new generation of PQI controllers from Microsemi. This driver is the first SCSI driver to implement the PQI queuing model and it will replace the aacraid driver for Adaptec Series 9 controllers.
Microsemi Product verification group has developed the test plan and executed them so that there is maximum code path coverage.
Diff Detail
Event TimelineComment Actions Hi, This is the Microsemi Smartpqi SCSI driver for PQI controllers. Please review it. Regards, Comment Actions Currently we created a custom policy so that only reviewer can see it. Once review completes, we will make it public. Comment Actions I've added a bunch of review comments. Nothing super major. The man page should indicate what loader.conf or sysctl.conf tuneables if any are available. It should also indicate what compile time options if any are available for this driver.
Comment Actions I agree all the other review comments and fixed them. But need some clarifications on the below.
This file seems like an overuse of include files. Since this is included in almost every file in the driver, it seems like we are including the header files many, many times. This does lead to bloat in the driver but is functionally harmless. ----> We actually have two sets of header files – OS specific and PQI protocol specific. We tried to consolidate them in one header file and use it. So could you please help us out by elaborating what could be the best solution here.
I suspect that this will not work on Big Endian architectures (but I doubt microsemi is supporting non-x86 architectures with this driver). Should we add a restriction to only AMD64 and I386 for this driver? ------>SCSI3Addr_struct etc are the objects used for pass-through ioctl between management software and Firmware. So from driver perspective, ideally we should not be facing the issue. Please correct if we are wrong. Comment Actions I don't think its an "important" review comment. It would be better if each file only included freebsd headers that it needed so that it could be easier to determine what is being used in each file. However, since you are maintaining a common code base in an effort to reduce work, I think this is fine.
In a big-endian system, you would get a struct passed down that would be in an inverted order, right? If you are using the bit operators to divide up your elements, they wouldn't be in the right place. Its not important if you do not wish to support big-endian CPUs, but you should indicate that by only allowing your driver to be used on little-endian CPUs. Does this make sense?
Comment Actions I have Updated the diff with the changes according to comments. Please review it. About sysctl.conf, Currently we don’t have sysctl.conf tumble parameters. We may add it in future patch.
Comment Actions Updated the diff. Please review it.
Comment Actions Huh, ok. This fails to build on TARGET=i386. I'm not sure if you want to fix this or restrict it to AMD64 only: make buildkernel TARGET=i386
...
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:339:21: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
signature = LE_64(PCI_MEM_GET64(softs, &softs->pqi_reg->signature, PQI_SIGNATURE));
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:339:21: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:359:13: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
COND_WAIT((PCI_MEM_GET64(softs, &softs->pqi_reg->admin_q_config,
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:476:8: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
val = PCI_MEM_GET64(softs, &softs->pqi_reg->pqi_dev_adminq_cap, PQI_ADMINQ_CAP);
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:805:2: error: incompatible pointer types passing 'OS_ATOMIC64_T *' (aka 'volatile unsigned long long *') to parameter of type 'volatile u_long *' (aka 'volatile unsigned long *') [-Werror,-Wincompatible-pointer-types]
OS_ATOMIC64_SET(softs, num_intrs, 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:864:62: note: expanded from macro 'OS_ATOMIC64_SET'
#define OS_ATOMIC64_SET(_softs, target, val) atomic_set_long(&(_softs)->target, val)
^~~~~~~~~~~~~~~~~
./machine/atomic.h:603:1: note: passing argument to parameter 'p' here
ATOMIC_ASM(set, long, "orl %1,%0", "ir", v);
^
./machine/atomic.h:109:48: note: expanded from macro 'ATOMIC_ASM'
void atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v); \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_init.c:819:33: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
softs->prev_heartbeat_count = CTRLR_HEARTBEAT_CNT(softs) - OS_FW_HEARTBEAT_TIMER_INTERVAL;
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:267:43: note: expanded from macro 'CTRLR_HEARTBEAT_CNT'
#define CTRLR_HEARTBEAT_CNT(softs) LE_64(PCI_MEM_GET64(softs, softs->heartbeat_counter_abs_addr, softs->heartbeat_counter_off))
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
6 errors generated.
--- smartpqi_init.o ---
*** [smartpqi_init.o] Error code 1
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/smartpqi
A failure has been detected in another branch of the parallel make
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/sn
--- all_subdir_sn ---
*** [all_subdir_sn] Error code 2
make[3]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:111:14: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
val = LE_64(PCI_MEM_GET64(softs, &softs->pqi_reg->pqi_dev_adminq_cap, PQI_ADMINQ_CAP));
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:111:14: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:233:2: error: implicit declaration of function 'bus_space_write_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
PCI_MEM_PUT64(softs, &softs->pqi_reg->admin_q_config, PQI_ADMINQ_CONFIG, LE_64(cmd));
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:882:5: note: expanded from macro 'PCI_MEM_PUT64'
bus_space_write_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:233:2: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:882:5: note: expanded from macro 'PCI_MEM_PUT64'
bus_space_write_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:241:13: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
COND_WAIT((PCI_MEM_GET64(softs, &softs->pqi_reg->admin_q_config, PQI_ADMINQ_CONFIG) ==
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:302:2: error: implicit declaration of function 'bus_space_write_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
PCI_MEM_PUT64(softs, &softs->pqi_reg->admin_ibq_elem_array_addr,
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:882:5: note: expanded from macro 'PCI_MEM_PUT64'
bus_space_write_8(_softs->pci_mem_handle.pqi_btag, \
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_queue.c:337:9: error: implicit declaration of function 'bus_space_read_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
PCI_MEM_GET64(softs, &softs->pqi_reg->admin_ibq_pi_offset, PQI_ADMIN_IBQ_PI_OFFSET));
^
/home/sbruno/bsd/fbsd_head/sys/modules/smartpqi/../../dev/smartpqi/smartpqi_defines.h:874:5: note: expanded from macro 'PCI_MEM_GET64'
bus_space_read_8(_softs->pci_mem_handle.pqi_btag, \
^
7 errors generated.
--- smartpqi_queue.o ---
*** [smartpqi_queue.o] Error code 1
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/smartpqi
2 errors
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/smartpqi
--- all_subdir_smartpqi ---
*** [all_subdir_smartpqi] Error code 2
make[3]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules
A failure has been detected in another branch of the parallel make
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/smbfs
--- all_subdir_smbfs ---
*** [all_subdir_smbfs] Error code 2
make[3]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules
A failure has been detected in another branch of the parallel make
make[4]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules/rtwn
--- all_subdir_rtwn ---
*** [all_subdir_rtwn] Error code 2
make[3]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules
4 errors
make[3]: stopped in /home/sbruno/bsd/fbsd_head/sys/modules
--- modules-all ---
*** [modules-all] Error code 2
make[2]: stopped in /var/tmp/home/sbruno/bsd/fbsd_head/i386.i386/sys/GENERIC-NODEBUG
1 error
make[2]: stopped in /var/tmp/home/sbruno/bsd/fbsd_head/i386.i386/sys/GENERIC-NODEBUG
--- buildkernel ---
*** [buildkernel] Error code 2
make[1]: stopped in /home/sbruno/bsd/fbsd_head
1 error
make[1]: stopped in /home/sbruno/bsd/fbsd_head
--- buildkernel ---
*** [buildkernel] Error code 2
make: stopped in /home/sbruno/bsd/fbsd_head
1 error
make: stopped in /home/sbruno/bsd/fbsd_headComment Actions Currently Smartpqi driver is not qualified with i386 architecture so for now it will support only AMD64. We are planning to add support for i386 in future. Comment Actions Ok, so how about something like this. Move the smartpqi entries from sys/conf/files to sys/conf/files.amd64 and restrict sys/modules/Makefile to amd64 only: Index: sys/modules/Makefile =================================================================== --- sys/modules/Makefile (revision 332894) +++ sys/modules/Makefile (working copy) @@ -355,6 +355,7 @@ siis \ sis \ sk \ + ${_smartpqi} \ smbfs \ sn \ snp \ @@ -729,6 +730,7 @@ _sfxge= sfxge _sgx= sgx _sgx_linux= sgx_linux +_smartpqi= smartpqi .if ${MK_BHYVE} != "no" || defined(ALL_MODULES) _vmm= vmm Comment Actions As suggested Restricted the driver to only amd64 and also Moved the smartpqi entries from sys/conf/files to sys/conf/files.amd64. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||