mmc: dw_mmc: exynos: Add the exynos3250 compatible

PATCH New Jaehoon Chung 2 participants 7 messages Oct 21, 2015 – Oct 22, 2015

Subsystems (4)

Device tree bindings Arm Samsung Exynos MMC, SD and SDIO Synopsys DesignWare MMC/SD/SDIO driver

lore

Share & tools

lore

Jaehoon Chung

author maintainer

2015-10-21 06:39 UTC #0 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.

Signed-off-by Jaehoon Chung <[email protected]>

---
 Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++
 drivers/mmc/host/dw_mmc-exynos.c                         | 7 +++++++
 2 files changed, 9 insertions(+)

Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +2 -0

index aad9844..cd22ac9 100644
@@ -22,6 +22,8 @@ Required Properties:
 	  specific extensions.
 	- "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7
 	  specific extensions having an SMU.
+	- "samsung,exynos3250-dw-mshc": for controllers with Samsung Exynos3250
+	  specific extensions.
 
 * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface
   unit (ciu) clock. This property is applicable only for Exynos5 SoC's and

drivers/mmc/host/dw_mmc-exynos.c +7 -0

index 1e75309..1c96218 100644
@@ -32,6 +32,7 @@ enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS5420_SMU,
 	DW_MCI_TYPE_EXYNOS7,
 	DW_MCI_TYPE_EXYNOS7_SMU,
+	DW_MCI_TYPE_EXYNOS3250,
 };
 
 /* Exynos implementation specific driver private data */

@@ -73,6 +74,9 @@ static struct dw_mci_exynos_compatible {
 	}, {
 		.compatible	= "samsung,exynos7-dw-mshc-smu",
 		.ctrl_type	= DW_MCI_TYPE_EXYNOS7_SMU,
+	}, {
+		.compatible	= "samsung,exynos3250-dw-mshc",
+		.ctrl_type	= DW_MCI_TYPE_EXYNOS3250,
 	},
 };

@@ -467,6 +471,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
+
 	if (found >= 0) {
 		dw_mci_exynos_set_clksmpl(host, found);
 		priv->tuned_sample = found;

@@ -520,6 +525,8 @@ static const struct of_device_id dw_mci_exynos_match[] = {
 			.data = &exynos_drv_data, },
 	{ .compatible = "samsung,exynos7-dw-mshc-smu",
 			.data = &exynos_drv_data, },
+	{ .compatible = "samsung,exynos3250-dw-mshc",
+			.data = &exynos_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);

Signature · 1 line

1.9.1

Jaehoon Chung

author maintainer

2015-10-21 06:39 UTC #1 lore raw

PATCH 2/2 ARM: dts: change the compatible from exynos5250 to exynos3250

To use more excatly, change compatible from exynos5250 to exynos3250. Exynos3250 and exnos5250 are some difference.

Signed-off-by Jaehoon Chung <[email protected]>

---
 arch/arm/boot/dts/exynos3250.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/boot/dts/exynos3250.dtsi +2 -2

index 033def4..05ed242 100644
@@ -333,7 +333,7 @@
 		};
 
 		mshc_0: mshc at 12510000 {
-			compatible = "samsung,exynos5250-dw-mshc";
+			compatible = "samsung,exynos3250-dw-mshc";
 			reg = <0x12510000 0x1000>;
 			interrupts = <0 142 0>;
 			clocks = <&cmu CLK_SDMMC0>, <&cmu CLK_SCLK_MMC0>;

@@ -345,7 +345,7 @@
 		};
 
 		mshc_1: mshc at 12520000 {
-			compatible = "samsung,exynos5250-dw-mshc";
+			compatible = "samsung,exynos3250-dw-mshc";
 			reg = <0x12520000 0x1000>;
 			interrupts = <0 143 0>;
 			clocks = <&cmu CLK_SDMMC1>, <&cmu CLK_SCLK_MMC1>;

Signature · 1 line

1.9.1

Krzysztof Kozlowski

2015-10-22 00:06 UTC #2 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

In reply to Jaehoon Chung

Quoted 2 lines · Jaehoon Chung

To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.

Hi,

I can't find any difference between old and new compatible. Maybe I am missing something? Maybe there is a difference for these devices?

Anyway if the code is the same then it should use exactly the same compatible.

Best regards,
Krzysztof

Quoted 60 lines

Signed-off-by Jaehoon Chung <[email protected]>

---
 Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++
 drivers/mmc/host/dw_mmc-exynos.c                         | 7 +++++++
 2 files changed, 9 insertions(+)

Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +2 -0

index aad9844..cd22ac9 100644
@@ -22,6 +22,8 @@ Required Properties:
 	  specific extensions.
 	- "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7
 	  specific extensions having an SMU.
+	- "samsung,exynos3250-dw-mshc": for controllers with Samsung Exynos3250
+	  specific extensions.
 
 * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface
   unit (ciu) clock. This property is applicable only for Exynos5 SoC's and

drivers/mmc/host/dw_mmc-exynos.c +7 -0

index 1e75309..1c96218 100644
@@ -32,6 +32,7 @@ enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS5420_SMU,
 	DW_MCI_TYPE_EXYNOS7,
 	DW_MCI_TYPE_EXYNOS7_SMU,
+	DW_MCI_TYPE_EXYNOS3250,
 };
 
 /* Exynos implementation specific driver private data */

@@ -73,6 +74,9 @@ static struct dw_mci_exynos_compatible {
 	}, {
 		.compatible	= "samsung,exynos7-dw-mshc-smu",
 		.ctrl_type	= DW_MCI_TYPE_EXYNOS7_SMU,
+	}, {
+		.compatible	= "samsung,exynos3250-dw-mshc",
+		.ctrl_type	= DW_MCI_TYPE_EXYNOS3250,
 	},
 };

@@ -467,6 +471,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
+
 	if (found >= 0) {
 		dw_mci_exynos_set_clksmpl(host, found);
 		priv->tuned_sample = found;

@@ -520,6 +525,8 @@ static const struct of_device_id dw_mci_exynos_match[] = {
 			.data = &exynos_drv_data, },
 	{ .compatible = "samsung,exynos7-dw-mshc-smu",
 			.data = &exynos_drv_data, },
+	{ .compatible = "samsung,exynos3250-dw-mshc",
+			.data = &exynos_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);

Jaehoon Chung

author maintainer

2015-10-22 02:54 UTC #3 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

Hi, Krzysztof.

Quoted 8 lines · Krzysztof Kozlowski

Quoted 2 lines · Jaehoon Chung
To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.
Hi,
I can't find any difference between old and new compatible. Maybe I am missing something? Maybe there is a difference for these devices?

Sorry, I needs to add more explanation about changing compatible. Exynos3250 has more registers than exynos5250 in dwmmc IP. Now, we used dwmmc controller with exynos5250 compatible. Then some codes can't run in dw-mmc.c, if SoC is exynos3250. For example, there is the checking point in dw_mci_exynos_config_hs400().

static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing)

{
        struct dw_mci_exynos_priv_data *priv = host->priv;
        u32 dqs, strobe;

        /*
         * Not supported to configure register
         * related to HS400
         */
        if (priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420)
                return;

        dqs = priv->saved_dqs_en;
        strobe = priv->saved_strobe_ctrl;

It can't use this feature with exynos5250 compatible.

Does it need to change the compatible from exynos5250 to exynos3250?

Beset Regards,
Jaehoon Chung

Quoted 69 lines

Anyway if the code is the same then it should use exactly the same compatible.

Best regards,
Krzysztof

Quoted 60 lines

Signed-off-by Jaehoon Chung <[email protected]>

---
 Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++
 drivers/mmc/host/dw_mmc-exynos.c                         | 7 +++++++
 2 files changed, 9 insertions(+)

Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +2 -0

index aad9844..cd22ac9 100644
@@ -22,6 +22,8 @@ Required Properties:
 	  specific extensions.
 	- "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7
 	  specific extensions having an SMU.
+	- "samsung,exynos3250-dw-mshc": for controllers with Samsung Exynos3250
+	  specific extensions.
 
 * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface
   unit (ciu) clock. This property is applicable only for Exynos5 SoC's and

drivers/mmc/host/dw_mmc-exynos.c +7 -0

index 1e75309..1c96218 100644
@@ -32,6 +32,7 @@ enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS5420_SMU,
 	DW_MCI_TYPE_EXYNOS7,
 	DW_MCI_TYPE_EXYNOS7_SMU,
+	DW_MCI_TYPE_EXYNOS3250,
 };
 
 /* Exynos implementation specific driver private data */

@@ -73,6 +74,9 @@ static struct dw_mci_exynos_compatible {
 	}, {
 		.compatible	= "samsung,exynos7-dw-mshc-smu",
 		.ctrl_type	= DW_MCI_TYPE_EXYNOS7_SMU,
+	}, {
+		.compatible	= "samsung,exynos3250-dw-mshc",
+		.ctrl_type	= DW_MCI_TYPE_EXYNOS3250,
 	},
 };

@@ -467,6 +471,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
+
 	if (found >= 0) {
 		dw_mci_exynos_set_clksmpl(host, found);
 		priv->tuned_sample = found;

@@ -520,6 +525,8 @@ static const struct of_device_id dw_mci_exynos_match[] = {
 			.data = &exynos_drv_data, },
 	{ .compatible = "samsung,exynos7-dw-mshc-smu",
 			.data = &exynos_drv_data, },
+	{ .compatible = "samsung,exynos3250-dw-mshc",
+			.data = &exynos_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);

Krzysztof Kozlowski

2015-10-22 03:51 UTC #4 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

Quoted 33 lines · Jaehoon Chung

Hi, Krzysztof.
Quoted 8 lines · Krzysztof Kozlowski
Quoted 2 lines · Jaehoon Chung
To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.
Hi,
I can't find any difference between old and new compatible. Maybe I am missing something? Maybe there is a difference for these devices?
Sorry, I needs to add more explanation about changing compatible. Exynos3250 has more registers than exynos5250 in dwmmc IP. Now, we used dwmmc controller with exynos5250 compatible. Then some codes can't run in dw-mmc.c, if SoC is exynos3250. For example, there is the checking point in dw_mci_exynos_config_hs400().
static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing)
{
        struct dw_mci_exynos_priv_data *priv = host->priv;
        u32 dqs, strobe;

        /*
         * Not supported to configure register
         * related to HS400
         */
        if (priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420)
                return;

        dqs = priv->saved_dqs_en;
        strobe = priv->saved_strobe_ctrl;

Quoted 1 line

It can't use this feature with exynos5250 compatible.

Ahh, I see it now. I missed that context and commit message wasn't explaining this real reason. This real reason - supporting HS400 or other features not present on Exynos5250 - was not mentioned.

The new compatible make sense now... but I am wondering if instead just "samsung,exynos5420-dw-mshc" should be used? I think it would give the same result as your change and if new compatible is not required then it will be better not to add it.

Best regards,
Krzysztof

Quoted 77 lines

Does it need to change the compatible from exynos5250 to exynos3250?

Beset Regards,
Jaehoon Chung

Quoted 69 lines

Anyway if the code is the same then it should use exactly the same compatible.

Best regards,
Krzysztof

Quoted 60 lines

Signed-off-by Jaehoon Chung <[email protected]>

---
 Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++
 drivers/mmc/host/dw_mmc-exynos.c                         | 7 +++++++
 2 files changed, 9 insertions(+)

Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +2 -0

index aad9844..cd22ac9 100644
@@ -22,6 +22,8 @@ Required Properties:
 	  specific extensions.
 	- "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7
 	  specific extensions having an SMU.
+	- "samsung,exynos3250-dw-mshc": for controllers with Samsung Exynos3250
+	  specific extensions.
 
 * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface
   unit (ciu) clock. This property is applicable only for Exynos5 SoC's and

drivers/mmc/host/dw_mmc-exynos.c +7 -0

index 1e75309..1c96218 100644
@@ -32,6 +32,7 @@ enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS5420_SMU,
 	DW_MCI_TYPE_EXYNOS7,
 	DW_MCI_TYPE_EXYNOS7_SMU,
+	DW_MCI_TYPE_EXYNOS3250,
 };
 
 /* Exynos implementation specific driver private data */

@@ -73,6 +74,9 @@ static struct dw_mci_exynos_compatible {
 	}, {
 		.compatible	= "samsung,exynos7-dw-mshc-smu",
 		.ctrl_type	= DW_MCI_TYPE_EXYNOS7_SMU,
+	}, {
+		.compatible	= "samsung,exynos3250-dw-mshc",
+		.ctrl_type	= DW_MCI_TYPE_EXYNOS3250,
 	},
 };

@@ -467,6 +471,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
+
 	if (found >= 0) {
 		dw_mci_exynos_set_clksmpl(host, found);
 		priv->tuned_sample = found;

@@ -520,6 +525,8 @@ static const struct of_device_id dw_mci_exynos_match[] = {
 			.data = &exynos_drv_data, },
 	{ .compatible = "samsung,exynos7-dw-mshc-smu",
 			.data = &exynos_drv_data, },
+	{ .compatible = "samsung,exynos3250-dw-mshc",
+			.data = &exynos_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);

Jaehoon Chung

author maintainer

2015-10-22 05:14 UTC #5 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

Quoted 31 lines · Krzysztof Kozlowski

Quoted 30 lines · Jaehoon Chung
Hi, Krzysztof.
Quoted 8 lines · Krzysztof Kozlowski
Quoted 2 lines · Jaehoon Chung
To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.
Hi,
I can't find any difference between old and new compatible. Maybe I am missing something? Maybe there is a difference for these devices?
Sorry, I needs to add more explanation about changing compatible. Exynos3250 has more registers than exynos5250 in dwmmc IP. Now, we used dwmmc controller with exynos5250 compatible. Then some codes can't run in dw-mmc.c, if SoC is exynos3250. For example, there is the checking point in dw_mci_exynos_config_hs400().
static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing)
{
        struct dw_mci_exynos_priv_data *priv = host->priv;
        u32 dqs, strobe;

        /*
         * Not supported to configure register
         * related to HS400
         */
        if (priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420)
                return;

Quoted 8 lines

Quoted 4 lines
        dqs = priv->saved_dqs_en;
        strobe = priv->saved_strobe_ctrl;
It can't use this feature with exynos5250 compatible.
Ahh, I see it now. I missed that context and commit message wasn't explaining this real reason. This real reason - supporting HS400 or other features not present on Exynos5250 - was not mentioned.

Right, it's not enough to explain why needs this patch.

Quoted 5 lines

The new compatible make sense now... but I am wondering if instead just "samsung,exynos5420-dw-mshc" should be used? I think it would give the same result as your change and if new compatible is not required then it will be better not to add it.

I need to check whether it can be used with "samsung,exynos5420-dw-mshc". If you don't want to add new compatible, then i will check Exynos5420 TRM. Maybe, i think it can be used with it.

Is it good that use more exact compatible?

Best Regards,
Jaehoon Chung

Quoted 83 lines

Best regards,
Krzysztof

Quoted 77 lines

Does it need to change the compatible from exynos5250 to exynos3250?

Beset Regards,
Jaehoon Chung

Quoted 69 lines

Anyway if the code is the same then it should use exactly the same compatible.

Best regards,
Krzysztof

Quoted 60 lines

Signed-off-by Jaehoon Chung <[email protected]>

---
 Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt | 2 ++
 drivers/mmc/host/dw_mmc-exynos.c                         | 7 +++++++
 2 files changed, 9 insertions(+)

Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +2 -0

index aad9844..cd22ac9 100644
@@ -22,6 +22,8 @@ Required Properties:
 	  specific extensions.
 	- "samsung,exynos7-dw-mshc-smu": for controllers with Samsung Exynos7
 	  specific extensions having an SMU.
+	- "samsung,exynos3250-dw-mshc": for controllers with Samsung Exynos3250
+	  specific extensions.
 
 * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface
   unit (ciu) clock. This property is applicable only for Exynos5 SoC's and

drivers/mmc/host/dw_mmc-exynos.c +7 -0

index 1e75309..1c96218 100644
@@ -32,6 +32,7 @@ enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS5420_SMU,
 	DW_MCI_TYPE_EXYNOS7,
 	DW_MCI_TYPE_EXYNOS7_SMU,
+	DW_MCI_TYPE_EXYNOS3250,
 };
 
 /* Exynos implementation specific driver private data */

@@ -73,6 +74,9 @@ static struct dw_mci_exynos_compatible {
 	}, {
 		.compatible	= "samsung,exynos7-dw-mshc-smu",
 		.ctrl_type	= DW_MCI_TYPE_EXYNOS7_SMU,
+	}, {
+		.compatible	= "samsung,exynos3250-dw-mshc",
+		.ctrl_type	= DW_MCI_TYPE_EXYNOS3250,
 	},
 };

@@ -467,6 +471,7 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot)
 	} while (start_smpl != smpl);
 
 	found = dw_mci_exynos_get_best_clksmpl(candiates);
+
 	if (found >= 0) {
 		dw_mci_exynos_set_clksmpl(host, found);
 		priv->tuned_sample = found;

@@ -520,6 +525,8 @@ static const struct of_device_id dw_mci_exynos_match[] = {
 			.data = &exynos_drv_data, },
 	{ .compatible = "samsung,exynos7-dw-mshc-smu",
 			.data = &exynos_drv_data, },
+	{ .compatible = "samsung,exynos3250-dw-mshc",
+			.data = &exynos_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);

Krzysztof Kozlowski

2015-10-22 05:57 UTC #6 lore raw

PATCH 1/2 mmc: dw_mmc: exynos: Add the exynos3250 compatible

Quoted 49 lines · Jaehoon Chung

Quoted 31 lines · Krzysztof Kozlowski
Quoted 30 lines · Jaehoon Chung
Hi, Krzysztof.
Quoted 8 lines · Krzysztof Kozlowski
Quoted 2 lines · Jaehoon Chung
To check more exactly, add the exynos3250 compatible. Not use exynos5250 compatibility.
Hi,
I can't find any difference between old and new compatible. Maybe I am missing something? Maybe there is a difference for these devices?
Sorry, I needs to add more explanation about changing compatible. Exynos3250 has more registers than exynos5250 in dwmmc IP. Now, we used dwmmc controller with exynos5250 compatible. Then some codes can't run in dw-mmc.c, if SoC is exynos3250. For example, there is the checking point in dw_mci_exynos_config_hs400().
static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing)
{
        struct dw_mci_exynos_priv_data *priv = host->priv;
        u32 dqs, strobe;

        /*
         * Not supported to configure register
         * related to HS400
         */
        if (priv->ctrl_type < DW_MCI_TYPE_EXYNOS5420)
                return;
Quoted 8 lines
Quoted 4 lines
        dqs = priv->saved_dqs_en;
        strobe = priv->saved_strobe_ctrl;
It can't use this feature with exynos5250 compatible.
Ahh, I see it now. I missed that context and commit message wasn't explaining this real reason. This real reason - supporting HS400 or other features not present on Exynos5250 - was not mentioned.
Right, it's not enough to explain why needs this patch.
Quoted 5 lines
The new compatible make sense now... but I am wondering if instead just "samsung,exynos5420-dw-mshc" should be used? I think it would give the same result as your change and if new compatible is not required then it will be better not to add it.

Quoted 6 lines

I need to check whether it can be used with "samsung,exynos5420-dw-mshc". If you don't want to add new compatible, then i will check Exynos5420 TRM. Maybe, i think it can be used with it.
Is it good that use more exact compatible?

Actually one thing is the real compatibility of devices and second is the source code.

Looking at the source code, the behaviour for new exynos3250 compatible and "samsung,exynos5420-dw-mshc" would be the same, right? This would be a indication for reusing existing compatible. No need to create new one.

However maybe the devices aren't really compatible and using the same compatible would prevent adding new features to Exynos5420?

Best regards,
Krzysztof