add support for DWC UFS Controller

Fixed typo in ufshcd-pltfrm.

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

Adding UFS 2.0 support to the UFS core driver.

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

Adding UFS 2.0 support to the UFS core driver.

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

---
Changes v8->v10:
- Nothing changed (just to keep up with patch set version). 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(-)

Adding UFS 2.0 support to the UFS core driver.

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

Acked-by Arnd Bergmann <[email protected]>

Acked-by Rob Herring <[email protected]>

Adding UFS 2.0 support to the UFS core driver.

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

Acked-by Arnd Bergmann <[email protected]>

Acked-by Rob Herring <[email protected]>

---
Changes v8->v11:
- Nothing changed (just to keep up with patch set version).
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(-)

Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt +2 -2

index 03c0e98..8d9a9d2 100644
@@ -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"

drivers/scsi/ufs/ufshcd.c index 85cd256..2b5f2bf 100644 renamed from drivers/scsi/ufs/ufshcd.c +0 -0

I think this should go in separate patch

drivers/scsi/ufs/ufshcd.c +1 -0

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

drivers/scsi/ufs/ufshcd.c +6 -1

 	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;

This translation can be pushed to prepare_req_desc_hdr and you end up with ~oneliner fix

drivers/scsi/ufs/ufshcd.c +2 -1

 	/* form UPIU before issuing the command */
-	ufshcd_compose_upiu(hba, lrbp);
+	if (hba->ufs_version == UFSHCI_VERSION_20) {
+		if (likely(lrbp->cmd)) {

How this can be possible NULL, the code above will crash or I'm missing something ?

drivers/scsi/ufs/ufshcd.c +3 -0

+			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+					lrbp->cmd->sc_data_direction);
+			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);

What is different her from the code in ufshcd_compose_upiu ?

drivers/scsi/ufs/ufshcd.c +7 -1

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

This translation can be pushed to prepare_req_desc_hdr and you end up with ~ oneliner fix

drivers/scsi/ufs/ufshcd.c +2 -1

 	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);
 		}

drivers/scsi/ufs/ufshci.h +1 -1

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

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

Adding UFS 2.0 support to the UFS core driver.

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

Acked-by Arnd Bergmann <[email protected]>

Acked-by Rob Herring <[email protected]>

---
Changes v8->v11:
- Nothing changed (just to keep up with patch set version).
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(-)

Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt +2 -2

index 03c0e98..8d9a9d2 100644
@@ -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"

drivers/scsi/ufs/ufshcd.c +0 -0

index 85cd256..2b5f2bf 100644

I think this should go in separate patch

drivers/scsi/ufs/ufshcd.c +1 -0

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

drivers/scsi/ufs/ufshcd.c +6 -1

 	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;

This translation can be pushed to prepare_req_desc_hdr and you end up with ~oneliner fix

drivers/scsi/ufs/ufshcd.c +2 -1

 	/* form UPIU before issuing the command */
-	ufshcd_compose_upiu(hba, lrbp);
+	if (hba->ufs_version == UFSHCI_VERSION_20) {
+		if (likely(lrbp->cmd)) {

How this can be possible NULL, the code above will crash or I'm missing something ?

drivers/scsi/ufs/ufshcd.c +3 -0

+			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+					lrbp->cmd->sc_data_direction);
+			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);

What is different her from the code in ufshcd_compose_upiu ?

drivers/scsi/ufs/ufshcd.c +7 -1

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

This translation can be pushed to prepare_req_desc_hdr and you end up with ~ oneliner fix

drivers/scsi/ufs/ufshcd.c +2 -1

 	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);
 		}

drivers/scsi/ufs/ufshci.h +0 -0

index 0ae0967..8dba0e7 100644

Hi Thomas,

Quoted 37 lines · Tomas

Adding UFS 2.0 support to the UFS core driver.

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

Acked-by Arnd Bergmann <[email protected]>

Acked-by Rob Herring <[email protected]>

---
Changes v8->v11:
- Nothing changed (just to keep up with patch set version).
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(-)

Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt +2 -2

index 03c0e98..8d9a9d2 100644
@@ -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"

drivers/scsi/ufs/ufshcd.c +0 -0

index 85cd256..2b5f2bf 100644

Quoted 1 line

I think this should go in separate patch

In my opinion it only makes sense to add 2.0 to the device-tree binding if the driver actually supports it, that was why I added to the same patch, but of course it can be separated if more people agree with the approach.

Yes, we have ufshcd-pci device that needs that so I think this should go separately

Quoted 28 lines

drivers/scsi/ufs/ufshcd.c +1 -0

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

drivers/scsi/ufs/ufshcd.c +6 -1

 	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;

Quoted 2 lines

This translation can be pushed to prepare_req_desc_hdr and you end up with ~oneliner fix

I think your suggestion is good! We have to check the 2.0 version in 2 places and with your approach we would only check it in prepare_req_desc_hdr() once. I will do that update!

Okay I think you can alter ufshcd_lrb structure and push the information there,
Add ufs version there as hba is not available in req_desc_hdr, I think both command_type and ufs_version can be u8 so the structure won't grow.

Quoted 13 lines

drivers/scsi/ufs/ufshcd.c +2 -1

 	/* form UPIU before issuing the command */
-	ufshcd_compose_upiu(hba, lrbp);
+	if (hba->ufs_version == UFSHCI_VERSION_20) {
+		if (likely(lrbp->cmd)) {

How this can be possible NULL, the code above will crash or I'm missing something ?

drivers/scsi/ufs/ufshcd.c +3 -0

+			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+					lrbp->cmd->sc_data_direction);
+			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);

What is different her from the code in ufshcd_compose_upiu ?

drivers/scsi/ufs/ufshcd.c

Quoted 4 lines

drivers/scsi/ufs/ufshcd.c +4 -0

+		} else
+			err = -EINVAL;
+	} else
+		ufshcd_compose_upiu(hba, lrbp);

If you check ufshcd_compose_upiu() you will see that it contains 2 scopes:
cmd_upiu and query_req_upiu. Before 2.0 this single function approach that had both scopes worked well, but now with 2.0 we must use the same command_type (UTP_CMD_TYPE_UFS_STORAGE) which causes incompatibility. This was why I put the same code from cmd_upiu in the outside.

Of course we can break ufshcd_compose_upiu() in 2: ufshcd_compose_cmd_upiu() and ufshcd_compose_query_upiu(). What do you think? If you use ufs_version only in req_desc_hdr then you don't need this

If (ufs_version == 2.0)
cmd_type = UFS_STORAGE
else

    cmd_type = lrb->command_type 

...
I think with this change

Quoted 7 lines

drivers/scsi/ufs/ufshcd.c +1 -0

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

Quoted 4 lines

drivers/scsi/ufs/ufshcd.c +0 -0

 				complete(hba->dev_cmd.complete);
 		}

drivers/scsi/ufs/ufshci.h +0 -0

index 0ae0967..8dba0e7 100644

drivers/scsi/ufs/ufshci.h enum { · line 276

Quoted 7 lines

drivers/scsi/ufs/ufshci.h +1 -0

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

Why 0x11?

Thanks
Tomas

This patch has the goal to add support for DesignWare UFS Controller specific operations.

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

drivers/scsi/ufs/ufshcd.h +1 -0

@@ -262,6 +262,7 @@ struct ufs_pwr_mode_info {
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
+ * @custom_phy_initialization: used to initialize phys in DWC Cores
  */
 struct ufs_hba_variant_ops {
 	const char *name;

drivers/scsi/ufs/ufshcd.h +5 -1

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

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

This patch adds support for Synopsys G210 Test Chip.

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

This patch adds a glue platform driver for the Synopsys G210 Test Chip.

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

drivers/scsi/ufs/tc-dwc-g210-pltfrm.c new file +17 -0

+/**
+ * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
+ */
+static struct ufs_hba_variant_ops tc_dwc_g210_pltfm_hba_vops = {
+	.name                   = "tc-dwc-g210-pltfm",
+	.link_startup_notify	= ufshcd_dwc_link_startup_notify,
+};
+
+/**
+ * tc_dwc_g210_pltfm_probe()
+ * @pdev: pointer to platform device structure
+ *
+ */
+static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev)
+{
+	int err;
+	struct device *dev = &pdev->dev;

Quoted 3 lines · On Friday 04 March 2016 17:22:18 Joao Pinto wrote:

This patch adds a glue platform driver for the Synopsys G210 Test Chip.

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

Looks basically ok, but I think it can be simplified a little:

Quoted 23 lines

drivers/scsi/ufs/tc-dwc-g210-pltfrm.c new file +23 -0

+/**
+ * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
+ */
+static struct ufs_hba_variant_ops tc_dwc_g210_pltfm_hba_vops = {
+	.name                   = "tc-dwc-g210-pltfm",
+	.link_startup_notify	= ufshcd_dwc_link_startup_notify,
+};
+
+/**
+ * tc_dwc_g210_pltfm_probe()
+ * @pdev: pointer to platform device structure
+ *
+ */
+static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev)
+{
+	int err;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+
+	/* Check Test Chip type and set the specific setup routine */
+	if (of_device_is_compatible(np, "snps, g210-tc-6.00-20bit")) {
+		tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization =
+						tc_dwc_g210_config_20_bit;

drivers/scsi/ufs/tc-dwc-g210-pltfrm.c line 49

Quoted 4 lines

drivers/scsi/ufs/tc-dwc-g210-pltfrm.c new file +4 -0

+	} else if (of_device_is_compatible(np, "snps, g210-tc-6.00-40bit")) {
+		tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization =
+						tc_dwc_g210_config_40_bit;
+	}

Instead of manually checking the compatible string, define two copies of the ufs_hba_variant_ops, and put a pointer to them into the .data field of tc_dwc_g210_pltfm_match.

Arnd

This patch adds a glue pci driver for the Synopsys G210 Test Chip.

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

Quoted 3 lines · On Friday 04 March 2016 17:22:19 Joao Pinto wrote:

This patch adds a glue pci driver for the Synopsys G210 Test Chip.

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

Mostly ok, just a few suggestions:

drivers/scsi/ufs/tc-dwc-g210-pci.c line 27

Quoted 10 lines

drivers/scsi/ufs/tc-dwc-g210-pci.c new file +9 -0

+
+/* Test Chip type expected values */
+#define TC_G210_20BIT 20
+#define TC_G210_40BIT 40
+#define TC_G210_DEFAULTBIT 40
+
+static int tc_type = TC_G210_DEFAULTBIT;
+module_param(tc_type, int, 0);
+MODULE_PARM_DESC(tc_type, "Test Chip Type (20 = 20-bit, 40 = 40-bit)");

What is the effect of setting the wrong one here? I was thinking it would be best to have the default be 'invalid' and then return an error from the probe() function when you neither value is set.

drivers/scsi/ufs/tc-dwc-g210-pci.c line 106

Quoted 10 lines

drivers/scsi/ufs/tc-dwc-g210-pci.c new file +9 -0

+
+	/* Check Test Chip type and set the specific setup routine */
+	if (tc_type == TC_G210_20BIT) {
+		tc_dwc_g210_pci_hba_vops.custom_phy_initialization =
+						tc_dwc_g210_config_20_bit;
+	} else if (tc_type == TC_G210_40BIT) {
+		tc_dwc_g210_pci_hba_vops.custom_phy_initialization =
+						tc_dwc_g210_config_40_bit;
+	}

As for the platform driver, I would define two separate structures here, and then mark the operations as 'const'.

drivers/scsi/ufs/tc-dwc-g210-pci.c line 159

Quoted 5 lines

drivers/scsi/ufs/tc-dwc-g210-pci.c new file +5 -0

+static const struct pci_device_id tc_dwc_g210_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 */
+};

Is there any difference between these two IDs?

Arnd

This patch adds a glue platform driver for the Synopsys G210 Test Chip.

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

---
Changes v10->v11 (Arnd Bergmann):
- vops structs are now passed in .data
Changes v0->v10:
- This patch only appeared in v10

 .../devicetree/bindings/ufs/tc-dwc-g210-pltfrm.txt |  26 +++++
 drivers/scsi/ufs/Kconfig                           |   9 ++
 drivers/scsi/ufs/Makefile                          |   1 +
 drivers/scsi/ufs/tc-dwc-g210-pltfrm.c              | 113 +++++++++++++++++++++
 4 files changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/tc-dwc-g210-pltfrm.txt
 create mode 100644 drivers/scsi/ufs/tc-dwc-g210-pltfrm.c

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

new file mode 100644
index 0000000..6ec9647

Documentation/devicetree/bindings/ufs/tc-dwc-g210-pltfrm.txt new file +4 -0

+			     "jedec,ufs-2.0";
+		reg = < 0xd0000000 0x10000 >;
+		interrupts = < 24 >;
+	};