mmc: dw_mmc: use the use_hold_reg bit in CMD register

PATCH In review Jaehoon Chung 3 participants 4 messages Sep 30, 2011 – Jun 13, 2012

Subsystems (2)

MMC, SD and SDIO Synopsys DesignWare MMC/SD/SDIO driver

lore

Share & tools

lore

Jaehoon Chung

author maintainer

2011-09-30 10:46 UTC #0 lore raw

PATCH 1/1 mmc: dw_mmc: use the use_hold_reg bit in CMD register

This patch is added the use_hold_reg bit in CMD register.

In 2.40a, bit[29] of CMD register is used the use_hold_reg. Some SoC is affected by this bit.
(This bit means whether use hold register when send data and cmd.
And related with cclk_in_drv phase)
if set IMPLEMENT_HOLD_REG in HCON register, i think fine that set this bit by default.

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

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/mmc/host/dw_mmc.c  |    9 +++++++++
 drivers/mmc/host/dw_mmc.h  |    1 +
 include/linux/mmc/dw_mmc.h |    1 +
 3 files changed, 11 insertions(+), 0 deletions(-)

drivers/mmc/host/dw_mmc.c +9 -0

index 0ed1d28..544a616 100644
@@ -276,6 +276,13 @@ static void dw_mci_start_command(struct dw_mci *host,
 	mci_writel(host, CMDARG, cmd->arg);
 	wmb();
 
+	/*
+	 * If use HOLD register,
+	 * CMD and DATA sent to card through HOLD register.
+	 */
+	if (host->use_hold_reg)
+		cmd_flags |= SDMMC_CMD_USE_HOLD_REG;
+
 	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
 }

@@ -1886,6 +1893,8 @@ static int dw_mci_probe(struct platform_device *pdev)
 		host->data_shift = 2;
 	}
 
+	host->use_hold_reg = (mci_readl(host, HCON) >> 22) & 0x1;
+
 	/* Reset all blocks */
 	if (!mci_wait_reset(&pdev->dev, host)) {
 		ret = -ENODEV;

drivers/mmc/host/dw_mmc.h +1 -0

index bfa3c1c..8c4edca 100644
@@ -102,6 +102,7 @@
 #define SDMMC_INT_ERROR			0xbfc2
 /* Command register defines */
 #define SDMMC_CMD_START			BIT(31)
+#define SDMMC_CMD_USE_HOLD_REG		BIT(29)
 #define SDMMC_CMD_CCS_EXP		BIT(23)
 #define SDMMC_CMD_CEATA_RD		BIT(22)
 #define SDMMC_CMD_UPD_CLK		BIT(21)

include/linux/mmc/dw_mmc.h +1 -0

index 6b46819..d6d1515 100644
@@ -147,6 +147,7 @@ struct dw_mci {
 	u32			current_speed;
 	u32			num_slots;
 	u32			fifoth_val;
+	bool			use_hold_reg;
 	struct platform_device	*pdev;
 	struct dw_mci_board	*pdata;
 	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];

James Hogan

2011-09-30 11:05 UTC #1 lore raw

Hi,

Quoted 22 lines · Jaehoon Chung

This patch is added the use_hold_reg bit in CMD register.
In 2.40a, bit[29] of CMD register is used the use_hold_reg. Some SoC is affected by this bit.
(This bit means whether use hold register when send data and cmd.
And related with cclk_in_drv phase)
if set IMPLEMENT_HOLD_REG in HCON register, i think fine that set this bit by default.
Signed-off-by Jaehoon Chung <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/dw_mmc.c  |    9 +++++++++
 drivers/mmc/host/dw_mmc.h  |    1 +
 include/linux/mmc/dw_mmc.h |    1 +
 3 files changed, 11 insertions(+), 0 deletions(-)
drivers/mmc/host/dw_mmc.c +0 -0
index 0ed1d28..544a616 100644

drivers/mmc/host/dw_mmc.c static void dw_mci_start_command(struct dw_mci *host, · line 285

Quoted 10 lines

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

@@ -276,6 +276,13 @@ static void dw_mci_start_command(struct dw_mci *host,
 	mci_writel(host, CMDARG, cmd->arg);
 	wmb();
 
+	/*
+	 * If use HOLD register,
+	 * CMD and DATA sent to card through HOLD register.
+	 */
+	if (host->use_hold_reg)
+		cmd_flags |= SDMMC_CMD_USE_HOLD_REG;
+

Is the HOLD register an internal register that is always required for hardware that has it to work? What effect does it have on the transaction?

drivers/mmc/host/dw_mmc.c static int dw_mci_probe(struct platform_device *pdev) · line 1897

Quoted 9 lines

drivers/mmc/host/dw_mmc.c +2 -0

 	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
 }

@@ -1886,6 +1893,8 @@ static int dw_mci_probe(struct platform_device *pdev)
 		host->data_shift = 2;
 	}
 
+	host->use_hold_reg = (mci_readl(host, HCON) >> 22) & 0x1;
+

it'd be nice to have a #define for the HCON bit.

Quoted 20 lines

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

 	/* Reset all blocks */
 	if (!mci_wait_reset(&pdev->dev, host)) {
 		ret = -ENODEV;

drivers/mmc/host/dw_mmc.h +1 -0

index bfa3c1c..8c4edca 100644
@@ -102,6 +102,7 @@
 #define SDMMC_INT_ERROR			0xbfc2
 /* Command register defines */
 #define SDMMC_CMD_START			BIT(31)
+#define SDMMC_CMD_USE_HOLD_REG		BIT(29)
 #define SDMMC_CMD_CCS_EXP		BIT(23)
 #define SDMMC_CMD_CEATA_RD		BIT(22)
 #define SDMMC_CMD_UPD_CLK		BIT(21)

include/linux/mmc/dw_mmc.h +0 -0

index 6b46819..d6d1515 100644

include/linux/mmc/dw_mmc.h struct dw_mci { · line 150

Quoted 4 lines

include/linux/mmc/dw_mmc.h +1 -0

@@ -147,6 +147,7 @@ struct dw_mci {
 	u32			current_speed;
 	u32			num_slots;
 	u32			fifoth_val;
+	bool			use_hold_reg;

in case of other similar bits being added it may be better to have a copy of the hcon register here, but its fine as it is too.

include/linux/mmc/dw_mmc.h struct dw_mci { · line 153

Quoted 3 lines

include/linux/mmc/dw_mmc.h +0 -0

 	struct platform_device	*pdev;
 	struct dw_mci_board	*pdata;
 	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];

Cheers
James

Jaehoon Chung

author maintainer

2011-10-04 01:21 UTC #2 lore raw

Hi James

Quoted 28 lines · James Hogan

Hi,
Quoted 25 lines · Jaehoon Chung
This patch is added the use_hold_reg bit in CMD register.
In 2.40a, bit[29] of CMD register is used the use_hold_reg. Some SoC is affected by this bit.
(This bit means whether use hold register when send data and cmd.
And related with cclk_in_drv phase)
if set IMPLEMENT_HOLD_REG in HCON register, i think fine that set this bit by default.
Signed-off-by Jaehoon Chung <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/dw_mmc.c  |    9 +++++++++
 drivers/mmc/host/dw_mmc.h  |    1 +
 include/linux/mmc/dw_mmc.h |    1 +
 3 files changed, 11 insertions(+), 0 deletions(-)
drivers/mmc/host/dw_mmc.c +0 -0
index 0ed1d28..544a616 100644
@@ -276,6 +276,13 @@ static void dw_mci_start_command(struct dw_mci *host,
 	mci_writel(host, CMDARG, cmd->arg);
 	wmb();
 

Quoted 10 lines

drivers/mmc/host/dw_mmc.c static void dw_mci_start_command(struct dw_mci *host, · line 285
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +7 -0
+	/*
+	 * If use HOLD register,
+	 * CMD and DATA sent to card through HOLD register.
+	 */
+	if (host->use_hold_reg)
+		cmd_flags |= SDMMC_CMD_USE_HOLD_REG;
+
Is the HOLD register an internal register that is always required for hardware that has it to work? What effect does it have on the transaction?

I known that it's internal register. if we used hold register, it should be work to delay cclk_in to drive the card output.

Quoted 4 lines

Quoted 4 lines
drivers/mmc/host/dw_mmc.c +0 -0
 	mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
 }
 

Quoted 8 lines

drivers/mmc/host/dw_mmc.c static int dw_mci_probe(struct platform_device *pdev) · line 1897
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +2 -0
@@ -1886,6 +1893,8 @@ static int dw_mci_probe(struct platform_device *pdev)
 		host->data_shift = 2;
 	}
 
+	host->use_hold_reg = (mci_readl(host, HCON) >> 22) & 0x1;
+
it'd be nice to have a #define for the HCON bit.

I will add a #define for the HCON

Quoted 43 lines

Quoted 21 lines
drivers/mmc/host/dw_mmc.c +0 -0
 	/* Reset all blocks */
 	if (!mci_wait_reset(&pdev->dev, host)) {
 		ret = -ENODEV;
drivers/mmc/host/dw_mmc.h +1 -0
index bfa3c1c..8c4edca 100644
@@ -102,6 +102,7 @@
 #define SDMMC_INT_ERROR			0xbfc2
 /* Command register defines */
 #define SDMMC_CMD_START			BIT(31)
+#define SDMMC_CMD_USE_HOLD_REG		BIT(29)
 #define SDMMC_CMD_CCS_EXP		BIT(23)
 #define SDMMC_CMD_CEATA_RD		BIT(22)
 #define SDMMC_CMD_UPD_CLK		BIT(21)
include/linux/mmc/dw_mmc.h +0 -0
index 6b46819..d6d1515 100644
include/linux/mmc/dw_mmc.h struct dw_mci { · line 150
Quoted 4 lines
include/linux/mmc/dw_mmc.h +1 -0
@@ -147,6 +147,7 @@ struct dw_mci {
 	u32			current_speed;
 	u32			num_slots;
 	u32			fifoth_val;
+	bool			use_hold_reg;
in case of other similar bits being added it may be better to have a copy of the hcon register here, but its fine as it is too.
include/linux/mmc/dw_mmc.h struct dw_mci { · line 153
Quoted 3 lines
include/linux/mmc/dw_mmc.h +0 -0
 	struct platform_device	*pdev;
 	struct dw_mci_board	*pdata;
 	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];
Cheers
James
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Girish K S

2012-06-13 06:33 UTC #3 lore raw

In reply to Jaehoon Chung

Quoted 22 lines · Jaehoon Chung

This patch is added the use_hold_reg bit in CMD register.
In 2.40a, bit[29] of CMD register is used the use_hold_reg. Some SoC is affected by this bit.
(This bit means whether use hold register when send data and cmd. And related with cclk_in_drv phase)
if set IMPLEMENT_HOLD_REG in HCON register, i think fine that set this bit by default.
Signed-off-by Jaehoon Chung <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
drivers/mmc/host/dw_mmc.c | 9 +++++++++
drivers/mmc/host/dw_mmc.h | 1 +
include/linux/mmc/dw_mmc.h | 1 +
 3 files changed, 11 insertions(+), 0 deletions(-)
drivers/mmc/host/dw_mmc.c +0 -0
index 0ed1d28..544a616 100644

drivers/mmc/host/dw_mmc.c static void dw_mci_start_command(struct dw_mci *host, · line 285

Quoted 10 lines

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

@@ -276,6 +276,13 @@ static void dw_mci_start_command(struct dw_mci *host,
        mci_writel(host, CMDARG, cmd->arg);
        wmb();
 
+       /*
+        * If use HOLD register,
+        * CMD and DATA sent to card through HOLD register.
+        */
+       if (host->use_hold_reg)
+               cmd_flags |= SDMMC_CMD_USE_HOLD_REG;
+

sorry for seeing the patch very late.
I think its not just enough to check the HCON register. it is also necessary to check whether CLKSEL[18:16] field is not 0 in combination with HCON

Quoted 40 lines

drivers/mmc/host/dw_mmc.c +2 -0

        mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START);
  }

@@ -1886,6 +1893,8 @@ static int dw_mci_probe(struct platform_device *pdev)
                host->data_shift = 2;
        }
 
+       host->use_hold_reg = (mci_readl(host, HCON) >> 22) & 0x1;
+
        /* Reset all blocks */
        if (!mci_wait_reset(&pdev->dev, host)) {
                ret = -ENODEV;

drivers/mmc/host/dw_mmc.h +1 -0

index bfa3c1c..8c4edca 100644
@@ -102,6 +102,7 @@
  #define SDMMC_INT_ERROR                        0xbfc2
  /* Command register defines */
  #define SDMMC_CMD_START                        BIT(31)
+#define SDMMC_CMD_USE_HOLD_REG         BIT(29)
  #define SDMMC_CMD_CCS_EXP              BIT(23)
  #define SDMMC_CMD_CEATA_RD             BIT(22)
  #define SDMMC_CMD_UPD_CLK              BIT(21)

include/linux/mmc/dw_mmc.h +1 -1

index 6b46819..d6d1515 100644
@@ -147,6 +147,7 @@ struct dw_mci {
        u32                     current_speed;
        u32                     num_slots;
        u32                     fifoth_val;
+       bool                    use_hold_reg;
        struct platform_device  *pdev;
        struct dw_mci_board     *pdata;
        struct dw_mci_slot      *slot[MAX_MCI_SLOTS];
--

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