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

On Wed, 13 Oct 2021 15:00:07 -0700
Bart Van Assche [off-list ref] wrote:

On 10/12/21 11:50 PM, Yanling Song wrote:

quoted

On Tue, 12 Oct 2021 09:59:30 -0700
Bart Van Assche [off-list ref] wrote:  

quoted

Why is it that SG_IO is not sufficient? This is something that
should have been explained in the patch description.  

There are two cases that there are no SG devices and SG_IO cannot
work. 1. To access raid controller:
a. Raid controller is a scsi host, not a scsi device, so there
is no SG device associated with it.
b. Even there is a scsi device for raid controller, SG_IO
cannot work when something wrong with IO queue and only admin queue
can work;
2. To access the physical disks behinds raid controller:
raid controller only reports VDs to OS and only VDs have SG
devices. OS has no idea about physical disks behinds raid
controller and there is no SG devices associated with physical
disks.  

Please take a look at the bsg_setup_queue() call in ufs_bsg_probe(). 
That call associates a BSG queue with the UFS host. That queue
supports requests of type struct ufs_bsg_request. The Fibre Channel
transport driver does something similar. I believe that this is a
better solution than introducing entirely new ioctls.

I need some time to investigate it. will response later.

quoted

quoted

quoted

quoted

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/.  

Sounds reasonable. But after checking the directory
include/uapi/scsi/, there are only several files in it. It is
expected that there should be many files if developers follow the
rule. Do you know why?  

If this rule is not followed, that will be a red flag for the SCSI
maintainer and something that will probably delay upstream
acceptance of this patch.  

Since there are not much examples in include/uapi/scsi/, what' your
suggestion on how to put the definitions into the folder? for
example, what's the file name? spraid_ioctrl.h?  

How about include/uapi/scsi/spraid.h, since I assume that that header 
file will cover all user space interfaces for interaction with the 
spraid driver?

Thanks,

Bart.