linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/3] add support for DWC UFS Controller
@ 2016-02-15 15:25 Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 1/3] fixed typo in ufshcd-pltfrm Joao Pinto
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Joao Pinto @ 2016-02-15 15:25 UTC (permalink / raw)
  To: vinholikatti, julian.calaby, akinobu.mita, hch, arnd, mark.rutland
  Cc: gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree, linux-kernel,
	linux-scsi, devicetree, Joao Pinto

The work consisted of:
- Fixed typo in ufshcd-pltfrm.c
- Tweak ufshcd.c for UFS 2.0 support
- Implement ufshcd-dwc which contains all DWC HW specific code
- Unipro attributes were added and new registers were added to the driver
- Implement a ufs-dwc glue platform driver
- Implement a ufs-dwc-pci glue pci driver
- Documentation update

Joao Pinto (3):
  fixed typo in ufshcd-pltfrm
  added UFS 2.0 capabilities
  add support for DWC UFS Host Controller

 Documentation/devicetree/bindings/ufs/ufs-dwc.txt  |  19 +
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |   4 +-
 MAINTAINERS                                        |   6 +
 drivers/scsi/ufs/Kconfig                           |  51 +++
 drivers/scsi/ufs/Makefile                          |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                     | 172 ++++++++
 drivers/scsi/ufs/ufs-dwc.c                         |  96 +++++
 drivers/scsi/ufs/ufshcd-dwc.c                      | 431 +++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                      |  18 +
 drivers/scsi/ufs/ufshcd-pltfrm.c                   |   2 +-
 drivers/scsi/ufs/ufshcd.c                          |  29 +-
 drivers/scsi/ufs/ufshci-dwc.h                      |  42 ++
 drivers/scsi/ufs/ufshci.h                          |   1 +
 drivers/scsi/ufs/unipro.h                          |  39 ++
 14 files changed, 906 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt
 create mode 100644 drivers/scsi/ufs/ufs-dwc-pci.c
 create mode 100644 drivers/scsi/ufs/ufs-dwc.c
 create mode 100644 drivers/scsi/ufs/ufshcd-dwc.c
 create mode 100644 drivers/scsi/ufs/ufshcd-dwc.h
 create mode 100644 drivers/scsi/ufs/ufshci-dwc.h

-- 
1.8.1.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v8 1/3] fixed typo in ufshcd-pltfrm
  2016-02-15 15:25 [PATCH v8 0/3] add support for DWC UFS Controller Joao Pinto
@ 2016-02-15 15:25 ` Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 2/3] added UFS 2.0 capabilities Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 3/3] add support for DWC UFS Host Controller Joao Pinto
  2 siblings, 0 replies; 14+ messages in thread
From: Joao Pinto @ 2016-02-15 15:25 UTC (permalink / raw)
  To: vinholikatti, julian.calaby, akinobu.mita, hch, arnd, mark.rutland
  Cc: gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree, linux-kernel,
	linux-scsi, devicetree, Joao Pinto

Fixed typo in ufshcd-pltfrm.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v0->v8:
- Nothing changed (just to keep up with patch set version).

 drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index d2a7b12..0522891 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -353,6 +353,6 @@ EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
 
 MODULE_AUTHOR("Santosh Yaragnavi <santosh.sy@samsung.com>");
 MODULE_AUTHOR("Vinayak Holikatti <h.vinayak@samsung.com>");
-MODULE_DESCRIPTION("UFS host controller Pltform bus based glue driver");
+MODULE_DESCRIPTION("UFS host controller Platform bus based glue driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(UFSHCD_DRIVER_VERSION);
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 2/3] added UFS 2.0 capabilities
  2016-02-15 15:25 [PATCH v8 0/3] add support for DWC UFS Controller Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 1/3] fixed typo in ufshcd-pltfrm Joao Pinto
@ 2016-02-15 15:25 ` Joao Pinto
  2016-02-18 14:38   ` Rob Herring
  2016-02-15 15:25 ` [PATCH v8 3/3] add support for DWC UFS Host Controller Joao Pinto
  2 siblings, 1 reply; 14+ messages in thread
From: Joao Pinto @ 2016-02-15 15:25 UTC (permalink / raw)
  To: vinholikatti, julian.calaby, akinobu.mita, hch, arnd, mark.rutland
  Cc: gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree, linux-kernel,
	linux-scsi, devicetree, Joao Pinto

Adding UFS 2.0 support to the UFS core driver.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v7->v8:
- Added "jedec, ufs-2.0" to the ufschd-platform compatibility strings
Changes v0->v7:
- Nothing changed (just to keep up with patch set version).

 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  4 +--
 drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++++++---
 drivers/scsi/ufs/ufshci.h                          |  1 +
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index 03c0e98..8d9a9d2 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -4,8 +4,8 @@ UFSHC nodes are defined to describe on-chip UFS host controllers.
 Each UFS controller instance should have its own node.
 
 Required properties:
-- compatible		: must contain "jedec,ufs-1.1", may also list one or more
-					  of the following:
+- compatible		: must contain "jedec,ufs-1.1" or "jedec,ufs-2.0", may
+			  also list one or more of the following:
 					  "qcom,msm8994-ufshc"
 					  "qcom,msm8996-ufshc"
 					  "qcom,ufshc"
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 85cd256..2b5f2bf 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1223,6 +1223,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 			ret = -EINVAL;
 		}
 		break;
+	case UTP_CMD_TYPE_UFS_STORAGE:
 	case UTP_CMD_TYPE_DEV_MANAGE:
 		ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
 		if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
@@ -1287,6 +1288,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	struct ufshcd_lrb *lrbp;
 	struct ufs_hba *hba;
 	unsigned long flags;
+	u32 upiu_flags;
 	int tag;
 	int err = 0;
 
@@ -1343,10 +1345,23 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	lrbp->task_tag = tag;
 	lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
 	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
-	lrbp->command_type = UTP_CMD_TYPE_SCSI;
+
+	if (hba->ufs_version == UFSHCI_VERSION_20)
+		lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
+	else
+		lrbp->command_type = UTP_CMD_TYPE_SCSI;
 
 	/* form UPIU before issuing the command */
-	ufshcd_compose_upiu(hba, lrbp);
+	if (hba->ufs_version == UFSHCI_VERSION_20) {
+		if (likely(lrbp->cmd)) {
+			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+					lrbp->cmd->sc_data_direction);
+			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
+		} else
+			err = -EINVAL;
+	} else
+		ufshcd_compose_upiu(hba, lrbp);
+
 	err = ufshcd_map_sg(lrbp);
 	if (err) {
 		lrbp->cmd = NULL;
@@ -1371,7 +1386,12 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
 	lrbp->sense_buffer = NULL;
 	lrbp->task_tag = tag;
 	lrbp->lun = 0; /* device management cmd is not specific to any LUN */
-	lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
+
+	if (hba->ufs_version == UFSHCI_VERSION_20)
+		lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
+	else
+		lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
+
 	lrbp->intr_cmd = true; /* No interrupt aggregation */
 	hba->dev_cmd.type = cmd_type;
 
@@ -3187,7 +3207,8 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
 			/* Do not touch lrbp after scsi done */
 			cmd->scsi_done(cmd);
 			__ufshcd_release(hba);
-		} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE) {
+		} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
+			lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
 			if (hba->dev_cmd.complete)
 				complete(hba->dev_cmd.complete);
 		}
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 0ae0967..8dba0e7 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -273,6 +273,7 @@ enum {
 	UTP_CMD_TYPE_SCSI		= 0x0,
 	UTP_CMD_TYPE_UFS		= 0x1,
 	UTP_CMD_TYPE_DEV_MANAGE		= 0x2,
+	UTP_CMD_TYPE_UFS_STORAGE	= 0x11,
 };
 
 enum {
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v8 3/3] add support for DWC UFS Host Controller
  2016-02-15 15:25 [PATCH v8 0/3] add support for DWC UFS Controller Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 1/3] fixed typo in ufshcd-pltfrm Joao Pinto
  2016-02-15 15:25 ` [PATCH v8 2/3] added UFS 2.0 capabilities Joao Pinto
@ 2016-02-15 15:25 ` Joao Pinto
  2016-02-18 14:38   ` Rob Herring
  2016-02-19 15:03   ` [PATCH v9 " Arnd Bergmann
  2 siblings, 2 replies; 14+ messages in thread
From: Joao Pinto @ 2016-02-15 15:25 UTC (permalink / raw)
  To: vinholikatti, julian.calaby, akinobu.mita, hch, arnd, mark.rutland
  Cc: gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree, linux-kernel,
	linux-scsi, devicetree, Joao Pinto

This patch has the goal to add support for DesignWare UFS Controller
specific operations and to add specific platform and pci drivers.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v7->v8 (Akinobu Mita):
- DME sets were simplified for easier reading
- CLK DIV default values definitions names were changed to match the
 register's name
- New line added to dev_err and dev_info statements
Changes v6->v7 (Arnd Bergmann):
- Changed DT node name (to ufs only) and the memory address (to 0xd000000)
- Removed CONFIG_PM from the PCI glue driver (pm.h already does this)
- No other changes are necessary in ufshcd.c because of the link up notify
 function usage (it is simpler now)
- Removed the PHY mentioning since the Test Chip is not a real PHY for real
 world usage, since it is a test chip for prototyping with a very specific
 usage
- Added again the Test Chip 20-bit option
Changes v5->v6:
- Patch bad format fixed
Changes v4->v5 (Akinobu Mita):
- All functions used only locally in ufshcd-dwc are now declared as static
- ufshcd_dwc_configuration() was removed in ufshcd-dwc and a notify
 function (ufshcd_dwc_link_startup_notify) was created to deal with the
 DWC specific init routines
- 20-bit RMMI option was removed from Kconfig. Now if MPHY TC is selected
 and 40-bit is not then it assumes a 20-bit config
Changes v3->v4 (Arnd Bergmann and Mark Rutland):
- SCSI_UFS_DWC_HOOKS is now silent and selected by the SCSI_UFS_DWC_PLAT
 or SCSI_UFS_DWC_PCI
- Compatibility string has the ufs core version for info purposes since
 the driver is capable of getting the controller version from its 
 registers
- Created ufs-dwc-pci glue driver with specific DWC data
- MPHY configuration remains in the ufshcd-dwc since it is unipro
 attribute writting only not following the a linux phy framework logic
Changes v2->v3 (Julian Calaby):
- Implement a common DWC code to be used by the platform and pci glue
 drivers
- Synopsys ID & Class added to the existing pci driver and specific DWC
 was also added to the pci driver
Changes v1->v2 (Akinobu Mita):
- Implement a platform driver that uses the existing UFS core driver
- Add DWC specific code to the existing UFS core driver

 Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  51 +++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 172 +++++++++
 drivers/scsi/ufs/ufs-dwc.c                        |  96 +++++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 431 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 +++
 drivers/scsi/ufs/unipro.h                         |  39 ++
 10 files changed, 877 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt
 create mode 100644 drivers/scsi/ufs/ufs-dwc-pci.c
 create mode 100644 drivers/scsi/ufs/ufs-dwc.c
 create mode 100644 drivers/scsi/ufs/ufshcd-dwc.c
 create mode 100644 drivers/scsi/ufs/ufshcd-dwc.h
 create mode 100644 drivers/scsi/ufs/ufshci-dwc.h

diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
new file mode 100644
index 0000000..59e9822
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
@@ -0,0 +1,19 @@
+* Universal Flash Storage (UFS) DesignWare Host Controller
+
+DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
+Each UFS controller instance should have its own node.
+
+Required properties:
+- compatible        : compatible list must contain "snps,ufshcd-dwc" and should
+		      also contain the JEDEC version of the controller:
+			"jedec,ufs-1.1"
+			"jedec,ufs-2.0"
+- reg               : <registers mapping>
+- interrupts        : <interrupt mapping for UFS host controller IRQ>
+
+Example:
+	ufs@0xd0000000 {
+		compatible = "jedec,ufs-2.0", "snps,dwc-ufshcd";
+		reg = < 0xd0000000 0x10000 >;
+		interrupts = < 24 >;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index d2f94e2..3db3c4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11006,6 +11006,12 @@ S:	Supported
 F:	Documentation/scsi/ufs.txt
 F:	drivers/scsi/ufs/
 
+UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER DWC HOOKS
+M:	Joao Pinto <Joao.Pinto@synopsys.com>
+L:	linux-scsi@vger.kernel.org
+S:	Supported
+F:	drivers/scsi/ufs/*dwc*
+
 UNSORTED BLOCK IMAGES (UBI)
 M:	Artem Bityutskiy <dedekind1@gmail.com>
 M:	Richard Weinberger <richard@nod.at>
diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 5f45307..35e9ddf 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -83,3 +83,54 @@ config SCSI_UFS_QCOM
 
 	  Select this if you have UFS controller on QCOM chipset.
 	  If unsure, say N.
+
+config SCSI_UFS_DWC_HOOKS
+	bool
+
+config SCSI_UFS_DWC_PLAT
+	tristate "DesignWare UFS controller platform glue driver"
+	depends on SCSI_UFSHCD_PLATFORM
+	select SCSI_UFS_DWC_HOOKS
+	help
+	  This selects the DesignWare UFS host controller platform glue driver.
+
+	  Select this if you have a DesignWare UFS controller on Platform bus.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_PCI
+	tristate "DesignWare UFS controller pci glue driver"
+	depends on SCSI_UFSHCD_PCI
+	select SCSI_UFS_DWC_HOOKS
+	help
+	  This selects the DesignWare UFS host controller pci glue driver.
+
+	  Select this if you have a DesignWare UFS controller on pci bus.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_TC
+	bool "Support for the Synopsys Test Chip"
+	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
+	---help---
+	  Synopsys Test Chip is a Phy for prototyping purposes.
+	  This selects the support for the Synopsys Test Chip.
+
+	  Select this if you have a Synopsys Test Chip.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_20BIT_RMMI
+	bool "20-bit RMMI support"
+	depends on SCSI_UFS_DWC_TC
+	---help---
+	  This specifies that the Synopsys Test Chip supports 20-bit RMMI.
+
+	  Select this if you are using a 20-bit RMMI Synopsys Test Chip.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_40BIT_RMMI
+	bool "40-bit RMMI support"
+	depends on SCSI_UFS_DWC_TC
+	---help---
+	  Synopsys Test Chip is a Phy for prototyping purposes.
+
+	  Select this if you are using a 40-bit RMMI Synopsys Test Chip.
+	  If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 8303bcc..bab8c05 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -1,4 +1,7 @@
 # UFSHCD makefile
+obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
+obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
+obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
diff --git a/drivers/scsi/ufs/ufs-dwc-pci.c b/drivers/scsi/ufs/ufs-dwc-pci.c
new file mode 100644
index 0000000..845eb7e
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-dwc-pci.c
@@ -0,0 +1,172 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "ufshcd.h"
+#include "ufshcd-dwc.h"
+
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+
+/**
+ * ufs_dw_pci_suspend - suspend power management function
+ * @pdev: pointer to PCI device handle
+ * @state: power state
+ *
+ * Returns 0 if successful
+ * Returns non-zero otherwise
+ */
+static int ufs_dw_pci_suspend(struct device *dev)
+{
+	return ufshcd_system_suspend(dev_get_drvdata(dev));
+}
+
+/**
+ * ufs_dw_pci_resume - resume power management function
+ * @pdev: pointer to PCI device handle
+ *
+ * Returns 0 if successful
+ * Returns non-zero otherwise
+ */
+static int ufs_dw_pci_resume(struct device *dev)
+{
+	return ufshcd_system_resume(dev_get_drvdata(dev));
+}
+
+static int ufs_dw_pci_runtime_suspend(struct device *dev)
+{
+	return ufshcd_runtime_suspend(dev_get_drvdata(dev));
+}
+static int ufs_dw_pci_runtime_resume(struct device *dev)
+{
+	return ufshcd_runtime_resume(dev_get_drvdata(dev));
+}
+static int ufs_dw_pci_runtime_idle(struct device *dev)
+{
+	return ufshcd_runtime_idle(dev_get_drvdata(dev));
+}
+
+/**
+ * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
+ */
+static struct ufs_hba_variant_ops ufs_dwc_pci_hba_vops = {
+	.name                   = "ufshcd-dwc-pci",
+	.link_startup_notify	= ufshcd_dwc_link_startup_notify,
+};
+
+/**
+ * ufs_dw_pci_shutdown - main function to put the controller in reset state
+ * @pdev: pointer to PCI device handle
+ */
+static void ufs_dw_pci_shutdown(struct pci_dev *pdev)
+{
+	ufshcd_shutdown((struct ufs_hba *)pci_get_drvdata(pdev));
+}
+
+/**
+ * ufs_dw_pci_remove - de-allocate PCI/SCSI host and host memory space
+ *		data structure memory
+ * @pdev - pointer to PCI handle
+ */
+static void ufs_dw_pci_remove(struct pci_dev *pdev)
+{
+	struct ufs_hba *hba = pci_get_drvdata(pdev);
+
+	pm_runtime_forbid(&pdev->dev);
+	pm_runtime_get_noresume(&pdev->dev);
+	ufshcd_remove(hba);
+}
+
+/**
+ * ufs_dw_pci_probe - probe routine of the driver
+ * @pdev: pointer to PCI device handle
+ * @id: PCI device id
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int
+ufs_dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct ufs_hba *hba;
+	void __iomem *mmio_base;
+	int err;
+
+	err = pcim_enable_device(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "pcim_enable_device failed\n");
+		return err;
+	}
+
+	pci_set_master(pdev);
+
+	err = pcim_iomap_regions(pdev, 1 << 0, UFSHCD);
+	if (err < 0) {
+		dev_err(&pdev->dev, "request and iomap failed\n");
+		return err;
+	}
+
+	mmio_base = pcim_iomap_table(pdev)[0];
+
+	err = ufshcd_alloc_host(&pdev->dev, &hba);
+	if (err) {
+		dev_err(&pdev->dev, "Allocation failed\n");
+		return err;
+	}
+
+	INIT_LIST_HEAD(&hba->clk_list_head);
+
+	hba->vops = &ufs_dwc_pci_hba_vops;
+
+	err = ufshcd_init(hba, mmio_base, pdev->irq);
+	if (err) {
+		dev_err(&pdev->dev, "Initialization failed\n");
+		return err;
+	}
+
+	pci_set_drvdata(pdev, hba);
+	pm_runtime_put_noidle(&pdev->dev);
+	pm_runtime_allow(&pdev->dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops ufs_dw_pci_pm_ops = {
+	.suspend	= ufs_dw_pci_suspend,
+	.resume		= ufs_dw_pci_resume,
+	.runtime_suspend = ufs_dw_pci_runtime_suspend,
+	.runtime_resume  = ufs_dw_pci_runtime_resume,
+	.runtime_idle    = ufs_dw_pci_runtime_idle,
+};
+
+static const struct pci_device_id ufs_dw_pci_tbl[] = {
+	{ PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ }	/* terminate list */
+};
+
+MODULE_DEVICE_TABLE(pci, ufs_dw_pci_tbl);
+
+static struct pci_driver ufs_dw_pci_driver = {
+	.name = UFSHCD,
+	.id_table = ufs_dw_pci_tbl,
+	.probe = ufs_dw_pci_probe,
+	.remove = ufs_dw_pci_remove,
+	.shutdown = ufs_dw_pci_shutdown,
+	.driver = {
+		.pm = &ufs_dw_pci_pm_ops
+	},
+};
+
+module_pci_driver(ufs_dw_pci_driver);
+
+MODULE_AUTHOR("Joao Pinto <Joao.Pinto@synopsys.com>");
+MODULE_DESCRIPTION("DesignWare UFS host controller PCI glue driver");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/scsi/ufs/ufs-dwc.c b/drivers/scsi/ufs/ufs-dwc.c
new file mode 100644
index 0000000..cf4ee2b
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-dwc.c
@@ -0,0 +1,96 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+
+#include "ufshcd-pltfrm.h"
+#include "ufshcd-dwc.h"
+
+/**
+ * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
+ *
+ */
+static struct ufs_hba_variant_ops ufs_dwc_hba_vops = {
+	.name                   = "ufshcd-dwc",
+	.link_startup_notify	= ufshcd_dwc_link_startup_notify,
+};
+
+/**
+ * ufs_dwc_probe()
+ * @pdev: pointer to platform device structure
+ *
+ */
+static int ufs_dwc_probe(struct platform_device *pdev)
+{
+	int err;
+	struct device *dev = &pdev->dev;
+
+	/* Perform generic probe */
+	err = ufshcd_pltfrm_init(pdev, &ufs_dwc_hba_vops);
+	if (err)
+		dev_err(dev, "ufshcd_pltfrm_init() failed %d\n", err);
+
+	return err;
+}
+
+/**
+ * ufs_dwc_remove()
+ * @pdev: pointer to platform device structure
+ *
+ */
+static int ufs_dwc_remove(struct platform_device *pdev)
+{
+	struct ufs_hba *hba =  platform_get_drvdata(pdev);
+
+	pm_runtime_get_sync(&(pdev)->dev);
+	ufshcd_remove(hba);
+
+	return 0;
+}
+
+static const struct of_device_id ufs_dwc_match[] = {
+	{
+		.compatible = "snps,ufshcd-dwc"
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ufs_dwc_match);
+
+static const struct dev_pm_ops ufs_dwc_pm_ops = {
+	.suspend	= ufshcd_pltfrm_suspend,
+	.resume		= ufshcd_pltfrm_resume,
+	.runtime_suspend = ufshcd_pltfrm_runtime_suspend,
+	.runtime_resume  = ufshcd_pltfrm_runtime_resume,
+	.runtime_idle    = ufshcd_pltfrm_runtime_idle,
+};
+
+static struct platform_driver ufs_dwc_driver = {
+	.probe		= ufs_dwc_probe,
+	.remove		= ufs_dwc_remove,
+	.shutdown = ufshcd_pltfrm_shutdown,
+	.driver		= {
+		.name	= "ufshcd-dwc",
+		.pm	= &ufs_dwc_pm_ops,
+		.of_match_table	= of_match_ptr(ufs_dwc_match),
+	},
+};
+
+module_platform_driver(ufs_dwc_driver);
+
+MODULE_ALIAS("platform:ufshcd-dwc");
+MODULE_DESCRIPTION("DesignWare UFS Host platform glue driver");
+MODULE_AUTHOR("Joao Pinto <Joao.Pinto@synopsys.com>");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/scsi/ufs/ufshcd-dwc.c b/drivers/scsi/ufs/ufshcd-dwc.c
new file mode 100644
index 0000000..46a8300
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-dwc.c
@@ -0,0 +1,431 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "ufshcd.h"
+#include "unipro.h"
+
+#include "ufshcd-dwc.h"
+#include "ufshci-dwc.h"
+
+struct ufshcd_dme_attr_val {
+	u32 attr_sel;
+	u32 mib_val;
+	u8 peer;
+};
+
+int ufshcd_dme_set_attrs(struct ufs_hba *hba,
+				const struct ufshcd_dme_attr_val *v, int n)
+{
+	int ret = 0;
+	int attr_node = 0;
+
+	for (attr_node = 0; attr_node < n; attr_node++) {
+		ret = ufshcd_dme_set_attr(hba, v[attr_node].attr_sel,
+			ATTR_SET_NOR, v[attr_node].mib_val, v[attr_node].peer);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * ufshcd_dwc_program_clk_div()
+ * This function programs the clk divider value. This value is needed to
+ * provide 1 microsecond tick to unipro layer.
+ * @hba: Private Structure pointer
+ * @divider_val: clock divider value to be programmed
+ *
+ */
+static void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
+{
+	ufshcd_writel(hba, divider_val, DWC_UFS_REG_HCLKDIV);
+}
+
+/**
+ * ufshcd_dwc_link_is_up()
+ * Check if link is up
+ * @hba: private structure poitner
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
+{
+	int dme_result = 0;
+
+	ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
+
+	if (dme_result == UFSHCD_LINK_IS_UP) {
+		ufshcd_set_link_active(hba);
+		return 0;
+	}
+
+	return 1;
+}
+
+/**
+ * ufshcd_dwc_connection_setup()
+ * This function configures both the local side (host) and the peer side
+ * (device) unipro attributes to establish the connection to application/
+ * cport.
+ * This function is not required if the hardware is properly configured to
+ * have this connection setup on reset. But invoking this function does no
+ * harm and should be fine even working with any ufs device.
+ *
+ * @hba: pointer to drivers private data
+ *
+ * Returns 0 on success non-zero value on failure
+ */
+static int ufshcd_dwc_connection_setup(struct ufs_hba *hba)
+{
+	const struct ufshcd_dme_attr_val setup_attrs[] = {
+		{ UIC_ARG_MIB(T_CONNECTIONSTATE), 0, DME_LOCAL },
+		{ UIC_ARG_MIB(N_DEVICEID), 0, DME_LOCAL },
+		{ UIC_ARG_MIB(N_DEVICEID_VALID), 0, DME_LOCAL },
+		{ UIC_ARG_MIB(T_PEERDEVICEID), 1, DME_LOCAL },
+		{ UIC_ARG_MIB(T_PEERCPORTID), 0, DME_LOCAL },
+		{ UIC_ARG_MIB(T_TRAFFICCLASS), 0, DME_LOCAL },
+		{ UIC_ARG_MIB(T_CPORTFLAGS), 0x6, DME_LOCAL },
+		{ UIC_ARG_MIB(T_CPORTMODE), 1, DME_LOCAL },
+		{ UIC_ARG_MIB(T_CONNECTIONSTATE), 1, DME_LOCAL },
+		{ UIC_ARG_MIB(T_CONNECTIONSTATE), 0, DME_PEER },
+		{ UIC_ARG_MIB(N_DEVICEID), 1, DME_PEER },
+		{ UIC_ARG_MIB(N_DEVICEID_VALID), 1, DME_PEER },
+		{ UIC_ARG_MIB(T_PEERDEVICEID), 1, DME_PEER },
+		{ UIC_ARG_MIB(T_PEERCPORTID), 0, DME_PEER },
+		{ UIC_ARG_MIB(T_TRAFFICCLASS), 0, DME_PEER },
+		{ UIC_ARG_MIB(T_CPORTFLAGS), 0x6, DME_PEER },
+		{ UIC_ARG_MIB(T_CPORTMODE), 1, DME_PEER },
+		{ UIC_ARG_MIB(T_CONNECTIONSTATE), 1, DME_PEER }
+	};
+
+	return ufshcd_dme_set_attrs(hba, setup_attrs, ARRAY_SIZE(setup_attrs));
+}
+
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+/**
+ * ufshcd_dwc_setup_40bit_rmmi()
+ * This function configures Synopsys TC specific atributes (40-bit RMMI)
+ * @hba: Pointer to drivers structure
+ *
+ * Returns 0 on success or non-zero value on failure
+ */
+static int ufshcd_dwc_setup_40bit_rmmi(struct ufs_hba *hba)
+{
+	const struct ufshcd_dme_attr_val setup_attrs[] = {
+		{ UIC_ARG_MIB(TX_GLOBALHIBERNATE), 0x00, DME_LOCAL },
+		{ UIC_ARG_MIB(REFCLKMODE), 0x01, DME_LOCAL },
+		{ UIC_ARG_MIB(CDIRECTCTRL6), 0x80, DME_LOCAL },
+		{ UIC_ARG_MIB(CBDIVFACTOR), 0x08, DME_LOCAL },
+		{ UIC_ARG_MIB(CBDCOCTRL5), 0x64, DME_LOCAL },
+		{ UIC_ARG_MIB(CBPRGTUNING), 0x09, DME_LOCAL },
+		{ UIC_ARG_MIB(RTOBSERVESELECT), 0x00, DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(TX_REFCLKFREQ, SELIND_LN0_TX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(TX_CFGCLKFREQVAL, SELIND_LN0_TX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGEXTRATTR, SELIND_LN0_TX), 0x14,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(DITHERCTRL2, SELIND_LN0_TX), 0xd6,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RX_REFCLKFREQ, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RX_CFGCLKFREQVAL, SELIND_LN0_RX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGWIDEINLN, SELIND_LN0_RX), 4,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXCDR8, SELIND_LN0_RX), 0x80,
+								DME_LOCAL },
+		{ UIC_ARG_MIB(DIRECTCTRL10), 0x04, DME_LOCAL },
+		{ UIC_ARG_MIB(DIRECTCTRL19), 0x02, DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXCDR8, SELIND_LN0_RX), 0x80,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG4, SELIND_LN0_RX), 0x03,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR8, SELIND_LN0_RX), 0x16,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXDIRECTCTRL2, SELIND_LN0_RX), 0x42,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG3, SELIND_LN0_RX), 0xa4,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXCALCTRL, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG2, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR4, SELIND_LN0_RX), 0x28,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXSQCTRL, SELIND_LN0_RX), 0x1E,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR6, SELIND_LN0_RX), 0x2f,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR6, SELIND_LN0_RX), 0x2f,
+								DME_LOCAL },
+		{ UIC_ARG_MIB(CBPRGPLL2), 0x00, DME_LOCAL },
+	};
+
+	return ufshcd_dme_set_attrs(hba, setup_attrs, ARRAY_SIZE(setup_attrs));
+}
+
+#endif
+
+#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI
+/**
+ * ufshcd_dwc_setup_20bit_rmmi_lane0()
+ * This function configures Synopsys TC 20-bit RMMI Lane 0
+ * @hba: Pointer to drivers structure
+ *
+ * Returns 0 on success or non-zero value on failure
+ */
+static int ufshcd_dwc_setup_20bit_rmmi_lane0(struct ufs_hba *hba)
+{
+	const struct ufshcd_dme_attr_val setup_attrs[] = {
+		{ UIC_ARG_MIB_SEL(TX_REFCLKFREQ, SELIND_LN0_TX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(TX_CFGCLKFREQVAL, SELIND_LN0_TX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RX_CFGCLKFREQVAL, SELIND_LN0_RX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGEXTRATTR, SELIND_LN0_TX), 0x12,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(DITHERCTRL2, SELIND_LN0_TX), 0xd6,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RX_REFCLKFREQ, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGWIDEINLN, SELIND_LN0_RX), 2,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXCDR8, SELIND_LN0_RX), 0x80,
+								DME_LOCAL },
+		{ UIC_ARG_MIB(DIRECTCTRL10), 0x04, DME_LOCAL },
+		{ UIC_ARG_MIB(DIRECTCTRL19), 0x02, DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG4, SELIND_LN0_RX), 0x03,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR8, SELIND_LN0_RX), 0x16,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXDIRECTCTRL2, SELIND_LN0_RX), 0x42,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG3, SELIND_LN0_RX), 0xa4,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXCALCTRL, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG2, SELIND_LN0_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR4, SELIND_LN0_RX), 0x28,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXSQCTRL, SELIND_LN0_RX), 0x1E,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR6, SELIND_LN0_RX), 0x2f,
+								DME_LOCAL },
+		{ UIC_ARG_MIB(CBPRGPLL2), 0x00, DME_LOCAL },
+	};
+
+	return ufshcd_dme_set_attrs(hba, setup_attrs, ARRAY_SIZE(setup_attrs));
+}
+
+/**
+ * ufshcd_dwc_setup_20bit_rmmi_lane1()
+ * This function configures Synopsys TC 20-bit RMMI Lane 1
+ * @hba: Pointer to drivers structure
+ *
+ * Returns 0 on success or non-zero value on failure
+ */
+static int ufshcd_dwc_setup_20bit_rmmi_lane1(struct ufs_hba *hba)
+{
+	int connected_rx_lanes = 0;
+	int connected_tx_lanes = 0;
+	int ret = 0;
+
+	const struct ufshcd_dme_attr_val setup_tx_attrs[] = {
+		{ UIC_ARG_MIB_SEL(TX_REFCLKFREQ, SELIND_LN1_TX), 0x0d,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(TX_CFGCLKFREQVAL, SELIND_LN1_TX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGEXTRATTR, SELIND_LN1_TX), 0x12,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(DITHERCTRL2, SELIND_LN0_TX), 0xd6,
+								DME_LOCAL },
+	};
+
+	const struct ufshcd_dme_attr_val setup_rx_attrs[] = {
+		{ UIC_ARG_MIB_SEL(RX_REFCLKFREQ, SELIND_LN1_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RX_CFGCLKFREQVAL, SELIND_LN1_RX), 0x19,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGWIDEINLN, SELIND_LN1_RX), 2,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXCDR8, SELIND_LN1_RX), 0x80,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG4, SELIND_LN1_RX), 0x03,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR8, SELIND_LN1_RX), 0x16,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXDIRECTCTRL2, SELIND_LN1_RX), 0x42,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG3, SELIND_LN1_RX), 0xa4,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXCALCTRL, SELIND_LN1_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(ENARXDIRECTCFG2, SELIND_LN1_RX), 0x01,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR4, SELIND_LN1_RX), 0x28,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(RXSQCTRL, SELIND_LN1_RX), 0x1E,
+								DME_LOCAL },
+		{ UIC_ARG_MIB_SEL(CFGRXOVR6, SELIND_LN1_RX), 0x2f,
+								DME_LOCAL },
+	};
+
+	/* Get the available lane count */
+	ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES),
+			&connected_rx_lanes);
+	ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
+			&connected_tx_lanes);
+
+	if (connected_tx_lanes == 2) {
+
+		ret = ufshcd_dme_set_attrs(hba, setup_tx_attrs,
+						ARRAY_SIZE(setup_tx_attrs));
+
+		if (ret)
+			goto out;
+	}
+
+	if (connected_rx_lanes == 2) {
+		ret = ufshcd_dme_set_attrs(hba, setup_rx_attrs,
+						ARRAY_SIZE(setup_rx_attrs));
+	}
+
+out:
+	return ret;
+}
+
+/**
+ * ufshcd_dwc_setup_20bit_rmmi()
+ * This function configures Synopsys TC specific atributes (20-bit RMMI)
+ * @hba: Pointer to drivers structure
+ *
+ * Returns 0 on success or non-zero value on failure
+ */
+static int ufshcd_dwc_setup_20bit_rmmi(struct ufs_hba *hba)
+{
+	int ret = 0;
+
+	const struct ufshcd_dme_attr_val setup_attrs[] = {
+		{ UIC_ARG_MIB(TX_GLOBALHIBERNATE), 0x00, DME_LOCAL },
+		{ UIC_ARG_MIB(REFCLKMODE), 0x01, DME_LOCAL },
+		{ UIC_ARG_MIB(CDIRECTCTRL6), 0xc0, DME_LOCAL },
+		{ UIC_ARG_MIB(CBDIVFACTOR), 0x44, DME_LOCAL },
+		{ UIC_ARG_MIB(CBDCOCTRL5), 0x64, DME_LOCAL },
+		{ UIC_ARG_MIB(CBPRGTUNING), 0x09, DME_LOCAL },
+		{ UIC_ARG_MIB(RTOBSERVESELECT), 0x00, DME_LOCAL },
+	};
+
+	ret = ufshcd_dme_set_attrs(hba, setup_attrs, ARRAY_SIZE(setup_attrs));
+	if (ret)
+		goto out;
+
+	/* Lane 0 configuration*/
+	ret = ufshcd_dwc_setup_20bit_rmmi_lane0(hba);
+	if (ret)
+		goto out;
+
+	/* Lane 1 configuration*/
+	ret = ufshcd_dwc_setup_20bit_rmmi_lane1(hba);
+	if (ret)
+		goto out;
+
+out:
+	return ret;
+}
+#endif
+
+/**
+ * ufshcd_dwc_setup_tc()
+ * This function configures Local (host) Synopsys TC specific attributes
+ *
+ * @hba: Pointer to drivers structure
+ *
+ * Returns 0 on success non-zero value on failure
+ */
+static int ufshcd_dwc_setup_tc(struct ufs_hba *hba)
+{
+	int ret = 0;
+
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+	dev_info(hba->dev, "Configuring Test Chip 40-bit RMMI\n");
+	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "Configuration failed\n");
+		goto out;
+	}
+#endif
+
+#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI
+	dev_info(hba->dev, "Configuring Test Chip 20-bit RMMI\n");
+	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "Configuration failed\n");
+		goto out;
+	}
+#endif
+	/* To write Shadow register bank to effective configuration block */
+	ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_MPHYCFGUPDT), 0x01);
+	if (ret)
+		goto out;
+
+	/* To configure Debug OMC */
+	ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_DEBUGOMC), 0x01);
+
+out:
+	return ret;
+}
+
+/**
+ * ufshcd_dwc_link_startup_notify()
+ * UFS Host DWC specific link startup sequence
+ * @hba: private structure poitner
+ * @status: Callback notify status
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
+					enum ufs_notify_change_status status)
+{
+	int err = 0;
+
+	if (status == PRE_CHANGE) {
+		ufshcd_dwc_program_clk_div(hba, DWC_UFS_REG_HCLKDIV_DIV_125);
+#ifdef CONFIG_SCSI_UFS_DWC_TC
+		err = ufshcd_dwc_setup_tc(hba);
+		if (err) {
+			dev_err(hba->dev, "Configuration failed (%d)\n",
+									err);
+			goto out;
+		}
+#endif
+	} else { /* POST_CHANGE */
+		err = ufshcd_dwc_link_is_up(hba);
+		if (err) {
+			dev_err(hba->dev, "Link is not up\n");
+			goto out;
+		}
+
+		err = ufshcd_dwc_connection_setup(hba);
+		if (err)
+			dev_err(hba->dev, "Connection setup failed (%d)\n",
+									err);
+	}
+
+out:
+	return err;
+}
+EXPORT_SYMBOL(ufshcd_dwc_link_startup_notify);
diff --git a/drivers/scsi/ufs/ufshcd-dwc.h b/drivers/scsi/ufs/ufshcd-dwc.h
new file mode 100644
index 0000000..ca65ee1
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-dwc.h
@@ -0,0 +1,18 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UFSHCD_DWC_H
+#define _UFSHCD_DWC_H
+
+int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
+					enum ufs_notify_change_status status);
+#endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshci-dwc.h b/drivers/scsi/ufs/ufshci-dwc.h
new file mode 100644
index 0000000..947a75c
--- /dev/null
+++ b/drivers/scsi/ufs/ufshci-dwc.h
@@ -0,0 +1,42 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UFSHCI_DWC_H
+#define _UFSHCI_DWC_H
+
+/* DWC HC UFSHCI specific Registers */
+enum dwc_specific_registers {
+	DWC_UFS_REG_HCLKDIV	= 0xFC,
+};
+
+/* Link Status*/
+enum link_status {
+	UFSHCD_LINK_IS_DOWN	= 1,
+	UFSHCD_LINK_IS_UP	= 2,
+};
+
+/* Clock Divider Values: Hex equivalent of frequency in MHz */
+enum clk_div_values {
+	DWC_UFS_REG_HCLKDIV_DIV_62_5	= 0x3e,
+	DWC_UFS_REG_HCLKDIV_DIV_125	= 0x7d,
+	DWC_UFS_REG_HCLKDIV_DIV_200	= 0xc8,
+};
+
+/* Selector Index */
+enum selector_index {
+	SELIND_LN0_TX		= 0x00,
+	SELIND_LN1_TX		= 0x01,
+	SELIND_LN0_RX		= 0x04,
+	SELIND_LN1_RX		= 0x05,
+};
+
+#endif /* End of Header */
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index 816a8a4..da16254 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -35,6 +35,10 @@
 #define TX_LCC_SEQUENCER			0x0032
 #define TX_MIN_ACTIVATETIME			0x0033
 #define TX_PWM_G6_G7_SYNC_LENGTH		0x0034
+#define TX_REFCLKFREQ				0x00EB
+#define TX_CFGCLKFREQVAL			0x00EC
+#define	CFGEXTRATTR				0x00F0
+#define DITHERCTRL2				0x00F1
 
 /*
  * M-RX Configuration Attributes
@@ -48,8 +52,38 @@
 #define RX_ENTER_HIBERN8			0x00A7
 #define RX_BYPASS_8B10B_ENABLE			0x00A8
 #define RX_TERMINATION_FORCE_ENABLE		0x0089
+#define RX_REFCLKFREQ				0x00EB
+#define	RX_CFGCLKFREQVAL			0x00EC
+#define CFGWIDEINLN				0x00F0
+#define CFGRXCDR8				0x00BA
+#define ENARXDIRECTCFG4				0x00F2
+#define CFGRXOVR8				0x00BD
+#define RXDIRECTCTRL2				0x00C7
+#define ENARXDIRECTCFG3				0x00F3
+#define RXCALCTRL				0x00B4
+#define ENARXDIRECTCFG2				0x00F4
+#define CFGRXOVR4				0x00E9
+#define RXSQCTRL				0x00B5
+#define CFGRXOVR6				0x00BF
 
 #define is_mphy_tx_attr(attr)			(attr < RX_MODE)
+
+/*
+ * Common Block Attributes
+ */
+#define TX_GLOBALHIBERNATE			UNIPRO_CB_OFFSET(0x002B)
+#define REFCLKMODE				UNIPRO_CB_OFFSET(0x00BF)
+#define DIRECTCTRL19				UNIPRO_CB_OFFSET(0x00CD)
+#define DIRECTCTRL10				UNIPRO_CB_OFFSET(0x00E6)
+#define CDIRECTCTRL6				UNIPRO_CB_OFFSET(0x00EA)
+#define RTOBSERVESELECT				UNIPRO_CB_OFFSET(0x00F0)
+#define CBDIVFACTOR				UNIPRO_CB_OFFSET(0x00F1)
+#define CBDCOCTRL5				UNIPRO_CB_OFFSET(0x00F3)
+#define CBPRGPLL2				UNIPRO_CB_OFFSET(0x00F8)
+#define CBPRGTUNING				UNIPRO_CB_OFFSET(0x00FB)
+
+#define UNIPRO_CB_OFFSET(x)			(0x8000 | x)
+
 /*
  * PHY Adpater attributes
  */
@@ -110,6 +144,11 @@
 #define PA_STALLNOCONFIGTIME	0x15A3
 #define PA_SAVECONFIGTIME	0x15A4
 
+/*Other attributes*/
+#define VS_MPHYCFGUPDT		0xD085
+#define VS_DEBUGOMC		0xD09E
+#define VS_POWERSTATE		0xD083
+
 /* PA power modes */
 enum {
 	FAST_MODE	= 1,
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v8 2/3] added UFS 2.0 capabilities
  2016-02-15 15:25 ` [PATCH v8 2/3] added UFS 2.0 capabilities Joao Pinto
@ 2016-02-18 14:38   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2016-02-18 14:38 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, arnd,
	mark.rutland, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Mon, Feb 15, 2016 at 03:25:12PM +0000, Joao Pinto wrote:
> Adding UFS 2.0 support to the UFS core driver.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Changes v7->v8:
> - Added "jedec, ufs-2.0" to the ufschd-platform compatibility strings
> Changes v0->v7:
> - Nothing changed (just to keep up with patch set version).
> 
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  4 +--

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++++++---
>  drivers/scsi/ufs/ufshci.h                          |  1 +
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v8 3/3] add support for DWC UFS Host Controller
  2016-02-15 15:25 ` [PATCH v8 3/3] add support for DWC UFS Host Controller Joao Pinto
@ 2016-02-18 14:38   ` Rob Herring
  2016-02-19 15:03   ` [PATCH v9 " Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring @ 2016-02-18 14:38 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, arnd,
	mark.rutland, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Mon, Feb 15, 2016 at 03:25:13PM +0000, Joao Pinto wrote:
> This patch has the goal to add support for DesignWare UFS Controller
> specific operations and to add specific platform and pci drivers.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Changes v7->v8 (Akinobu Mita):
> - DME sets were simplified for easier reading
> - CLK DIV default values definitions names were changed to match the
>  register's name
> - New line added to dev_err and dev_info statements
> Changes v6->v7 (Arnd Bergmann):
> - Changed DT node name (to ufs only) and the memory address (to 0xd000000)
> - Removed CONFIG_PM from the PCI glue driver (pm.h already does this)
> - No other changes are necessary in ufshcd.c because of the link up notify
>  function usage (it is simpler now)
> - Removed the PHY mentioning since the Test Chip is not a real PHY for real
>  world usage, since it is a test chip for prototyping with a very specific
>  usage
> - Added again the Test Chip 20-bit option
> Changes v5->v6:
> - Patch bad format fixed
> Changes v4->v5 (Akinobu Mita):
> - All functions used only locally in ufshcd-dwc are now declared as static
> - ufshcd_dwc_configuration() was removed in ufshcd-dwc and a notify
>  function (ufshcd_dwc_link_startup_notify) was created to deal with the
>  DWC specific init routines
> - 20-bit RMMI option was removed from Kconfig. Now if MPHY TC is selected
>  and 40-bit is not then it assumes a 20-bit config
> Changes v3->v4 (Arnd Bergmann and Mark Rutland):
> - SCSI_UFS_DWC_HOOKS is now silent and selected by the SCSI_UFS_DWC_PLAT
>  or SCSI_UFS_DWC_PCI
> - Compatibility string has the ufs core version for info purposes since
>  the driver is capable of getting the controller version from its 
>  registers
> - Created ufs-dwc-pci glue driver with specific DWC data
> - MPHY configuration remains in the ufshcd-dwc since it is unipro
>  attribute writting only not following the a linux phy framework logic
> Changes v2->v3 (Julian Calaby):
> - Implement a common DWC code to be used by the platform and pci glue
>  drivers
> - Synopsys ID & Class added to the existing pci driver and specific DWC
>  was also added to the pci driver
> Changes v1->v2 (Akinobu Mita):
> - Implement a platform driver that uses the existing UFS core driver
> - Add DWC specific code to the existing UFS core driver
> 
>  Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
>  MAINTAINERS                                       |   6 +
>  drivers/scsi/ufs/Kconfig                          |  51 +++
>  drivers/scsi/ufs/Makefile                         |   3 +
>  drivers/scsi/ufs/ufs-dwc-pci.c                    | 172 +++++++++
>  drivers/scsi/ufs/ufs-dwc.c                        |  96 +++++
>  drivers/scsi/ufs/ufshcd-dwc.c                     | 431 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
>  drivers/scsi/ufs/ufshci-dwc.h                     |  42 +++
>  drivers/scsi/ufs/unipro.h                         |  39 ++
>  10 files changed, 877 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>  create mode 100644 drivers/scsi/ufs/ufs-dwc-pci.c
>  create mode 100644 drivers/scsi/ufs/ufs-dwc.c
>  create mode 100644 drivers/scsi/ufs/ufshcd-dwc.c
>  create mode 100644 drivers/scsi/ufs/ufshcd-dwc.h
>  create mode 100644 drivers/scsi/ufs/ufshci-dwc.h
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> new file mode 100644
> index 0000000..59e9822
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> @@ -0,0 +1,19 @@
> +* Universal Flash Storage (UFS) DesignWare Host Controller
> +
> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
> +Each UFS controller instance should have its own node.
> +
> +Required properties:
> +- compatible        : compatible list must contain "snps,ufshcd-dwc" and should
> +		      also contain the JEDEC version of the controller:
> +			"jedec,ufs-1.1"
> +			"jedec,ufs-2.0"

As I said before, this needs to mention also requiring an SoC specific 
compatible.

> +- reg               : <registers mapping>
> +- interrupts        : <interrupt mapping for UFS host controller IRQ>
> +
> +Example:
> +	ufs@0xd0000000 {

Drop '0x'

> +		compatible = "jedec,ufs-2.0", "snps,dwc-ufshcd";

This should be reversed. Most specific first.

> +		reg = < 0xd0000000 0x10000 >;
> +		interrupts = < 24 >;
> +	};

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-02-15 15:25 ` [PATCH v8 3/3] add support for DWC UFS Host Controller Joao Pinto
  2016-02-18 14:38   ` Rob Herring
@ 2016-02-19 15:03   ` Arnd Bergmann
  2016-03-02 16:46     ` Joao Pinto
  1 sibling, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2016-02-19 15:03 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
> 
>  Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
>  MAINTAINERS                                       |   6 +
>  drivers/scsi/ufs/Kconfig                          |  51 +++
>  drivers/scsi/ufs/Makefile                         |   3 +
>  drivers/scsi/ufs/ufs-dwc-pci.c                    | 172 +++++++++
>  drivers/scsi/ufs/ufs-dwc.c                        |  96 +++++
>  drivers/scsi/ufs/ufshcd-dwc.c                     | 431 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
>  drivers/scsi/ufs/ufshci-dwc.h                     |  42 +++
>  drivers/scsi/ufs/unipro.h                         |  39 ++

I still think this can be split up further. From your previous
explanation, I understand that there is a specific test chip
that uses the "snps,ufshcd-dwc" implementation along with some
other glue logic.

Please split this out so you have anything related to the test
chips separate from the patch that implements core logic.
 
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> new file mode 100644
> index 0000000..59e9822
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> @@ -0,0 +1,19 @@
> +* Universal Flash Storage (UFS) DesignWare Host Controller
> +
> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
> +Each UFS controller instance should have its own node.
> +
> +Required properties:
> +- compatible        : compatible list must contain "snps,ufshcd-dwc" and should
> +		      also contain the JEDEC version of the controller:
> +			"jedec,ufs-1.1"
> +			"jedec,ufs-2.0"
> +- reg               : <registers mapping>
> +- interrupts        : <interrupt mapping for UFS host controller IRQ>


Based on your last reply to me (sorry for coming back to this so
late), I think having just "snps,ufshcd-dwc" as the compatible
string is not appropriate: This assumes that all "snps,ufshcd-dwc"
instances are 100% compatible, but your code makes it very clear
that this is not the case.

Please for the last time (!) add a proper version number of the
specific IP block to the compatible strings so you can identify
what hardware you are talking to.

You earlier confused the version number of the UFS standard with
the version number of the controller, and that has been clarified
now, but you really still need to use a version for the hardware
as well.

> +config SCSI_UFS_DWC_TC
> +	bool "Support for the Synopsys Test Chip"
> +	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
> +	---help---
> +	  Synopsys Test Chip is a Phy for prototyping purposes.
> +	  This selects the support for the Synopsys Test Chip.
> +
> +	  Select this if you have a Synopsys Test Chip.
> +	  If unsure, say N.
> +
> +config SCSI_UFS_DWC_20BIT_RMMI
> +	bool "20-bit RMMI support"
> +	depends on SCSI_UFS_DWC_TC
> +	---help---
> +	  This specifies that the Synopsys Test Chip supports 20-bit RMMI.
> +
> +	  Select this if you are using a 20-bit RMMI Synopsys Test Chip.
> +	  If unsure, say N.
> +
> +config SCSI_UFS_DWC_40BIT_RMMI
> +	bool "40-bit RMMI support"
> +	depends on SCSI_UFS_DWC_TC
> +	---help---
> +	  Synopsys Test Chip is a Phy for prototyping purposes.
> +
> +	  Select this if you are using a 40-bit RMMI Synopsys Test Chip.
> +	  If unsure, say N.

I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
support both. There is not really much need for the options
as this is just a test chip, and nobody is going to care much
about saving a few bytes of object code.

> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 8303bcc..bab8c05 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -1,4 +1,7 @@
>  # UFSHCD makefile
> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o

However, please split out the SCSI_UFS_DWC_TC specific bits into
a separate file, and put the module_platform_driver() bits into
that file, to get the right abstraction where the most specific
driver calls into the next driver, like

dwc-test-chip -> dwc-platform -> dwc -> ufs

It's possible you can leave out the dwc-platform level here, as there
are no other users for now, we can add others later as needed.

> +/**
> + * ufshcd_dwc_setup_tc()
> + * This function configures Local (host) Synopsys TC specific attributes
> + *
> + * @hba: Pointer to drivers structure
> + *
> + * Returns 0 on success non-zero value on failure
> + */
> +static int ufshcd_dwc_setup_tc(struct ufs_hba *hba)
> +{
> +	int ret = 0;
> +
> +#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
> +	dev_info(hba->dev, "Configuring Test Chip 40-bit RMMI\n");
> +	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
> +	if (ret) {
> +		dev_err(hba->dev, "Configuration failed\n");
> +		goto out;
> +	}
> +#endif
> +
> +#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI
> +	dev_info(hba->dev, "Configuring Test Chip 20-bit RMMI\n");
> +	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
> +	if (ret) {
> +		dev_err(hba->dev, "Configuration failed\n");
> +		goto out;
> +	}
> +#endif

You have changed the #if/#else into two #if sections, but this
still seems broken when both are enabled, it just cannot work
unless you have some runtime detection in place.

Depending on whether this is actually the same hardware with
different settings, or different variants of the test chip,
please use either a boolean DT property to determine which
one you need, or use a separate "compatible" string in DT
for each version of the test chip.

> +/**
> + * ufshcd_dwc_link_startup_notify()
> + * UFS Host DWC specific link startup sequence
> + * @hba: private structure poitner
> + * @status: Callback notify status
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
> +					enum ufs_notify_change_status status)
> +{
> +	int err = 0;
> +
> +	if (status == PRE_CHANGE) {
> +		ufshcd_dwc_program_clk_div(hba, DWC_UFS_REG_HCLKDIV_DIV_125);
> +#ifdef CONFIG_SCSI_UFS_DWC_TC
> +		err = ufshcd_dwc_setup_tc(hba);
> +		if (err) {
> +			dev_err(hba->dev, "Configuration failed (%d)\n",
> +									err);
> +			goto out;
> +		}
> +#endif
> +	} else { /* POST_CHANGE */

Similar, this #ifdef should almost certainly be rewritten so that
only a DT that identifies as the test chip will call that.

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-02-19 15:03   ` [PATCH v9 " Arnd Bergmann
@ 2016-03-02 16:46     ` Joao Pinto
  2016-03-02 19:55       ` Arnd Bergmann
  0 siblings, 1 reply; 14+ messages in thread
From: Joao Pinto @ 2016-03-02 16:46 UTC (permalink / raw)
  To: Arnd Bergmann, Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree


Hi Arnd,

On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
>>
>>  Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
>>  MAINTAINERS                                       |   6 +
>>  drivers/scsi/ufs/Kconfig                          |  51 +++
>>  drivers/scsi/ufs/Makefile                         |   3 +
>>  drivers/scsi/ufs/ufs-dwc-pci.c                    | 172 +++++++++
>>  drivers/scsi/ufs/ufs-dwc.c                        |  96 +++++
>>  drivers/scsi/ufs/ufshcd-dwc.c                     | 431 ++++++++++++++++++++++
>>  drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
>>  drivers/scsi/ufs/ufshci-dwc.h                     |  42 +++
>>  drivers/scsi/ufs/unipro.h                         |  39 ++
> 
> I still think this can be split up further. From your previous
> explanation, I understand that there is a specific test chip
> that uses the "snps,ufshcd-dwc" implementation along with some
> other glue logic.
> 
> Please split this out so you have anything related to the test
> chips separate from the patch that implements core logic.

Ok, I will split more the patches.

>  
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> new file mode 100644
>> index 0000000..59e9822
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> @@ -0,0 +1,19 @@
>> +* Universal Flash Storage (UFS) DesignWare Host Controller
>> +
>> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
>> +Each UFS controller instance should have its own node.
>> +
>> +Required properties:
>> +- compatible        : compatible list must contain "snps,ufshcd-dwc" and should
>> +		      also contain the JEDEC version of the controller:
>> +			"jedec,ufs-1.1"
>> +			"jedec,ufs-2.0"
>> +- reg               : <registers mapping>
>> +- interrupts        : <interrupt mapping for UFS host controller IRQ>
> 
> 
> Based on your last reply to me (sorry for coming back to this so
> late), I think having just "snps,ufshcd-dwc" as the compatible
> string is not appropriate: This assumes that all "snps,ufshcd-dwc"
> instances are 100% compatible, but your code makes it very clear
> that this is not the case.
> 
> Please for the last time (!) add a proper version number of the
> specific IP block to the compatible strings so you can identify
> what hardware you are talking to.
> 
> You earlier confused the version number of the UFS standard with
> the version number of the controller, and that has been clarified
> now, but you really still need to use a version for the hardware
> as well.

Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree?

> 
>> +config SCSI_UFS_DWC_TC
>> +	bool "Support for the Synopsys Test Chip"
>> +	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
>> +	---help---
>> +	  Synopsys Test Chip is a Phy for prototyping purposes.
>> +	  This selects the support for the Synopsys Test Chip.
>> +
>> +	  Select this if you have a Synopsys Test Chip.
>> +	  If unsure, say N.
>> +
>> +config SCSI_UFS_DWC_20BIT_RMMI
>> +	bool "20-bit RMMI support"
>> +	depends on SCSI_UFS_DWC_TC
>> +	---help---
>> +	  This specifies that the Synopsys Test Chip supports 20-bit RMMI.
>> +
>> +	  Select this if you are using a 20-bit RMMI Synopsys Test Chip.
>> +	  If unsure, say N.
>> +
>> +config SCSI_UFS_DWC_40BIT_RMMI
>> +	bool "40-bit RMMI support"
>> +	depends on SCSI_UFS_DWC_TC
>> +	---help---
>> +	  Synopsys Test Chip is a Phy for prototyping purposes.
>> +
>> +	  Select this if you are using a 40-bit RMMI Synopsys Test Chip.
>> +	  If unsure, say N.
> 
> I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
> and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
> support both. There is not really much need for the options
> as this is just a test chip, and nobody is going to care much
> about saving a few bytes of object code.

We need to know if we are dealing with a 20-bit test chip or a 40-bit test chip
because the initialization is different. That can be made in the device tree as
you say bellow, but we can also use a setup in which the host is a PC (so no
device tree) connected by pci bus to an fpga containing the UFS controller.
Having this, I think that the only way is to choose the 20/40bit stuff in the
menuconfig.

> 
>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
>> index 8303bcc..bab8c05 100644
>> --- a/drivers/scsi/ufs/Makefile
>> +++ b/drivers/scsi/ufs/Makefile
>> @@ -1,4 +1,7 @@
>>  # UFSHCD makefile
>> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
>> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
>> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
>>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
>>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> 
> However, please split out the SCSI_UFS_DWC_TC specific bits into
> a separate file, and put the module_platform_driver() bits into
> that file, to get the right abstraction where the most specific
> driver calls into the next driver, like
> 
> dwc-test-chip -> dwc-platform -> dwc -> ufs

I think that it is a good idea and we isolate the test chip logic. If in the
future anyone uses DWC core with a real PHY then they can have a phy driver
calling dwc-platform. Agree?

> 
> It's possible you can leave out the dwc-platform level here, as there
> are no other users for now, we can add others later as needed.
> 
>> +/**
>> + * ufshcd_dwc_setup_tc()
>> + * This function configures Local (host) Synopsys TC specific attributes
>> + *
>> + * @hba: Pointer to drivers structure
>> + *
>> + * Returns 0 on success non-zero value on failure
>> + */
>> +static int ufshcd_dwc_setup_tc(struct ufs_hba *hba)
>> +{
>> +	int ret = 0;
>> +
>> +#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
>> +	dev_info(hba->dev, "Configuring Test Chip 40-bit RMMI\n");
>> +	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Configuration failed\n");
>> +		goto out;
>> +	}
>> +#endif
>> +
>> +#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI
>> +	dev_info(hba->dev, "Configuring Test Chip 20-bit RMMI\n");
>> +	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Configuration failed\n");
>> +		goto out;
>> +	}
>> +#endif
> 
> You have changed the #if/#else into two #if sections, but this
> still seems broken when both are enabled, it just cannot work
> unless you have some runtime detection in place.
> 

Please check my comments upstream.

> Depending on whether this is actually the same hardware with
> different settings, or different variants of the test chip,
> please use either a boolean DT property to determine which
> one you need, or use a separate "compatible" string in DT
> for each version of the test chip.

As I say upstream, we can have a setup with the kernel running in a PC connected
to the UFS controller by PCI, so the DT scenario is no good.

> 
>> +/**
>> + * ufshcd_dwc_link_startup_notify()
>> + * UFS Host DWC specific link startup sequence
>> + * @hba: private structure poitner
>> + * @status: Callback notify status
>> + *
>> + * Returns 0 on success, non-zero value on failure
>> + */
>> +int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
>> +					enum ufs_notify_change_status status)
>> +{
>> +	int err = 0;
>> +
>> +	if (status == PRE_CHANGE) {
>> +		ufshcd_dwc_program_clk_div(hba, DWC_UFS_REG_HCLKDIV_DIV_125);
>> +#ifdef CONFIG_SCSI_UFS_DWC_TC
>> +		err = ufshcd_dwc_setup_tc(hba);
>> +		if (err) {
>> +			dev_err(hba->dev, "Configuration failed (%d)\n",
>> +									err);
>> +			goto out;
>> +		}
>> +#endif
>> +	} else { /* POST_CHANGE */
> 
> Similar, this #ifdef should almost certainly be rewritten so that
> only a DT that identifies as the test chip will call that.

The same.

> 
> 	Arnd
> 

Joao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-02 16:46     ` Joao Pinto
@ 2016-03-02 19:55       ` Arnd Bergmann
  2016-03-03 11:39         ` Joao Pinto
  0 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2016-03-02 19:55 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:
> On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
> > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
> >> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt

> > Please for the last time (!) add a proper version number of the
> > specific IP block to the compatible strings so you can identify
> > what hardware you are talking to.
> > 
> > You earlier confused the version number of the UFS standard with
> > the version number of the controller, and that has been clarified
> > now, but you really still need to use a version for the hardware
> > as well.
> 
> Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree?

I have no idea what versions of the dwc hardware block exist, but
I find it highly suspicious that the numbers happen to be the
same as the UFS protocol numbers, so I'd say that is probably
not the version of the IP block.

There are a few things you could try to find out the actual
version:

* If you are able to contact the team that worked on the test chip,
  please ask them what hardware revision you have.

* if you have some form of documentation of the hardware, look
  on the first few pages of the manual that describes the registers
  to see if the document has a revision history.

* If you have access to the hardware design files, look at the
  file names.

On https://www.synopsys.com/dw/ipdir.php?ds=ufs, I see a
version "1.30a" listed, which follows the typical numbering
scheme that your employer uses, a single digit followed by
a dot and a two-digit number and then a letter.

There is also a test chip with version 1.10a listed on the
same page, and that follows the same numbering system.

See if you can find a version number that fits into that scheme
in the documents you have available.

> >> +config SCSI_UFS_DWC_TC
> >> +	bool "Support for the Synopsys Test Chip"
> >> +	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
> >> +	---help---
> >> +	  Synopsys Test Chip is a Phy for prototyping purposes.
> >> +	  This selects the support for the Synopsys Test Chip.
> >> +
> >> +	  Select this if you have a Synopsys Test Chip.
> >> +	  If unsure, say N.
> >> +
> >> +config SCSI_UFS_DWC_20BIT_RMMI
> >> +	bool "20-bit RMMI support"
> >> +	depends on SCSI_UFS_DWC_TC
> >> +	---help---
> >> +	  This specifies that the Synopsys Test Chip supports 20-bit RMMI.
> >> +
> >> +	  Select this if you are using a 20-bit RMMI Synopsys Test Chip.
> >> +	  If unsure, say N.
> >> +
> >> +config SCSI_UFS_DWC_40BIT_RMMI
> >> +	bool "40-bit RMMI support"
> >> +	depends on SCSI_UFS_DWC_TC
> >> +	---help---
> >> +	  Synopsys Test Chip is a Phy for prototyping purposes.
> >> +
> >> +	  Select this if you are using a 40-bit RMMI Synopsys Test Chip.
> >> +	  If unsure, say N.
> > 
> > I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
> > and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
> > support both. There is not really much need for the options
> > as this is just a test chip, and nobody is going to care much
> > about saving a few bytes of object code.
> 
> We need to know if we are dealing with a 20-bit test chip or a 40-bit test chip
> because the initialization is different. That can be made in the device tree as
> you say bellow, but we can also use a setup in which the host is a PC (so no
> device tree) connected by pci bus to an fpga containing the UFS controller.
> Having this, I think that the only way is to choose the 20/40bit stuff in the
> menuconfig.

NAK.

Mutually exclusive compile-time configuration options are always wrong.

If the PCI hosts both have the same PCI device ID and there is no other
identification register that lets you find out which one you have,
maybe you can use a module parameter that defaults to invalid and that
has to be set explicitly when loading the PCI driver?

Are both test chips the same way? I see that the driver supports two
distinct PCI device IDs, so please check of they both come in variations
for the two PHYs, or if at least one of them always uses the same PHY
so you don't need the module parameter for that.

> >> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> >> index 8303bcc..bab8c05 100644
> >> --- a/drivers/scsi/ufs/Makefile
> >> +++ b/drivers/scsi/ufs/Makefile
> >> @@ -1,4 +1,7 @@
> >>  # UFSHCD makefile
> >> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
> >> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
> >> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
> >>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> >>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> >>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> > 
> > However, please split out the SCSI_UFS_DWC_TC specific bits into
> > a separate file, and put the module_platform_driver() bits into
> > that file, to get the right abstraction where the most specific
> > driver calls into the next driver, like
> > 
> > dwc-test-chip -> dwc-platform -> dwc -> ufs
> 
> I think that it is a good idea and we isolate the test chip logic. If in the
> future anyone uses DWC core with a real PHY then they can have a phy driver
> calling dwc-platform. Agree?

Yes, that would be good. It's likely that such a system would use
some licensed IP block for the PHY, which can then have a separate
PHY driver.

> > It's possible you can leave out the dwc-platform level here, as there
> > are no other users for now, we can add others later as needed.
> > 
> >> +/**
> >> + * ufshcd_dwc_setup_tc()
> >> + * This function configures Local (host) Synopsys TC specific attributes
> >> + *
> >> + * @hba: Pointer to drivers structure
> >> + *
> >> + * Returns 0 on success non-zero value on failure
> >> + */
> >> +static int ufshcd_dwc_setup_tc(struct ufs_hba *hba)
> >> +{
> >> +	int ret = 0;
> >> +
> >> +#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
> >> +	dev_info(hba->dev, "Configuring Test Chip 40-bit RMMI\n");
> >> +	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
> >> +	if (ret) {
> >> +		dev_err(hba->dev, "Configuration failed\n");
> >> +		goto out;
> >> +	}
> >> +#endif
> >> +
> >> +#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI
> >> +	dev_info(hba->dev, "Configuring Test Chip 20-bit RMMI\n");
> >> +	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
> >> +	if (ret) {
> >> +		dev_err(hba->dev, "Configuration failed\n");
> >> +		goto out;
> >> +	}
> >> +#endif
> > 
> > You have changed the #if/#else into two #if sections, but this
> > still seems broken when both are enabled, it just cannot work
> > unless you have some runtime detection in place.
> > 
> 
> Please check my comments upstream.
> 
> > Depending on whether this is actually the same hardware with
> > different settings, or different variants of the test chip,
> > please use either a boolean DT property to determine which
> > one you need, or use a separate "compatible" string in DT
> > for each version of the test chip.
> 
> As I say upstream, we can have a setup with the kernel running in a PC connected
> to the UFS controller by PCI, so the DT scenario is no good.

The DT scenario can work out of the box, that is fine, you just
need a hack in the PCI driver to work around the lack of identification
there.

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-02 19:55       ` Arnd Bergmann
@ 2016-03-03 11:39         ` Joao Pinto
  2016-03-03 12:04           ` Arnd Bergmann
  0 siblings, 1 reply; 14+ messages in thread
From: Joao Pinto @ 2016-03-03 11:39 UTC (permalink / raw)
  To: Arnd Bergmann, Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

Hi Arnd,

On 3/2/2016 7:55 PM, Arnd Bergmann wrote:
> On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:
>> On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
>>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:

Facts:

- Test Chip type are currently not detectable in runtime through the controller
- In the future the Test Chip version will be available in the controller
- Test Chip initialization is different for each type
- The IP Core version is 1.40a
- Test Chip version is 6.00
- Teh UFS version is 2.0

Suggested driver architecture:

Platform setup:
 tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> ufs

The test chip platform driver could be called through 2 compatibility strings.
indicating the chip's version and bit type:
 "snps, g210-tc-6.00-20bit"
 "snps, g210-tc-6.00-40bit"

The device tree node would have additional info compatibility strings as the DWC
IP core version and UFS version:
 "snps, dwc-ufshcd-1.40a"
 "jedec, ufs-2.0"

PCI based setup:
 tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs

The test chip type would be configured by a parameter to be passed in the kernel
boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit)

Having this in mind the KConfig would be:

"config SCSI_UFS_DWC_HOOKS
	bool

config SCSI_UFS_DWC_PLAT
	tristate "DesignWare UFS controller platform glue driver"
	depends on SCSI_UFSHCD_PLATFORM
	select SCSI_UFS_DWC_HOOKS
	help
	  This selects the DesignWare UFS host controller platform glue driver.

	  Select this if you have a DesignWare UFS controller on Platform bus.
	  If unsure, say N.

config SCSI_UFS_DWC_PCI
	tristate "DesignWare UFS controller pci glue driver"
	depends on SCSI_UFSHCD_PCI
	select SCSI_UFS_DWC_HOOKS
	help
	  This selects the DesignWare UFS host controller pci glue driver.

	  Select this if you have a DesignWare UFS controller on pci bus.
	  If unsure, say N.

config SCSI_UFS_DWC_TC
	bool "Support for the Synopsys Test Chip"
	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
	---help---
	  Synopsys Test Chip is a Phy for prototyping purposes.
	  This selects the support for the Synopsys Test Chip.

	  Select this if you have a Synopsys Test Chip.
	  If unsure, say N."

Agree with the approach?

Thanks for the help.

Joao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-03 11:39         ` Joao Pinto
@ 2016-03-03 12:04           ` Arnd Bergmann
  2016-03-03 13:52             ` Joao Pinto
  0 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2016-03-03 12:04 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Thursday 03 March 2016 11:39:05 Joao Pinto wrote:
> Hi Arnd,
> 
> On 3/2/2016 7:55 PM, Arnd Bergmann wrote:
> > On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:
> >> On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
> >>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
> 
> Facts:
> 
> - Test Chip type are currently not detectable in runtime through the controller
> - In the future the Test Chip version will be available in the controller
> - Test Chip initialization is different for each type
> - The IP Core version is 1.40a
> - Test Chip version is 6.00
> - Teh UFS version is 2.0

Ok.

> Suggested driver architecture:
> 
> Platform setup:
>  tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> ufs
> 
> The test chip platform driver could be called through 2 compatibility strings.
> indicating the chip's version and bit type:
>  "snps, g210-tc-6.00-20bit"
>  "snps, g210-tc-6.00-40bit"

Yes, this sounds good. We can probably skip one of the middle layers,
but basically that is what I was looking for.

> The device tree node would have additional info compatibility strings as the DWC
> IP core version and UFS version:
>  "snps, dwc-ufshcd-1.40a"
>  "jedec, ufs-2.0"
> 
> PCI based setup:
>  tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs

The tc-dwc-g210 portion probably shouldn't depend on both
ufshcd-dwc-pltfrm and ufshcd-dwc-pci here, so how about leaving
those two out?


Then it becomes

   tc-dwc-g210-pci ---> tc-dwc-g210 --> ufshcd-dwc --> ufs
tc-dwc-g210-pltfrm --/

> The test chip type would be configured by a parameter to be passed in the kernel
> boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit)

Right. With module_param() helper, this will be either a boot command
line option, or a module load option, depending on whether the driver
is built-on or not.

modprobe tc-dwc-g210-pci tc_type=20

command line: tc-dwc-g210-pci.tc_type=20
 
> Having this in mind the KConfig would be:
> 
> "config SCSI_UFS_DWC_HOOKS
> 	bool

I think we can now remove the config option for the hooks as well.

> config SCSI_UFS_DWC_PLAT
> 	tristate "DesignWare UFS controller platform glue driver"
> 	depends on SCSI_UFSHCD_PLATFORM
> 	select SCSI_UFS_DWC_HOOKS
> 	help
> 	  This selects the DesignWare UFS host controller platform glue driver.
> 
> 	  Select this if you have a DesignWare UFS controller on Platform bus.
> 	  If unsure, say N.
> 
> config SCSI_UFS_DWC_PCI
> 	tristate "DesignWare UFS controller pci glue driver"
> 	depends on SCSI_UFSHCD_PCI
> 	select SCSI_UFS_DWC_HOOKS
> 	help
> 	  This selects the DesignWare UFS host controller pci glue driver.
> 
> 	  Select this if you have a DesignWare UFS controller on pci bus.
> 	  If unsure, say N.
> 
> config SCSI_UFS_DWC_TC
> 	bool "Support for the Synopsys Test Chip"
> 	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
> 	---help---
> 	  Synopsys Test Chip is a Phy for prototyping purposes.
> 	  This selects the support for the Synopsys Test Chip.
> 
> 	  Select this if you have a Synopsys Test Chip.
> 	  If unsure, say N."
> 
> Agree with the approach?

This would work, but I think it's better to define the options in terms
of the top-level drivers, i.e. SCSI_UFS_DWC_TC_PCI and SCSI_UFS_DWC_TC_PLATFORM,
and then make the other options hidden and implicitly turned out by them.

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-03 12:04           ` Arnd Bergmann
@ 2016-03-03 13:52             ` Joao Pinto
  2016-03-03 14:12               ` Arnd Bergmann
  0 siblings, 1 reply; 14+ messages in thread
From: Joao Pinto @ 2016-03-03 13:52 UTC (permalink / raw)
  To: Arnd Bergmann, Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

Hi,

On 3/3/2016 12:04 PM, Arnd Bergmann wrote:
> On Thursday 03 March 2016 11:39:05 Joao Pinto wrote:
>> Hi Arnd,
>>
>> On 3/2/2016 7:55 PM, Arnd Bergmann wrote:
>>> On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:
>>>> On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
>>>>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
>>
>> Facts:
>>
>> - Test Chip type are currently not detectable in runtime through the controller
>> - In the future the Test Chip version will be available in the controller
>> - Test Chip initialization is different for each type
>> - The IP Core version is 1.40a
>> - Test Chip version is 6.00
>> - Teh UFS version is 2.0
> 
> Ok.
> 
>> Suggested driver architecture:
>>
>> Platform setup:
>>  tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> ufs
>>
>> The test chip platform driver could be called through 2 compatibility strings.
>> indicating the chip's version and bit type:
>>  "snps, g210-tc-6.00-20bit"
>>  "snps, g210-tc-6.00-40bit"
> 
> Yes, this sounds good. We can probably skip one of the middle layers,
> but basically that is what I was looking for.
> 
>> The device tree node would have additional info compatibility strings as the DWC
>> IP core version and UFS version:
>>  "snps, dwc-ufshcd-1.40a"
>>  "jedec, ufs-2.0"
>>
>> PCI based setup:
>>  tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs
> 
> The tc-dwc-g210 portion probably shouldn't depend on both
> ufshcd-dwc-pltfrm and ufshcd-dwc-pci here, so how about leaving
> those two out?
> 
> 
> Then it becomes
> 
>    tc-dwc-g210-pci ---> tc-dwc-g210 --> ufshcd-dwc --> ufs
> tc-dwc-g210-pltfrm --/

Ok, understood. It becomes simpler without the pltfm and pci "middle layer".

> 
>> The test chip type would be configured by a parameter to be passed in the kernel
>> boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit)
> 
> Right. With module_param() helper, this will be either a boot command
> line option, or a module load option, depending on whether the driver
> is built-on or not.
> 
> modprobe tc-dwc-g210-pci tc_type=20
> 
> command line: tc-dwc-g210-pci.tc_type=20
>  

Right, that was the idea.

>> Having this in mind the KConfig would be:
>>
>> "config SCSI_UFS_DWC_HOOKS
>> 	bool
> 
> I think we can now remove the config option for the hooks as well.
> 
>> config SCSI_UFS_DWC_PLAT
>> 	tristate "DesignWare UFS controller platform glue driver"
>> 	depends on SCSI_UFSHCD_PLATFORM
>> 	select SCSI_UFS_DWC_HOOKS
>> 	help
>> 	  This selects the DesignWare UFS host controller platform glue driver.
>>
>> 	  Select this if you have a DesignWare UFS controller on Platform bus.
>> 	  If unsure, say N.
>>
>> config SCSI_UFS_DWC_PCI
>> 	tristate "DesignWare UFS controller pci glue driver"
>> 	depends on SCSI_UFSHCD_PCI
>> 	select SCSI_UFS_DWC_HOOKS
>> 	help
>> 	  This selects the DesignWare UFS host controller pci glue driver.
>>
>> 	  Select this if you have a DesignWare UFS controller on pci bus.
>> 	  If unsure, say N.
>>
>> config SCSI_UFS_DWC_TC
>> 	bool "Support for the Synopsys Test Chip"
>> 	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
>> 	---help---
>> 	  Synopsys Test Chip is a Phy for prototyping purposes.
>> 	  This selects the support for the Synopsys Test Chip.
>>
>> 	  Select this if you have a Synopsys Test Chip.
>> 	  If unsure, say N."
>>
>> Agree with the approach?
> 
> This would work, but I think it's better to define the options in terms
> of the top-level drivers, i.e. SCSI_UFS_DWC_TC_PCI and SCSI_UFS_DWC_TC_PLATFORM,
> and then make the other options hidden and implicitly turned out by them.
> 

config SCSI_UFS_DWC
	bool

config SCSI_UFS_DWC_TC_PLATFORM
 	tristate "DesignWare platform support using a G210 Test Chip"
 	depends on SCSI_UFSHCD_PLATFORM
	select SCSI_UFS_DWC
 	---help---
 	  Synopsys Test Chip is a PHY for prototyping purposes.

 	  If unsure, say N."

config SCSI_UFS_DWC_TC_PCI
	tristate "DesignWare pci support using a G210 Test Chip"
	depends on SCSI_UFSHCD_PCI
	select SCSI_UFS_DWC
	---help---
	  Synopsys Test Chip is a PHY for prototyping purposes.

	  If unsure, say N."

I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime.

Agree?

> 	Arnd
> 

Joao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-03 13:52             ` Joao Pinto
@ 2016-03-03 14:12               ` Arnd Bergmann
  2016-03-03 14:17                 ` Joao Pinto
  0 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2016-03-03 14:12 UTC (permalink / raw)
  To: Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree

On Thursday 03 March 2016 13:52:39 Joao Pinto wrote:
> 
> config SCSI_UFS_DWC
>         bool
> 
> config SCSI_UFS_DWC_TC_PLATFORM
>         tristate "DesignWare platform support using a G210 Test Chip"
>         depends on SCSI_UFSHCD_PLATFORM
>         select SCSI_UFS_DWC
>         ---help---
>           Synopsys Test Chip is a PHY for prototyping purposes.
> 
>           If unsure, say N."
> 
> config SCSI_UFS_DWC_TC_PCI
>         tristate "DesignWare pci support using a G210 Test Chip"
>         depends on SCSI_UFSHCD_PCI
>         select SCSI_UFS_DWC
>         ---help---
>           Synopsys Test Chip is a PHY for prototyping purposes.
> 
>           If unsure, say N."
> 
> I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime.
> 
> Agree?
> 

Yes, looks good to me.

	Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
  2016-03-03 14:12               ` Arnd Bergmann
@ 2016-03-03 14:17                 ` Joao Pinto
  0 siblings, 0 replies; 14+ messages in thread
From: Joao Pinto @ 2016-03-03 14:17 UTC (permalink / raw)
  To: Arnd Bergmann, Joao Pinto
  Cc: vinholikatti, julian.calaby, akinobu.mita, hch, mark.rutland,
	robh, gbroner, subhashj, CARLOS.PALMINHA, ijc+devicetree,
	linux-kernel, linux-scsi, devicetree


On 3/3/2016 2:12 PM, Arnd Bergmann wrote:
> On Thursday 03 March 2016 13:52:39 Joao Pinto wrote:
>>
>> config SCSI_UFS_DWC
>>         bool
>>
>> config SCSI_UFS_DWC_TC_PLATFORM
>>         tristate "DesignWare platform support using a G210 Test Chip"
>>         depends on SCSI_UFSHCD_PLATFORM
>>         select SCSI_UFS_DWC
>>         ---help---
>>           Synopsys Test Chip is a PHY for prototyping purposes.
>>
>>           If unsure, say N."
>>
>> config SCSI_UFS_DWC_TC_PCI
>>         tristate "DesignWare pci support using a G210 Test Chip"
>>         depends on SCSI_UFSHCD_PCI
>>         select SCSI_UFS_DWC
>>         ---help---
>>           Synopsys Test Chip is a PHY for prototyping purposes.
>>
>>           If unsure, say N."
>>
>> I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime.
>>
>> Agree?
>>
> 
> Yes, looks good to me.
> 
> 	Arnd
> 

Nice! Thanks for the inputs. I'm going to implement the changes and send a v10 soon.

Joao

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-03-03 14:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-15 15:25 [PATCH v8 0/3] add support for DWC UFS Controller Joao Pinto
2016-02-15 15:25 ` [PATCH v8 1/3] fixed typo in ufshcd-pltfrm Joao Pinto
2016-02-15 15:25 ` [PATCH v8 2/3] added UFS 2.0 capabilities Joao Pinto
2016-02-18 14:38   ` Rob Herring
2016-02-15 15:25 ` [PATCH v8 3/3] add support for DWC UFS Host Controller Joao Pinto
2016-02-18 14:38   ` Rob Herring
2016-02-19 15:03   ` [PATCH v9 " Arnd Bergmann
2016-03-02 16:46     ` Joao Pinto
2016-03-02 19:55       ` Arnd Bergmann
2016-03-03 11:39         ` Joao Pinto
2016-03-03 12:04           ` Arnd Bergmann
2016-03-03 13:52             ` Joao Pinto
2016-03-03 14:12               ` Arnd Bergmann
2016-03-03 14:17                 ` Joao Pinto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).