Re: [PATCH] spraid: initial commit of Ramaxel spraid driver

[Bart Van Assche <bvanassche@acm.org>]

On 9/29/21 20:47, Yanling Song wrote:
> +#define SPRAID_IOCTL_RESET_CMD _IOWR('N', 0x80, struct spraid_passthru_common_cmd)
> +#define SPRAID_IOCTL_ADMIN_CMD _IOWR('N', 0x41, struct spraid_passthru_common_cmd)

Do these new ioctls provide any functionality that is not yet provided 
by SG_IO + SG_SCSI_RESET_BUS?

Additionally, mixing driver-internal and user space definitions in a 
single header file is not OK. Definitions of data structures and ioctls 
that are needed by user space software should occur in a header file in 
the directory include/uapi/scsi/.

> +#define SPRAID_IOCTL_IOQ_CMD _IOWR('N', 0x42, struct spraid_ioq_passthru_cmd)

What functionality does this ioctl provide that is not yet provided by 
SG_IO?

> +#define SPRAID_DRV_VERSION	"1.0.0.0"

Although many Linux kernel drivers include a version number, a version 
number is only useful in an out-of-tree driver and not in an upstream 
driver. The Linux kernel itself already has a version number.

> +MODULE_AUTHOR("Ramaxel Memory Technology");

My understanding is that the MODULE_AUTHOR argument should mention the 
name of a person. From include/linux/module.h:

/*
  * Author(s), use "Name <email>" or just "Name", for multiple
  * authors use multiple MODULE_AUTHOR() statements/lines.
  */
#define MODULE_AUTHOR(_author) MODULE_INFO(author, _author)

Thanks,

Bart.