Skip to contentSkip to conversation

add support for DWC UFS Controller

PATCH Superseded Joao Pinto 2 participants 6 messages Feb 10, 2016 – Feb 11, 2016

Subsystems (3)

v6 of 16 Superseded Latest is v16 →

lore
Share & tools
lore

Joao Pinto

author
#0 lore raw

v6 cover add support for DWC UFS Controller

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

Joao Pinto (2):
fixed typo in ufshcd-pltfrm
add support for DWC UFS Host Controller

 Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  17 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  41 ++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 180 ++++++
 drivers/scsi/ufs/ufs-dwc.c                        | 102 +++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 736 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshcd-pltfrm.c                  |   2 +-
 drivers/scsi/ufs/ufshcd.c                         |  50 +-
 drivers/scsi/ufs/ufshcd.h                         |  13 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 ++
 drivers/scsi/ufs/ufshci.h                         |   1 +
 drivers/scsi/ufs/unipro.h                         |  39 ++
 14 files changed, 1232 insertions(+), 18 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
Signature · 6 lines
1.8.1.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#1 lore raw

PATCH v6 1/2 fixed typo in ufshcd-pltfrm

Fixed typo in ufshcd-pltfrm.

---
Changes v0->v6:
- Nothing changed (just to keep up with patch set version).

 drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/ufs/ufshcd-pltfrm.c +1 -1
index d2a7b12..0522891 100644
@@ -353,6 +353,6 @@ EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
 
 MODULE_AUTHOR("Santosh Yaragnavi <[email protected]>");
 MODULE_AUTHOR("Vinayak Holikatti <[email protected]>");
-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);
Signature · 1 line
1.8.1.5
author
#2 lore raw

PATCH v6 2/2 add support for DWC UFS Host Controller

In reply to Joao Pinto

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

---
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 |  17 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  41 ++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 180 ++++++
 drivers/scsi/ufs/ufs-dwc.c                        | 102 +++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 736 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshcd.c                         |  50 +-
 drivers/scsi/ufs/ufshcd.h                         |  13 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 ++
 drivers/scsi/ufs/ufshci.h                         |   1 +
 drivers/scsi/ufs/unipro.h                         |  39 ++
 13 files changed, 1231 insertions(+), 17 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
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +17 -0
new file mode 100644
index 0000000..f38a3f5
@@ -0,0 +1,17 @@
+* 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 string ("snps,ufshcd-1.0", "snps,ufshcd-1.1"
+  or "snps,ufshcd-2.0")
+- reg               : <registers mapping>
+- interrupts        : <interrupt mapping for UFS host controller IRQ>
+
+Example:
+	dwc_ufshcd@0xD0000000 {
+		compatible = "snps,ufshcd-2.0";
+		reg = < 0xD0000000 0x10000 >;
+		interrupts = < 24 >;
+	};
MAINTAINERS +6 -0
index d2f94e2..3db3c4c 100644
@@ -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 <[email protected]>
+L:	[email protected]
+S:	Supported
+F:	drivers/scsi/ufs/*dwc*
+
 UNSORTED BLOCK IMAGES (UBI)
 M:	Artem Bityutskiy <[email protected]>
 M:	Richard Weinberger <[email protected]>
drivers/scsi/ufs/Kconfig +41 -0

44 lines not on this page · raw message

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
drivers/scsi/ufs/ufs-dwc-pci.c new file +180 -0

180 lines not on this page · raw message

drivers/scsi/ufs/ufs-dwc.c new file +102 -0

102 lines not on this page · raw message

drivers/scsi/ufs/ufshcd-dwc.c new file +736 -0

736 lines not on this page · raw message

drivers/scsi/ufs/ufshcd-dwc.h new file +18 -0
new file mode 100644
index 0000000..ca65ee1
@@ -0,0 +1,18 @@
+/*
+ * UFS Host driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Joao Pinto <[email protected]>
+ *
+ * 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 */
drivers/scsi/ufs/ufshcd.c +33 -17

142 lines not on this page · raw message

drivers/scsi/ufs/ufshcd.h +13 -0
index 2570d94..e03ea36 100644
@@ -76,6 +76,13 @@ enum dev_cmd_type {
 	DEV_CMD_TYPE_QUERY		= 0x1,
 };
 
+/* UFSHCD states */
+enum {
+	UFSHCD_STATE_RESET,
+	UFSHCD_STATE_ERROR,
+	UFSHCD_STATE_OPERATIONAL,
+};
+
 /**
  * struct uic_command - UIC command structure
  * @command: UIC command
@@ -124,6 +131,8 @@ enum uic_link_state {
 				    UIC_LINK_ACTIVE_STATE)
 #define ufshcd_set_link_hibern8(hba) ((hba)->uic_link_state = \
 				    UIC_LINK_HIBERN8_STATE)
+#define ufshcd_set_ufs_dev_active(h) \
+	((h)->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE)
 
 /*
  * UFS Power management levels.
@@ -634,6 +643,10 @@ extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
 			       u8 attr_set, u32 mib_val, u8 peer);
 extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
 			       u32 *mib_val, u8 peer);
+int ufshcd_dme_link_startup(struct ufs_hba *hba);
+int ufshcd_make_hba_operational(struct ufs_hba *hba);
+int ufshcd_verify_dev_init(struct ufs_hba *hba);
+int ufshcd_complete_dev_init(struct ufs_hba *hba);
 
 /* UIC command interfaces for DME primitives */
 #define DME_LOCAL	0
drivers/scsi/ufs/ufshci-dwc.h new file +42 -0

42 lines not on this page · raw message

drivers/scsi/ufs/ufshci.h +1 -0
index 0ae0967..8dba0e7 100644
@@ -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 {
drivers/scsi/ufs/unipro.h +39 -0
index 816a8a4..da16254 100644
@@ -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,
Signature · 1 line
1.8.1.5
#3 lore raw

Re: [PATCH v6 2/2] add support for DWC UFS Host Controller

Quoted 4 lines · On Wednesday 10 February 2016 16:06:13 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.

Quoted 13 lines
 Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  17 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  41 ++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 180 ++++++
 drivers/scsi/ufs/ufs-dwc.c                        | 102 +++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 736 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshcd.c                         |  50 +-
 drivers/scsi/ufs/ufshcd.h                         |  13 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 ++
 drivers/scsi/ufs/ufshci.h                         |   1 +
 drivers/scsi/ufs/unipro.h                         |  39 ++

Can you split this into separate patches for changes to the common code, the addition of the PCI driver and the addition of the platform driver?

Quoted 9 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +3 -0
new file mode 100644
index 0000000..f38a3f5
@@ -0,0 +1,17 @@
+* Universal Flash Storage (UFS) DesignWare Host Controller
+
+DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+Each UFS controller instance should have its own node.
+
+Required properties:
+- compatible        : compatible string ("snps,ufshcd-1.0", "snps,ufshcd-1.1"
+  or "snps,ufshcd-2.0")
+- reg               : <registers mapping>
+- interrupts        : <interrupt mapping for UFS host controller IRQ>
+
+Example:
+	dwc_ufshcd@0xD0000000 {

Please fix the node name and address in the example. I think you want "ufs@d0000000".

Quoted 6 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +6 -0
+config SCSI_UFS_DWC_MPHY_TC
+	bool "Support for the Synopsys MPHY Test Chip"
+	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
+	---help---
+	  This selects the support for the Synopsys MPHY Test Chip.
+

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+	  Select this if you have a Synopsys MPHY Test Chip.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_40BIT_RMMI
+	bool "40-bit RMMI MPHY"
+	depends on SCSI_UFS_DWC_MPHY_TC
+	---help---
+	  This specifies that the Synopsys MPHY supports 40-bit RMMI operations.
+
+	  Select this if you are using a 40-bit RMMI Synopsys MPHY.

I don't think I understood you explanation why this has to be here, rather than using a proper PHY driver. Please try again.

Quoted 4 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +4 -0
+
+#ifdef CONFIG_PM
+/**
+ * ufs_dw_pci_suspend - suspend power management function

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+ * @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));
+}

Please remove the #ifdef here.

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

Quoted 8 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +8 -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,
+};

Instead, use the macros from include/linux/pm.h

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

Quoted 11 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +11 -0
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+/**
+ * ufshcd_dwc_setup_40bit_rmmi()
+ * This function configures Synopsys MPHY 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)
+{
+	int ret = 0;

This looks like it should go into the external driver

Quoted 6 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +6 -0
+
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+	dev_info(hba->dev, "Configuring MPHY 40-bit RMMI");
+	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "40-bit RMMI configuration failed");

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+		goto out;
+	}
+#else
+	dev_info(hba->dev, "Configuring MPHY 20-bit RMMI");
+	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "20-bit RMMI configuration failed");
+		goto out;
+	}
+#endif

In particular, the part above cannot possibly work: When a distro ships a kernel with CONFIG_SCSI_UFS_DWC_40BIT_RMMI set, it won't ever call the ufshcd_dwc_setup_20bit_rmmi() function, regardless of what the hardware is.

Arnd

author
#4 lore raw

Re: [PATCH v6 2/2] add support for DWC UFS Host Controller

Quoted 19 lines · Arnd Bergmann
Quoted 4 lines · On Wednesday 10 February 2016 16:06:13 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.

Quoted 13 lines
 Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  17 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  41 ++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 180 ++++++
 drivers/scsi/ufs/ufs-dwc.c                        | 102 +++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 736 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshcd.c                         |  50 +-
 drivers/scsi/ufs/ufshcd.h                         |  13 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 ++
 drivers/scsi/ufs/ufshci.h                         |   1 +
 drivers/scsi/ufs/unipro.h                         |  39 ++
Quoted 2 lines

Can you split this into separate patches for changes to the common code, the addition of the PCI driver and the addition of the platform driver?

I can split it in these parts:
- Fix typo
- Add UFS 2.0 support to core driver
- Changes to core driver + dwc platform driver + dwc pci driver

The last one cannot be split because the glue drivers depends on the changes of common code, including the creation of ufshcd-dwc.

Quoted 14 lines
Quoted 14 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +7 -0
new file mode 100644
index 0000000..f38a3f5
@@ -0,0 +1,17 @@
+* 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 string ("snps,ufshcd-1.0", "snps,ufshcd-1.1"
Quoted 8 lines

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

Quoted 6 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +6 -0
+  or "snps,ufshcd-2.0")
+- reg               : <registers mapping>
+- interrupts        : <interrupt mapping for UFS host controller IRQ>
+
+Example:
+	dwc_ufshcd@0xD0000000 {

Please fix the node name and address in the example. I think you want "ufs@d0000000".

Sure.

Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +11 -0
+config SCSI_UFS_DWC_MPHY_TC
+	bool "Support for the Synopsys MPHY Test Chip"
+	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
+	---help---
+	  This selects the support for the Synopsys MPHY Test Chip.
+
+	  Select this if you have a Synopsys MPHY Test Chip.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_40BIT_RMMI
+	bool "40-bit RMMI MPHY"
Quoted 8 lines

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

Quoted 5 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +5 -0
+	depends on SCSI_UFS_DWC_MPHY_TC
+	---help---
+	  This specifies that the Synopsys MPHY supports 40-bit RMMI operations.
+
+	  Select this if you are using a 40-bit RMMI Synopsys MPHY.

I don't think I understood you explanation why this has to be here, rather than using a proper PHY driver. Please try again.

This initialization is for Synopsys Test Chip which needs only some Unipro attribute configuration, which needs to be close to the UFS core since it uses its dme set/get functions. Qcom phy driver is differente, because you are really managing the phy itself, here in the Synopsys Test Chip you only set some Unipro attributes to put controller+Test Chip working together as one.

Quoted 33 lines
Quoted 5 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +4 -0
+
+#ifdef CONFIG_PM
+/**
+ * ufs_dw_pci_suspend - suspend power management function

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+ * @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));
+}

Please remove the #ifdef here.

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

Quoted 8 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +8 -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,
+};

Instead, use the macros from include/linux/pm.h

Quoted 13 lines

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

Quoted 11 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +11 -0
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+/**
+ * ufshcd_dwc_setup_40bit_rmmi()
+ * This function configures Synopsys MPHY 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)
+{
+	int ret = 0;

This looks like it should go into the external driver

This is common init code for platform glue driver and pci glue driver, so it should remain in a common spot and in my option ufshcd-dwc is the right spot.

Quoted 13 lines
Quoted 13 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +12 -0
+
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+	dev_info(hba->dev, "Configuring MPHY 40-bit RMMI");
+	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "40-bit RMMI configuration failed");
+		goto out;
+	}
+#else
+	dev_info(hba->dev, "Configuring MPHY 20-bit RMMI");
+	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
+	if (ret) {
Quoted 8 lines

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

Quoted 4 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +4 -0
+		dev_err(hba->dev, "20-bit RMMI configuration failed");
+		goto out;
+	}
+#endif

In particular, the part above cannot possibly work: When a distro ships a kernel with CONFIG_SCSI_UFS_DWC_40BIT_RMMI set, it won't ever call the ufshcd_dwc_setup_20bit_rmmi() function, regardless of what the hardware is.

You are right... never thought about it that way... I am going to check with the IP guys the best way to go.

Quoted 3 lines

Arnd

Joao
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#5 lore raw

Re: [PATCH v6 2/2] add support for DWC UFS Host Controller

In reply to Arnd Bergmann
Quoted 19 lines · Arnd Bergmann
Quoted 4 lines · On Wednesday 10 February 2016 16:06:13 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.

Quoted 13 lines
 Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  17 +
 MAINTAINERS                                       |   6 +
 drivers/scsi/ufs/Kconfig                          |  41 ++
 drivers/scsi/ufs/Makefile                         |   3 +
 drivers/scsi/ufs/ufs-dwc-pci.c                    | 180 ++++++
 drivers/scsi/ufs/ufs-dwc.c                        | 102 +++
 drivers/scsi/ufs/ufshcd-dwc.c                     | 736 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-dwc.h                     |  18 +
 drivers/scsi/ufs/ufshcd.c                         |  50 +-
 drivers/scsi/ufs/ufshcd.h                         |  13 +
 drivers/scsi/ufs/ufshci-dwc.h                     |  42 ++
 drivers/scsi/ufs/ufshci.h                         |   1 +
 drivers/scsi/ufs/unipro.h                         |  39 ++
Quoted 2 lines

Can you split this into separate patches for changes to the common code, the addition of the PCI driver and the addition of the platform driver?

I can split it in these parts:
- Fix typo
- Add UFS 2.0 support to core driver
- Changes to core driver + dwc platform driver + dwc pci driver

The last one cannot be split because the glue drivers depends on the changes of common code, including the creation of ufshcd-dwc.

Quoted 14 lines
Quoted 14 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +7 -0
new file mode 100644
index 0000000..f38a3f5
@@ -0,0 +1,17 @@
+* 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 string ("snps,ufshcd-1.0", "snps,ufshcd-1.1"
Quoted 8 lines

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

Quoted 6 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +6 -0
+  or "snps,ufshcd-2.0")
+- reg               : <registers mapping>
+- interrupts        : <interrupt mapping for UFS host controller IRQ>
+
+Example:
+	dwc_ufshcd@0xD0000000 {

Please fix the node name and address in the example. I think you want "ufs@d0000000".

Sure.

Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +11 -0
+config SCSI_UFS_DWC_MPHY_TC
+	bool "Support for the Synopsys MPHY Test Chip"
+	depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
+	---help---
+	  This selects the support for the Synopsys MPHY Test Chip.
+
+	  Select this if you have a Synopsys MPHY Test Chip.
+	  If unsure, say N.
+
+config SCSI_UFS_DWC_40BIT_RMMI
+	bool "40-bit RMMI MPHY"
Quoted 8 lines

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

Quoted 5 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +5 -0
+	depends on SCSI_UFS_DWC_MPHY_TC
+	---help---
+	  This specifies that the Synopsys MPHY supports 40-bit RMMI operations.
+
+	  Select this if you are using a 40-bit RMMI Synopsys MPHY.

I don't think I understood you explanation why this has to be here, rather than using a proper PHY driver. Please try again.

This initialization is for Synopsys Test Chip which needs only some Unipro attribute configuration, which needs to be close to the UFS core since it uses its dme set/get functions. Qcom phy driver is differente, because you are really managing the phy itself, here in the Synopsys Test Chip you only set some Unipro attributes to put controller+Test Chip working together as one.

Quoted 17 lines
Quoted 5 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +4 -0
+
+#ifdef CONFIG_PM
+/**
+ * ufs_dw_pci_suspend - suspend power management function

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

Quoted 10 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +10 -0
+ * @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));
+}

Please remove the #ifdef here.

I'll check it out.

Quoted 11 lines

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

Quoted 9 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +8 -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,
+};

Instead, use the macros from include/linux/pm.h

I'll check it out.

Quoted 14 lines

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

Quoted 12 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +11 -0
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+/**
+ * ufshcd_dwc_setup_40bit_rmmi()
+ * This function configures Synopsys MPHY 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)
+{
+	int ret = 0;

This looks like it should go into the external driver

This is common init code for platform glue driver and pci glue driver, so it should remain in a common spot and in my option ufshcd-dwc is the right spot.

Quoted 13 lines
Quoted 13 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +12 -0
+
+#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI
+	dev_info(hba->dev, "Configuring MPHY 40-bit RMMI");
+	ret = ufshcd_dwc_setup_40bit_rmmi(hba);
+	if (ret) {
+		dev_err(hba->dev, "40-bit RMMI configuration failed");
+		goto out;
+	}
+#else
+	dev_info(hba->dev, "Configuring MPHY 20-bit RMMI");
+	ret = ufshcd_dwc_setup_20bit_rmmi(hba);
+	if (ret) {
Quoted 8 lines

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

Quoted 4 lines
Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file +4 -0
+		dev_err(hba->dev, "20-bit RMMI configuration failed");
+		goto out;
+	}
+#endif

In particular, the part above cannot possibly work: When a distro ships a kernel with CONFIG_SCSI_UFS_DWC_40BIT_RMMI set, it won't ever call the ufshcd_dwc_setup_20bit_rmmi() function, regardless of what the hardware is.

You are right... never thought about it that way... I am going to check with the IP guys the best way to go.

Quoted 3 lines

Arnd

Joao