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

[Yanling Song <songyl@ramaxel.com>]

On Tue, 19 Oct 2021 20:24:45 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> On 10/19/21 17:33, Yanling Song wrote:
> > On Wed, 13 Oct 2021 15:00:07 -0700
> > Bart Van Assche <bvanassche@acm.org> wrote:  
> >> 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 wish there was a standard way to address the ioctrl issue.
> > Unfortunately ioctrl is the only way to meet our requirements as
> > listed in the above.
> > As discussed in previous megaraid's patchsets:
> > https://lore.kernel.org/linux-scsi/yq1inc2y019.fsf@oracle.com/,
> > that's why every raid controller has it's own ioctrl.  
> 
> Why are ioctls the only solution? Why is a bsg interface attached to
> the SCSI host not appropriate? I haven't found the answer in the 
> conversation about the Megaraid driver.
>
Hi Bart,

We've studied BSG and in general it can work.

The following is our draft design, please give your comments:
1. Applications from user space send commands to driver thru struct
sg_io_v4, the private data(used by driver) is saved in sg_io_v4->request
and the data length is saved in sg_io_v4->request_len.

2. SG_IO is used in bsg_ioctl(), the following has to be set because
bsg_transport_check_proto() will check the fields:
    sg_io_v4->protocol = BSG_PROTOCOL_SCSI;
    sg_io_v4->subprotocol = BSG_SUB_PROTOCOL_SCSI_TRANSPORT;

Does the above match the BSG design?

And one question:
The number of queue and queue depth are hardcoded in bsg_setup_queue().
       set->nr_hw_queues = 1;
       set->queue_depth = 128;

Any reason to do it? how about it does not match the chip's capability?
for example, the chip supports 8 hardware queues and each queue depth
is 4096?
 
> Thanks,
> 
> Bart.
> 
>