Re: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
On Thu, Sep 27, 2018 at 06:16:44AM +, Avri Altman wrote: > In V6, we removed the host and device indices from the bsg device name, > But I have some seconds thoughts about it. > > We are using the bsg device in passthrough mode (bsg_transport_ops), > But the device name: "ufs-bsg" does not imply that. > > Given that the ABI should never change, > if someone in the future will want to add a bsg device that uses the > bsg_scsi_ops, > ufs-bsg-scsi seems a little bit awkward, does it? We should already have a bsg_scsi_ops instance for every SCSI LU, so they already exist - without any bsg in the name. I think ufs-bsg is ok.
RE: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
Bart/Christoph,
> + */
> +int ufs_bsg_probe(struct ufs_hba *hba)
> +{
> + struct device *bsg_dev = >bsg_dev;
> + struct Scsi_Host *shost = hba->host;
> + struct device *parent = >shost_gendev;
> + struct request_queue *q;
> + int ret;
> +
> + device_initialize(bsg_dev);
> +
> + bsg_dev->parent = get_device(parent);
> + bsg_dev->release = ufs_bsg_node_release;
> +
> + dev_set_name(bsg_dev, "ufs-bsg");
In V6, we removed the host and device indices from the bsg device name,
But I have some seconds thoughts about it.
We are using the bsg device in passthrough mode (bsg_transport_ops),
But the device name: "ufs-bsg" does not imply that.
Given that the ABI should never change,
if someone in the future will want to add a bsg device that uses the
bsg_scsi_ops,
ufs-bsg-scsi seems a little bit awkward, does it?
What do you think?
Thanks,
Avri
> +
> + ret = device_add(bsg_dev);
> + if (ret)
> + goto out;
> +
> + q = bsg_setup_queue(bsg_dev, dev_name(bsg_dev), ufs_bsg_request,
> 0);
RE: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
> > Avri, > > > this looks generally good to me, but I'd suggest two small tweaks: > > > > - please split out a new prep patch that creates > >include/uapi/scsi/scsi_bsg_ufs.h with the structures move there > > - pleae keep the copyrights from drivers/scsi/ufs/ufs.h in this > >new file > > Also, when you repost, please make sure to carry over Reviewed-by: tags > received for patches you haven't (substantially) changed. > > Looking over these yesterday, I noticed they were a blank slate in the > tags department and that's very unusual for a 6th iteration of a patch > set. Done. Thanks, Avri > > -- > Martin K. PetersenOracle Linux Engineering
RE: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
> Hi Avri, > > this looks generally good to me, but I'd suggest two small tweaks: > > - please split out a new prep patch that creates >include/uapi/scsi/scsi_bsg_ufs.h with the structures move there > - pleae keep the copyrights from drivers/scsi/ufs/ufs.h in this >new file Done. Thanks, Avri
Re: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
Avri, > this looks generally good to me, but I'd suggest two small tweaks: > > - please split out a new prep patch that creates >include/uapi/scsi/scsi_bsg_ufs.h with the structures move there > - pleae keep the copyrights from drivers/scsi/ufs/ufs.h in this >new file Also, when you repost, please make sure to carry over Reviewed-by: tags received for patches you haven't (substantially) changed. Looking over these yesterday, I noticed they were a blank slate in the tags department and that's very unusual for a 6th iteration of a patch set. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module
Hi Avri, this looks generally good to me, but I'd suggest two small tweaks: - please split out a new prep patch that creates include/uapi/scsi/scsi_bsg_ufs.h with the structures move there - pleae keep the copyrights from drivers/scsi/ufs/ufs.h in this new file
