add support for DWC UFS Controller

Adding UFS 2.0 support to the UFS core driver.

Signed-off-by Joao Pinto <[email protected]>

---
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(-)

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 <[email protected]>

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

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +0 -0

new file mode 100644
index 0000000..59e9822

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +3 -0

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

 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 ++

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +2 -0

new file mode 100644
index 0000000..59e9822
@@ -0,0 +1,19 @@
+* Universal Flash Storage (UFS) DesignWare Host Controller
+

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +17 -0

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

drivers/scsi/ufs/Makefile +19 -0

+/**
+ * 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;
+	}

drivers/scsi/ufs/Makefile +14 -0

+/**
+ * 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) {

Quoted 11 lines · 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.

Quoted 9 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +2 -0

new file mode 100644
index 0000000..59e9822
@@ -0,0 +1,19 @@
+* Universal Flash Storage (UFS) DesignWare Host Controller
+

Documentation/devicetree/bindings/ufs/ufs-dwc.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0

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

Quoted 26 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +25 -0

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

Documentation/devicetree/bindings/ufs/ufs-dwc.txt

Quoted 2 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +2 -0

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

drivers/scsi/ufs/Makefile line 7

Quoted 13 lines

drivers/scsi/ufs/Makefile +3 -0

index 8303bcc..bab8c05 100644
@@ -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.

Quoted 25 lines

drivers/scsi/ufs/Makefile +25 -0

+/**
+ * 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) {

drivers/scsi/ufs/Makefile

Quoted 4 lines

drivers/scsi/ufs/Makefile +4 -0

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

Quoted 20 lines

drivers/scsi/ufs/Makefile +19 -0

+/**
+ * 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",

drivers/scsi/ufs/Makefile

Quoted 5 lines

drivers/scsi/ufs/Makefile +5 -0

+									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

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/ufs/ufs-dwc.txt

Quoted 1 line · On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:

Documentation/devicetree/bindings/ufs/ufs-dwc.txt +0 -0

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.

Quoted 4 lines

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?

Quoted 25 lines

Quoted 25 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt +25 -0

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

Quoted 8 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt

Quoted 2 lines

Documentation/devicetree/bindings/ufs/ufs-dwc.txt +2 -0

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

Quoted 18 lines

drivers/scsi/ufs/Makefile line 7

Quoted 12 lines

drivers/scsi/ufs/Makefile +3 -0

index 8303bcc..bab8c05 100644
@@ -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

Quoted 1 line

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?

Quoted 28 lines

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.

Quoted 25 lines

drivers/scsi/ufs/Makefile +25 -0

+/**
+ * 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) {

Quoted 9 lines

drivers/scsi/ufs/Makefile

Quoted 4 lines

drivers/scsi/ufs/Makefile +4 -0

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

Quoted 5 lines

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.

Quoted 2 lines · On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:

Quoted 1 line · Arnd Bergmann

On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:

Hi Arnd,

Quoted 3 lines · Arnd Bergmann

Quoted 2 lines · On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:

Quoted 1 line · Arnd Bergmann

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?

Quoted 7 lines · On Thursday 03 March 2016 11:39:05 Joao Pinto wrote:

Hi Arnd,

Quoted 3 lines · Arnd Bergmann

Quoted 2 lines · On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:

Quoted 1 line · Arnd Bergmann

On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 7 lines

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

Quoted 3 lines

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

Quoted 4 lines

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.

Quoted 27 lines

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.

Quoted 4 lines

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

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?

Quoted 22 lines · 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."

Quoted 4 lines

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

Agree?

Yes, looks good to me.

Arnd