[Deepin-Kernel-SIG] [linux 6.6-y] [3SNIC] SCSI: SSSRAID: Support 3SNIC 3S5XX serial RAID/HBA#1355
Conversation
Add 3SNIC SSSRAID controller support. Co-developed-by: fei zhigang <feizg@3snic.com> Signed-off-by: fei zhigang <feizg@3snic.com> Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
There was a problem hiding this comment.
Sorry @Avenger-285714, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
There was a problem hiding this comment.
Pull request overview
This pull request adds support for the 3SNIC SSSRAID controller driver, a SCSI RAID driver for 3S5XX series controllers. The driver provides support for PCI Express Gen4 interface controllers with SAS/SATA HDD/SSD support, implementing both HBA and RAID functionality.
Key changes:
- New SCSI driver implementation with ~4,159 lines of code across multiple files
- Complete controller initialization, queue management, and I/O path implementation
- Support for admin and I/O command queues with interrupt handling
- Device discovery, management, and event handling capabilities
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/scsi/sssraid/sssraid_os.c | Main driver file with SCSI callbacks, I/O handling, and device management |
| drivers/scsi/sssraid/sssraid_fw.c | Firmware interaction, queue setup, and admin command handling |
| drivers/scsi/sssraid/sssraid.h | Driver structures, constants, and function declarations |
| drivers/scsi/sssraid/sssraid_debug.h | Debug logging macros |
| drivers/scsi/sssraid/Kconfig | Kernel configuration for the driver |
| drivers/scsi/sssraid/Makefile | Build configuration |
| drivers/scsi/Kconfig | Integration into SCSI subsystem |
| drivers/scsi/Makefile | Build system integration |
| MAINTAINERS | Maintainer information for the driver |
| Documentation/scsi/sssraid.rst | Driver documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the zero-length array 'event_data[0]' with a C99 flexible array
member 'event_data[]' in struct sssraid_fwevt.
Zero-length arrays are a GNU extension that predates C99 flexible array
members. The kernel's fortify-string checks treat zero-length arrays as
having size 0, which causes false-positive warnings when memcpy() is used
to copy data into them:
In function 'fortify_memcpy_chk',
inlined from 'sssraid_process_admin_cq.isra' at
drivers/scsi/sssraid/sssraid_fw.c:1574:3:
./include/linux/fortify-string.h:597:25: error: call to
'__write_overflow_field' declared with attribute warning: detected
write beyond size of field (1st parameter); maybe use struct_group()?
This is part of the ongoing effort to replace all zero-length arrays with
flexible array members, as recommended by the kernel coding style.
Link: KSPP/linux#78
Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
deepin pr auto review我来帮你分析这个SCSI RAID驱动的代码。这个驱动实现了对3SNIC RAID控制器的支持。我会从几个方面进行分析:
代码组织良好,主要分为几个关键文件:
a) 队列管理
b) 设备管理
c) 错误处理
优点:
潜在问题:
a) 代码质量改进: // 建议在sssraid_setup_prps中添加更严格的边界检查
if (unlikely(length <= 0 || length > max_transfer_length)) {
ioc_err(sdioc, "Invalid transfer length: %d\n", length);
return -EINVAL;
}b) 性能优化: // 建议优化队列分配逻辑,减少内存碎片
static int sssraid_alloc_qpair(struct sssraid_ioc *sdioc, u16 qidx, u16 depth)
{
// 使用连续内存分配
void *mem = dma_alloc_coherent(&sdioc->pdev->dev,
CQ_SIZE(depth) + SQ_SIZE(qidx, depth),
&dma_addr, GFP_KERNEL);
if (!mem)
return -ENOMEM;
// 分配CQ和SQ在连续内存中
cqinfo->cqes = mem;
sqinfo->sq_cmds = mem + CQ_SIZE(depth);
// ...
}c) 安全性增强: // 建议添加更严格的输入验证
static int sssraid_user_admin_cmd(struct sssraid_ioc *sdioc, struct bsg_job *job)
{
if (!job || !job->request || job->request_len != sizeof(struct sssraid_bsg_request)) {
ioc_err(sdioc, "Invalid BSG request\n");
return -EINVAL;
}
// 验证命令参数
if (cmd->timeout_ms > MAX_TIMEOUT_MS) {
ioc_err(sdioc, "Invalid timeout value: %u\n", cmd->timeout_ms);
return -EINVAL;
}
// ...
}d) 错误处理改进: // 建议完善错误恢复机制
static int sssraid_scsi_reset(struct scsi_cmnd *scmd, enum sssraid_scsi_rst_type rst)
{
int ret;
unsigned long flags;
spin_lock_irqsave(&sdioc->state_lock, flags);
if (sdioc->state != SSSRAID_LIVE) {
spin_unlock_irqrestore(&sdioc->state_lock, flags);
return FAILED;
}
// 添加重试机制
for (int retry = 0; retry < MAX_RESET_RETRIES; retry++) {
ret = sssraid_send_reset_cmd(sdioc, rst, hostdata->hdid);
if (ret == 0)
break;
msleep(RESET_RETRY_DELAY);
}
spin_unlock_irqrestore(&sdioc->state_lock, flags);
// ...
}
a) 添加更多调试信息: #define ioc_debug(ioc, fmt, ...) \
do { \
if (ioc->logging_level & SSSRAID_DEBUG) \
dev_dbg(&ioc->pdev->dev, fmt, ##__VA_ARGS__); \
} while (0)b) 优化内存使用: // 使用内存池减少动态分配
static struct kmem_cache *sssraid_cmd_cache;
static int sssraid_init_cmd_cache(void)
{
sssraid_cmd_cache = kmem_cache_create("sssraid_cmd",
sizeof(struct sssraid_cmd),
0, SLAB_HWCACHE_ALIGN, NULL);
return sssraid_cmd_cache ? 0 : -ENOMEM;
}c) 改进热插拔支持: // 添加设备状态监控
static void sssraid_monitor_devices(struct work_struct *work)
{
struct sssraid_ioc *sdioc = container_of(work, struct sssraid_ioc, monitor_work);
// 定期检查设备状态
sssraid_check_device_status(sdioc);
// 重新调度监控
schedule_delayed_work(&sdioc->monitor_work,
msecs_to_jiffies(DEVICE_MONITOR_INTERVAL));
}这些改进建议主要围绕:
建议在实现这些改进时进行充分的测试,确保不会影响现有功能的稳定性。 |
|
Tested and passed on x86 using LLVM-20. I don’t have Deepin’s development environment (build toolchain), so I can’t update the defconfig for all architectures myself — could the Deepin team please handle this? Many thanks! @opsiff |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: opsiff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
deepin inclusion
category: feature
SSSRAID - 3SNIC SCSI RAID Controller driver
PCI ID (pci.ids) OEM Product
1F3F:2100 3SNIC 3S510(HBA:8Ports,1G DDR)
1F3F:2100 3SNIC 3S520(HBA:16Ports,1G DDR)
1F3F:2100 3SNIC 3S530(HBA:32Ports,1G DDR)
1F3F:2100 3SNIC 3S540(HBA:40Ports,1G DDR)
1F3F:2200 3SNIC 3S580(RAID:16Ports,2G cache)
1F3F:2200 3SNIC 3S585(RAID:16Ports,4G cache)
1F3F:2200 3SNIC 3S590(RAID:32Ports,4G cache)
1F3F:2200 3SNIC 3S5A0(RAID:40Ports,2G cache)
1F3F:2200 3SNIC 3S5A5(RAID:40Ports,4G cache)
pci_device_id:
{ PCI_DEVICE(PCI_VENDOR_ID_3SNIC_LOGIC, SSSRAID_SERVER_DEVICE_HBA_DID) },
{ PCI_DEVICE(PCI_VENDOR_ID_3SNIC_LOGIC, SSSRAID_SERVER_DEVICE_RAID_DID) },
Link: deepin-community#1355
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
| tristate "3SNIC sssraid Adapter" | ||
| depends on PCI && SCSI | ||
| select BLK_DEV_BSGLIB | ||
| depends on ARM64 || X86_64 |
There was a problem hiding this comment.
Note: that was my oversight. There should be at least a "|| COMPILE_TEST" here.
deepin inclusion
category: feature
SSSRAID - 3SNIC SCSI RAID Controller driver
PCI ID (pci.ids) OEM Product
1F3F:2100 3SNIC 3S510(HBA:8Ports,1G DDR)
1F3F:2100 3SNIC 3S520(HBA:16Ports,1G DDR)
1F3F:2100 3SNIC 3S530(HBA:32Ports,1G DDR)
1F3F:2100 3SNIC 3S540(HBA:40Ports,1G DDR)
1F3F:2200 3SNIC 3S580(RAID:16Ports,2G cache)
1F3F:2200 3SNIC 3S585(RAID:16Ports,4G cache)
1F3F:2200 3SNIC 3S590(RAID:32Ports,4G cache)
1F3F:2200 3SNIC 3S5A0(RAID:40Ports,2G cache)
1F3F:2200 3SNIC 3S5A5(RAID:40Ports,4G cache)
pci_device_id:
{ PCI_DEVICE(PCI_VENDOR_ID_3SNIC_LOGIC, SSSRAID_SERVER_DEVICE_HBA_DID) },
{ PCI_DEVICE(PCI_VENDOR_ID_3SNIC_LOGIC, SSSRAID_SERVER_DEVICE_RAID_DID) },
Link: #1355
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
[ Upstream commit 1f029b4 ] When PA Create Sync is enabled, advertising resumes unexpectedly. Therefore, it's necessary to check whether advertising is currently active before attempting to pause it. < HCI Command: LE Add Device To... (0x08|0x0011) plen 7 deepin-community#1345 [hci0] 48.306205 Address type: Random (0x01) Address: 4F:84:84:5F:88:17 (Resolvable) Identity type: Random (0x01) Identity: FC:5B:8C:F7:5D:FB (Static) < HCI Command: LE Set Address Re.. (0x08|0x002d) plen 1 deepin-community#1347 [hci0] 48.308023 Address resolution: Enabled (0x01) ... < HCI Command: LE Set Extended A.. (0x08|0x0039) plen 6 deepin-community#1349 [hci0] 48.309650 Extended advertising: Enabled (0x01) Number of sets: 1 (0x01) Entry 0 Handle: 0x01 Duration: 0 ms (0x00) Max ext adv events: 0 ... < HCI Command: LE Periodic Adve.. (0x08|0x0044) plen 14 deepin-community#1355 [hci0] 48.314575 Options: 0x0000 Use advertising SID, Advertiser Address Type and address Reporting initially enabled SID: 0x02 Adv address type: Random (0x01) Adv address: 4F:84:84:5F:88:17 (Resolvable) Identity type: Random (0x01) Identity: FC:5B:8C:F7:5D:FB (Static) Skip: 0x0000 Sync timeout: 20000 msec (0x07d0) Sync CTE type: 0x0000 Fixes: ad383c2 ("Bluetooth: hci_sync: Enable advertising when LL privacy is enabled") Signed-off-by: Yang Li <yang.li@amlogic.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org> (cherry picked from commit 529281206f1167d875a8dde4a9eb85298d40fcf4)
Add 3SNIC SSSRAID controller support.
Co-developed-by: fei zhigang feizg@3snic.com