Messages in this thread | | | Date | Thu, 18 Aug 2022 13:24:17 -0700 | Subject | Re: [RFC PATCH v2 1/2] scsi: ufs: Add Multi-Circular Queue support | From | Bart Van Assche <> |
| |
On 8/12/22 02:10, Manivannan Sadhasivam wrote: > On Thu, Aug 11, 2022 at 03:33:03AM -0700, Can Guo wrote: >> +static unsigned int dev_cmd_queue = 1; >> +static unsigned int read_queues; > > Where is this initialized?
The Linux kernel coding style does not allow to explicitly initialize static variables to zero.
>> + >> +static int rw_queue_count_set(const char *val, const struct kernel_param *kp) >> +{ >> + unsigned int n; >> + int ret; >> + >> + ret = kstrtouint(val, 10, &n); >> + if (ret) >> + return ret; >> + if (n > num_possible_cpus()) >> + return -EINVAL; >> + return param_set_uint(val, kp); >> +} >> + >> +static const struct kernel_param_ops rw_queue_count_ops = { >> + .set = rw_queue_count_set, >> + .get = param_get_uint, >> +}; >> + >> +static unsigned int rw_queues = 8; >> +module_param_cb(rw_queues, &rw_queue_count_ops, &rw_queues, 0644); >> +MODULE_PARM_DESC(rw_queues, "Number of queues used for rw. Default value is 8"); >> + > > Using module params is not encouraged these days. So please switch to Kconfig.
Hmm ... I think using CONFIG_* variables is worse than using module parameters since modifying CONFIG_* variables requires rebuilding the kernel.
>> +struct cq_entry { >> + /* DW 0-1 */ >> + __le32 command_desc_base_addr_lo; >> + __le32 command_desc_base_addr_hi; >> + >> + /* DW 2 */ >> + __le16 response_upiu_length; >> + __le16 response_upiu_offset; >> + >> + /* DW 3 */ >> + __le16 prd_table_length; >> + __le16 prd_table_offset; >> + >> + /* DW 4 */ >> + __le32 status; >> + >> + /* DW 5-7 */ >> + u32 reserved[3]; >> +}; > > packed?
Using __packed if it is not necessary is wrong since it makes code slower.
Thanks,
Bart.
| |