Re: [PATCH v6 3/7] scsi: ufs: Add ufs-bsg module

2018-09-27 Thread Christoph Hellwig 
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

2018-09-26 Thread Avri Altman 
Bart/Christoph,

> + */
> +int ufs_bsg_probe(struct ufs_hba *hba)
> +{
> + struct device *bsg_dev = &hba->bsg_dev;
> + struct Scsi_Host *shost = hba->host;
> + struct device *parent = &shost->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

2018-09-26 Thread Avri Altman 
> 
> 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

2018-09-26 Thread Avri Altman 


> 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

2018-09-26 Thread Martin K. Petersen 


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

2018-09-26 Thread Christoph Hellwig 
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

[PATCH v6 3/7] scsi: ufs: Add ufs-bsg module

2018-09-21 Thread Avri Altman 
Add a bsg endpoint that supports UPIUs.

For now, just provide an API to allocate and remove
ufs-bsg node. We will use this framework to
manage ufs devices by sending UPIU transactions.

For the time being, implements an empty bsg_request() -
will add some more functionality in coming patches.

Nonetheless, we reveal here the protocol we are planning to use:
UFS Transport Protocol Transactions. UFS transactions consist
of packets called UFS Protocol Information Units (UPIU).

There are UPIU’s defined for UFS SCSI commands, responses,
data in and data out, task management, utility functions,
vendor functions, transaction synchronization and control, and more.

By using UPIUs, we get access to the most fine-grained internals
of this protocol, and able to communicate with the device in ways,
that are sometimes beyond the capacity of the ufs driver.

Moreover and as a result, our core structure - ufs_bsg_node has
a pretty lean structure: using upiu transactions that contains
the outmost detailed info, so we don't really need complex
constructs to support it.

Signed-off-by: Avri Altman 
---
 Documentation/scsi/ufs.txt   |  20 
 drivers/scsi/ufs/Kconfig |  19 +++
 drivers/scsi/ufs/Makefile|   3 +-
 drivers/scsi/ufs/ufs.h   |  61 +-
 drivers/scsi/ufs/ufs_bsg.c   |  93 ++
 drivers/scsi/ufs/ufs_bsg.h   |  23 +
 drivers/scsi/ufs/ufshcd.c|   4 ++
 drivers/scsi/ufs/ufshcd.h|   3 ++
 include/uapi/scsi/scsi_bsg_ufs.h | 107 +++
 9 files changed, 272 insertions(+), 61 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs_bsg.c
 create mode 100644 drivers/scsi/ufs/ufs_bsg.h
 create mode 100644 include/uapi/scsi/scsi_bsg_ufs.h

diff --git a/Documentation/scsi/ufs.txt b/Documentation/scsi/ufs.txt
index 41a6164..2616d5f 100644
--- a/Documentation/scsi/ufs.txt
+++ b/Documentation/scsi/ufs.txt
@@ -128,6 +128,26 @@ The current UFSHCD implementation supports following 
functionality,
 In this version of UFSHCD Query requests and power management
 functionality are not implemented.
 
+4. BSG Support
+--
+
+This transport driver supports exchanging UFS protocol information units
+(UPIUs) with an UFS device. Typically, user space will allocate
+struct ufs_bsg_request and struct ufs_bsg_reply (see ufs_bsg.h) as
+request_upiu and reply_upiu respectively.  Filling those UPIUs should
+be done in accordance with JEDEC spec UFS2.1 paragraph 10.7.
+*Caveat emptor*: The driver makes no further input validations and sends the
+UPIU to the device as it is.  Open the bsg device in /dev/ufs-bsg and
+send SG_IO with the applicable sg_io_v4:
+
+   io_hdr_v4.guard = 'Q';
+   io_hdr_v4.protocol = BSG_PROTOCOL_SCSI;
+   io_hdr_v4.subprotocol = BSG_SUB_PROTOCOL_SCSI_TRANSPORT;
+   io_hdr_v4.response = (__u64)reply_upiu;
+   io_hdr_v4.max_response_len = reply_len;
+   io_hdr_v4.request_len = request_len;
+   io_hdr_v4.request = (__u64)request_upiu;
+
 UFS Specifications can be found at,
 UFS - http://www.jedec.org/sites/default/files/docs/JESD220.pdf
 UFSHCI - http://www.jedec.org/sites/default/files/docs/JESD223.pdf
diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index e09fe6a..83ba979 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -109,3 +109,22 @@ config SCSI_UFS_HISI
 
  Select this if you have UFS controller on Hisilicon chipset.
  If unsure, say N.
+
+config SCSI_UFS_BSG
+   bool "Universal Flash Storage BSG device node"
+   depends on SCSI_UFSHCD
+   select BLK_DEV_BSGLIB
+   help
+ Universal Flash Storage (UFS) is SCSI transport specification for
+ accessing flash storage on digital cameras, mobile phones and
+ consumer electronic devices.
+ An UFS controller communicates with an UFS device by exchanging
+ UFS Protocol Information Units (UPIUs).
+ UPIUs can not only be used as a transport layer for the SCSI protocol
+ but are also used by the UFS native command set.
+ This transport driver supports exchanging UFS protocol information 
units
+ with an UFS device. See also the ufshcd driver, which is a SCSI driver
+ that supports UFS devices.
+
+ Select this if you need a bsg device node for your UFS controller.
+ If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 2c50f03..aca4813 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -4,7 +4,8 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o 
ufshcd-dwc.o tc-dwc-g210.
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o 
tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
-ufshcd-core-objs := ufshcd.o ufs-sysfs.o
+ufshcd-core-y  += ufshcd.o ufs-sysfs.o
+ufshcd-core