build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33582 )
Change subject: libpayload: nvme driver ......................................................................
Patch Set 1:
(49 comments)
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... File payloads/libpayload/drivers/storage/nvme.c:
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 13: #define NVME_CC_EN (1 << 0) please, no space before tabs
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 44: static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 52: static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 53: static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 67: static ssize_t nvme_read_blocks512(struct storage_dev *dev, lba_t start, size_t count, unsigned char *buf) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 74: if (read((struct nvme_dev*)dev, buffer+(i*512), start + i, 1)) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 85: static int read(struct nvme_dev *nvme, void *buffer, uint64_t base, uint16_t count) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 108: void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 110: printf("NVMe ERROR: Faild to allocate memory for io submission queue.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 110: printf("NVMe ERROR: Faild to allocate memory for io submission queue.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 131: nvme->queue[ios].bell = nvme->config + 0x1000 + (ios * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 143: void *const cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); 'competion' may be misspelled - perhaps 'completion'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 145: printf("NVMe ERROR: Faild to allocate memory for io competion queue.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 166: nvme->queue[ioc].bell = nvme->config + 0x1000 + (ioc * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 173: static int nvme_cmd(struct nvme_dev *nvme, enum nvme_queue q, const struct nvme_s_queue_entry *cmd) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 177: void *s_entry = nvme->queue[sq].base + (nvme->queue[sq].idx * NVME_SQ_ENTRY_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 182: struct nvme_c_queue_entry *c_entry = nvme->queue[cq].base + (nvme->queue[cq].idx * NVME_CQ_ENTRY_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 183: while (((c_entry->dw[3] >> 16 ) & 0x1) == nvme->queue[cq].round) space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 204: void *sq_buffer = memalign(0x1000, NVME_SQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 206: printf("NVMe ERROR: faild to allocated memory for admin submission queue\n"); 'faild' may be misspelled - perhaps 'failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 206: printf("NVMe ERROR: faild to allocated memory for admin submission queue\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 213: nvme->queue[ads].bell = nvme->config + 0x1000 + (ads * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 215: trailing whitespace
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 216: void *cq_buffer = memalign(0x1000, NVME_CQ_ENTRY_SIZE * NVME_QUEUE_SIZE); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 218: printf("NVMe ERROR: Faild to allocate memory for admin completion queue\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 218: printf("NVMe ERROR: Faild to allocate memory for admin completion queue\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 226: nvme->queue[adc].bell = nvme->config + 0x1000 + (adc * (4 << cap_dstrd)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 235: printf("NVMe init (Device %02x:%02x.%02x)\n", PCI_BUS(dev), PCI_SLOT(dev), PCI_FUNC(dev)); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 239: if ( !(((read64(pci_bar0) >> 37 ) & 0xff) == 0x01)) { space prohibited after that open parenthesis '('
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 239: if ( !(((read64(pci_bar0) >> 37 ) & 0xff) == 0x01)) { space prohibited before that close parenthesis ')'
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 240: printf("NVMe ERROR: PCIe device does not support the NVMe command set\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 246: printf("NVMe ERROR: Faild to allocate buffer for nvme driver struct\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 246: printf("NVMe ERROR: Faild to allocate buffer for nvme driver struct\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 263: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 263: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 267: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 267: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 277: cc = NVME_CC_EN | NVME_CC_CSS | NVME_CC_MPS | NVME_CC_AMS |NVME_CC_SHN need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 285: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 285: printf("NVMe ERROR: Faild to disable controller. FATAL ERROR\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 289: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); 'Faild' may be misspelled - perhaps 'Failed'?
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 289: printf("NVMe ERROR: Faild to disable controller. Timeout.\n"); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 304: storage_attach_device((storage_dev_t*)nvme); "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 315: } void function return statements are not generally useful
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/drivers/storage/... PS1, Line 325: class = pci_read_config16(PCI_DEV(bus, dev, func), 0xa); line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... File payloads/libpayload/include/x86/arch/io.h:
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... PS1, Line 67: static inline __attribute__((always_inline)) uint64_t read64(const volatile void *addr) line over 80 characters
https://review.coreboot.org/#/c/33582/1/payloads/libpayload/include/x86/arch... PS1, Line 87: static inline __attribute__((always_inline)) void write64(volatile void *addr, uint64_t value) line over 80 characters