mmc: core: hw_reset changes

PATCH Applied Johan Rudholm 3 participants 12 messages Nov 4, 2014 – Nov 6, 2014 MMC, SD and SDIO

Share & tools

Johan Rudholm

author

2014-11-04 15:07 UTC #0 lore raw

v2 cover mmc: core: hw_reset changes

Make the mmc_hw_reset routines more generic, so we can easily add a power cycle of SD cards as well. Also simplify the (e)MMC specific parts of the reset code.

As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC?

v2:
- Call the new bus_ops member hw_reset instead of power_reset
- Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up
- Keep "mmc_hw_reset" naming

Johan Rudholm (4):
mmc: core: use mmc_send_status to check hw_reset
mmc: core: group default initial state
mmc: core: turn hw_reset into a bus_ops
mmc: sd: add hw_reset callback

 drivers/mmc/card/mmc_test.c |    3 -
 drivers/mmc/core/core.c     |   99 +++++++++++++-----------------------------
 drivers/mmc/core/core.h     |    5 ++
 drivers/mmc/core/mmc.c      |   37 ++++++++++++++++
 drivers/mmc/core/sd.c       |   13 ++++++
 include/linux/mmc/core.h    |    1 -
 6 files changed, 86 insertions(+), 72 deletions(-)

Signature · 1 line

1.7.2.5

Johan Rudholm

author

2014-11-04 15:07 UTC #1 lore raw

PATCH v2 1/4 mmc: core: use mmc_send_status to check hw_reset

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/core/core.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

drivers/mmc/core/core.c +2 -8

index 7dc0c85..5d215ee 100644
@@ -2265,15 +2265,9 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
 
 	/* If the reset has happened, then a status command will fail */
 	if (check) {
-		struct mmc_command cmd = {0};
-		int err;
+		u32 status;
 
-		cmd.opcode = MMC_SEND_STATUS;
-		if (!mmc_host_is_spi(card->host))
-			cmd.arg = card->rca << 16;
-		cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
-		err = mmc_wait_for_cmd(card->host, &cmd, 0);
-		if (!err) {
+		if (!mmc_send_status(card, &status)) {
 			mmc_host_clk_release(host);
 			return -ENOSYS;
 		}

Signature · 1 line

1.7.2.5

Johan Rudholm

author

2014-11-04 15:07 UTC #3 lore raw

PATCH v2 2/4 mmc: core: group default initial state

In reply to Johan Rudholm

mmc_do_hw_reset, mmc_power_up and mmc_power_off all set the same group of initial values, simplify by sticking them together.

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/core/core.c |   49 +++++++++++++++++++++++------------------------
 drivers/mmc/core/core.h |    1 +
 2 files changed, 25 insertions(+), 25 deletions(-)

drivers/mmc/core/core.c +24 ~~-25~~

index 5d215ee..2c39d26 100644
@@ -1088,6 +1088,24 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
 	mmc_host_clk_release(host);
 }
 
+/*
+ * Set initial state after a power cycle or a hw_reset.
+ */
+void mmc_set_initial_state(struct mmc_host *host)
+{
+	if (mmc_host_is_spi(host)) {
+		host->ios.chip_select = MMC_CS_HIGH;
+		host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
+	} else {
+		host->ios.chip_select = MMC_CS_DONTCARE;
+		host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
+	}
+	host->ios.bus_width = MMC_BUS_WIDTH_1;
+	host->ios.timing = MMC_TIMING_LEGACY;
+
+	mmc_set_ios(host);
+}
+
 /**
  * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number
  * @vdd:	voltage (mV)

@@ -1534,15 +1552,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	mmc_host_clk_hold(host);
 
 	host->ios.vdd = fls(ocr) - 1;
-	if (mmc_host_is_spi(host))
-		host->ios.chip_select = MMC_CS_HIGH;
-	else
-		host->ios.chip_select = MMC_CS_DONTCARE;
-	host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
 	host->ios.power_mode = MMC_POWER_UP;
-	host->ios.bus_width = MMC_BUS_WIDTH_1;
-	host->ios.timing = MMC_TIMING_LEGACY;
-	mmc_set_ios(host);
+	/* Set initial state and call mmc_set_ios */
+	mmc_set_initial_state(host);
 
 	/* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */
 	if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0)

@@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host)
 	host->ios.clock = 0;
 	host->ios.vdd = 0;
 
-	if (!mmc_host_is_spi(host)) {
-		host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-		host->ios.chip_select = MMC_CS_DONTCARE;
-	}
 	host->ios.power_mode = MMC_POWER_OFF;
-	host->ios.bus_width = MMC_BUS_WIDTH_1;
-	host->ios.timing = MMC_TIMING_LEGACY;
-	mmc_set_ios(host);
+	/* Set initial state and call mmc_set_ios */
+	mmc_set_initial_state(host);
 
 	/*
 	 * Some configurations, such as the 802.11 SDIO card in the OLPC

@@ -2273,16 +2280,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
 		}
 	}
 
-	if (mmc_host_is_spi(host)) {
-		host->ios.chip_select = MMC_CS_HIGH;
-		host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
-	} else {
-		host->ios.chip_select = MMC_CS_DONTCARE;
-		host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-	}
-	host->ios.bus_width = MMC_BUS_WIDTH_1;
-	host->ios.timing = MMC_TIMING_LEGACY;
-	mmc_set_ios(host);
+	/* Set initial state and call mmc_set_ios */
+	mmc_set_initial_state(host);
 
 	mmc_host_clk_release(host);

drivers/mmc/core/core.h +1 -0

index 443a584..d76597c 100644
@@ -49,6 +49,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
 void mmc_power_up(struct mmc_host *host, u32 ocr);
 void mmc_power_off(struct mmc_host *host);
 void mmc_power_cycle(struct mmc_host *host, u32 ocr);
+void mmc_set_initial_state(struct mmc_host *host);
 
 static inline void mmc_delay(unsigned int ms)
 {

Signature · 1 line

1.7.2.5

Ulf Hansson

2014-11-05 10:13 UTC #4 lore raw

Re: [PATCH v2 2/4] mmc: core: group default initial state

Quoted 2 lines · Johan Rudholm

mmc_do_hw_reset, mmc_power_up and mmc_power_off all set the same group of initial values, simplify by sticking them together.

That's not entirely true. This patch will actually change the behaviour for the "ios.bus_mode", which isn't exactly what the commit message are telling us. See more below.

I still think this patch is interesting, since apparently the value of "ios.bus_mode" hasn't been handled consistently. In principle I like the approach of this patch, but we need to make sure it's working and following the specs.

Kind regards
Uffe

Quoted 37 lines

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/core/core.c |   49 +++++++++++++++++++++++------------------------
 drivers/mmc/core/core.h |    1 +
 2 files changed, 25 insertions(+), 25 deletions(-)

drivers/mmc/core/core.c +18 -0

index 5d215ee..2c39d26 100644
@@ -1088,6 +1088,24 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
        mmc_host_clk_release(host);
 }
 
+/*
+ * Set initial state after a power cycle or a hw_reset.
+ */
+void mmc_set_initial_state(struct mmc_host *host)
+{
+       if (mmc_host_is_spi(host)) {
+               host->ios.chip_select = MMC_CS_HIGH;
+               host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
+       } else {
+               host->ios.chip_select = MMC_CS_DONTCARE;
+               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
+       }
+       host->ios.bus_width = MMC_BUS_WIDTH_1;
+       host->ios.timing = MMC_TIMING_LEGACY;
+
+       mmc_set_ios(host);
+}
+
 /**
  * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number
  * @vdd:       voltage (mV)

drivers/mmc/core/core.c void mmc_power_up(struct mmc_host *host, u32 ocr) · line 1541

Quoted 8 lines

drivers/mmc/core/core.c +0 -5

@@ -1534,15 +1552,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
        mmc_host_clk_hold(host);
 
        host->ios.vdd = fls(ocr) - 1;
-       if (mmc_host_is_spi(host))
-               host->ios.chip_select = MMC_CS_HIGH;
-       else
-               host->ios.chip_select = MMC_CS_DONTCARE;
-       host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;

Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on "spi".

Quoted 10 lines

drivers/mmc/core/core.c +2 -3

        host->ios.power_mode = MMC_POWER_UP;
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */
        if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0)

drivers/mmc/core/core.c void mmc_power_off(struct mmc_host *host) · line 1587

Quoted 6 lines

drivers/mmc/core/core.c +0 -3

@@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host)
        host->ios.clock = 0;
        host->ios.vdd = 0;
 
-       if (!mmc_host_is_spi(host)) {
-               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-               host->ios.chip_select = MMC_CS_DONTCARE;

So in here another change in behaviour, right!?

Quoted 42 lines

drivers/mmc/core/core.c +4 ~~-14~~

-       }
        host->ios.power_mode = MMC_POWER_OFF;
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        /*
         * Some configurations, such as the 802.11 SDIO card in the OLPC

@@ -2273,16 +2280,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
                }
        }
 
-       if (mmc_host_is_spi(host)) {
-               host->ios.chip_select = MMC_CS_HIGH;
-               host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
-       } else {
-               host->ios.chip_select = MMC_CS_DONTCARE;
-               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-       }
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        mmc_host_clk_release(host);

drivers/mmc/core/core.h +1 -1

index 443a584..d76597c 100644
@@ -49,6 +49,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
 void mmc_power_up(struct mmc_host *host, u32 ocr);
 void mmc_power_off(struct mmc_host *host);
 void mmc_power_cycle(struct mmc_host *host, u32 ocr);
+void mmc_set_initial_state(struct mmc_host *host);
 
 static inline void mmc_delay(unsigned int ms)
 {
--

Quoted 2 lines

1.7.2.5

Kind regards
Uffe

Johan Rudholm

author

2014-11-05 12:22 UTC #5 lore raw

Re: [PATCH v2 2/4] mmc: core: group default initial state

Hi Ulf,

2014-11-05 11:13 GMT+01:00 Ulf Hansson <[email protected]>:

Quoted 12 lines · Johan Rudholm

Quoted 2 lines
mmc_do_hw_reset, mmc_power_up and mmc_power_off all set the same group of initial values, simplify by sticking them together.
That's not entirely true. This patch will actually change the behaviour for the "ios.bus_mode", which isn't exactly what the commit message are telling us. See more below.
I still think this patch is interesting, since apparently the value of "ios.bus_mode" hasn't been handled consistently. In principle I like the approach of this patch, but we need to make sure it's working and following the specs.

Thank you for pointing this out. I've double-checked the JEDEC 5.01 spec regarding the bus mode in the context of this initial state, see below for comments.

Quoted 43 lines

Kind regards
Uffe

Quoted 39 lines

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/core/core.c |   49 +++++++++++++++++++++++------------------------
 drivers/mmc/core/core.h |    1 +
 2 files changed, 25 insertions(+), 25 deletions(-)

drivers/mmc/core/core.c +18 -0

index 5d215ee..2c39d26 100644
@@ -1088,6 +1088,24 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
        mmc_host_clk_release(host);
 }
 
+/*
+ * Set initial state after a power cycle or a hw_reset.
+ */
+void mmc_set_initial_state(struct mmc_host *host)
+{
+       if (mmc_host_is_spi(host)) {
+               host->ios.chip_select = MMC_CS_HIGH;
+               host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
+       } else {
+               host->ios.chip_select = MMC_CS_DONTCARE;
+               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
+       }
+       host->ios.bus_width = MMC_BUS_WIDTH_1;
+       host->ios.timing = MMC_TIMING_LEGACY;
+
+       mmc_set_ios(host);
+}
+
 /**
  * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number
  * @vdd:       voltage (mV)

@@ -1534,15 +1552,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
        mmc_host_clk_hold(host);

Quoted 8 lines

drivers/mmc/core/core.c void mmc_power_up(struct mmc_host *host, u32 ocr) · line 1541
Quoted 6 lines
drivers/mmc/core/core.c +0 -5
        host->ios.vdd = fls(ocr) - 1;
-       if (mmc_host_is_spi(host))
-               host->ios.chip_select = MMC_CS_HIGH;
-       else
-               host->ios.chip_select = MMC_CS_DONTCARE;
-       host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on "spi".

The JEDEC spec says that in device identification mode the bus should be open-drain and not push-pull. So this way should be compliant to the spec, right? Unfortunately, I will require help in testing this on a proper eMMC.

I can't find anything in the JEDEC or the SD spec about bus mode in SPI mode though. However, this behavior is not changed by this patch, so we should be safe.

Quoted 13 lines

drivers/mmc/core/core.c +2 -3

        host->ios.power_mode = MMC_POWER_UP;
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */
        if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0)

@@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host)
        host->ios.clock = 0;
        host->ios.vdd = 0;

Quoted 5 lines

drivers/mmc/core/core.c void mmc_power_off(struct mmc_host *host) · line 1587
Quoted 3 lines
drivers/mmc/core/core.c +0 -3
-       if (!mmc_host_is_spi(host)) {
-               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-               host->ios.chip_select = MMC_CS_DONTCARE;
So in here another change in behaviour, right!?

Ok, so the behavior for SPI is changed in this case, now we explicitly set the bus+cs modes and before we left them as they were.

Does this matter though, since we're powering off the device and the bus+cs modes will be set on the subsequent mmc_power_up?

Quoted 40 lines

drivers/mmc/core/core.c +4 ~~-14~~

-       }
        host->ios.power_mode = MMC_POWER_OFF;
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        /*
         * Some configurations, such as the 802.11 SDIO card in the OLPC

@@ -2273,16 +2280,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
                }
        }
 
-       if (mmc_host_is_spi(host)) {
-               host->ios.chip_select = MMC_CS_HIGH;
-               host->ios.bus_mode = MMC_BUSMODE_PUSHPULL;
-       } else {
-               host->ios.chip_select = MMC_CS_DONTCARE;
-               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-       }
-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        mmc_host_clk_release(host);

drivers/mmc/core/core.h +1 -0

index 443a584..d76597c 100644
@@ -49,6 +49,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
 void mmc_power_up(struct mmc_host *host, u32 ocr);
 void mmc_power_off(struct mmc_host *host);
 void mmc_power_cycle(struct mmc_host *host, u32 ocr);
+void mmc_set_initial_state(struct mmc_host *host);

Quoted 8 lines

Quoted 5 lines
drivers/mmc/core/core.h +0 -1
 static inline void mmc_delay(unsigned int ms)
 {
--
1.7.2.5
Kind regards
Uffe

//Johan

Ulf Hansson

2014-11-06 09:38 UTC #6 lore raw

Re: [PATCH v2 2/4] mmc: core: group default initial state

[...]

Quoted 7 lines

Quoted 2 lines
Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on "spi".
The JEDEC spec says that in device identification mode the bus should be open-drain and not push-pull. So this way should be compliant to the spec, right? Unfortunately, I will require help in testing this on a proper eMMC.

For MMC yes, but you will break SD/SDIO in this patch.

The default bus mode at initialization needs to be MMC_BUSMODE_PUSHPULL, since that's required by the SD/SDIO spec.

The MMC cards are being initialized after SD and SDIO, which means a switch to MMC_BUSMODE_OPENDRAIN is done in mmc_attach_mmc(). Additionally, to handle resume of MMC cards, mmc_init_card() also do a switch to MMC_BUSMODE_OPENDRAIN.

Quoted 4 lines

I can't find anything in the JEDEC or the SD spec about bus mode in SPI mode though. However, this behavior is not changed by this patch, so we should be safe.

SPI mode has been removed from the eMMC spec.

The SD spec doesn't have open drain mode but do support SPI mode. That tells me that we should keep using MMC_BUSMODE_PUSHPULL for SPI mode.

Quoted 20 lines

Quoted 14 lines

        host->ios.power_mode = MMC_POWER_UP;

drivers/mmc/core/core.c +2 -3

-       host->ios.bus_width = MMC_BUS_WIDTH_1;
-       host->ios.timing = MMC_TIMING_LEGACY;
-       mmc_set_ios(host);
+       /* Set initial state and call mmc_set_ios */
+       mmc_set_initial_state(host);
 
        /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */
        if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0)

@@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host)
        host->ios.clock = 0;
        host->ios.vdd = 0;

Quoted 5 lines

drivers/mmc/core/core.c void mmc_power_off(struct mmc_host *host) · line 1587
Quoted 3 lines
drivers/mmc/core/core.c +0 -3
-       if (!mmc_host_is_spi(host)) {
-               host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN;
-               host->ios.chip_select = MMC_CS_DONTCARE;
So in here another change in behaviour, right!?

Quoted 5 lines

Ok, so the behavior for SPI is changed in this case, now we explicitly set the bus+cs modes and before we left them as they were.
Does this matter though, since we're powering off the device and the bus+cs modes will be set on the subsequent mmc_power_up?

That seems reasonable, it shouldn't be an issue.

Kind regards
Uffe

Johan Rudholm

author

2014-11-06 10:43 UTC #7 lore raw

Re: [PATCH v2 2/4] mmc: core: group default initial state

Hi Ulf,

2014-11-06 10:38 GMT+01:00 Ulf Hansson <[email protected]>:

Quoted 21 lines

[...]
Quoted 7 lines
Quoted 2 lines
Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on "spi".
The JEDEC spec says that in device identification mode the bus should be open-drain and not push-pull. So this way should be compliant to the spec, right? Unfortunately, I will require help in testing this on a proper eMMC.
For MMC yes, but you will break SD/SDIO in this patch.
The default bus mode at initialization needs to be MMC_BUSMODE_PUSHPULL, since that's required by the SD/SDIO spec.
The MMC cards are being initialized after SD and SDIO, which means a switch to MMC_BUSMODE_OPENDRAIN is done in mmc_attach_mmc(). Additionally, to handle resume of MMC cards, mmc_init_card() also do a switch to MMC_BUSMODE_OPENDRAIN.
Quoted 1 line

Quoted 8 lines

Quoted 3 lines
I can't find anything in the JEDEC or the SD spec about bus mode in SPI mode though. However, this behavior is not changed by this patch, so we should be safe.
SPI mode has been removed from the eMMC spec.
The SD spec doesn't have open drain mode but do support SPI mode. That tells me that we should keep using MMC_BUSMODE_PUSHPULL for SPI mode.

Thank you for explaining this. So I think we should be OK by always choosing MMC_BUSMODE_PUSHPULL, because for MMC cards mmc_init_card is called from mmc_power_restore which is called after the hw_reset. Will post a v3 of this patch.

//Johan

Johan Rudholm

author

2014-11-04 15:07 UTC #8 lore raw

PATCH v2 3/4 mmc: core: turn hw_reset into a bus_ops

In reply to Johan Rudholm

Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for reseting SD cards as well.

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/card/mmc_test.c |    3 --
 drivers/mmc/core/core.c     |   48 ++++++++----------------------------------
 drivers/mmc/core/core.h     |    4 +++
 drivers/mmc/core/mmc.c      |   37 +++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h    |    1 -
 5 files changed, 50 insertions(+), 43 deletions(-)

drivers/mmc/card/mmc_test.c +0 -3

index 0c0fc52..2cd3385 100644
@@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test)
 	if (err != -EOPNOTSUPP)
 		return err;
 
-	if (!mmc_can_reset(card))
-		return RESULT_UNSUP_CARD;
-
 	return RESULT_UNSUP_HOST;
 }

drivers/mmc/core/core.c +9 ~~-39~~

73 lines not on this page · raw message

drivers/mmc/core/core.h +4 -0

index d76597c..918bbe8 100644
@@ -27,6 +27,10 @@ struct mmc_bus_ops {
 	int (*power_restore)(struct mmc_host *);
 	int (*alive)(struct mmc_host *);
 	int (*shutdown)(struct mmc_host *);
+	int (*hw_reset)(struct mmc_host *, int);
+#define MMC_HW_RESET_RESET	0
+#define MMC_HW_RESET_TEST	1
+#define MMC_HW_RESET_CHECK	2
 };
 
 void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);

drivers/mmc/core/mmc.c +37 -0

index 793c6f7..55e1a97 100644
@@ -1773,6 +1773,42 @@ static int mmc_power_restore(struct mmc_host *host)
 	return ret;
 }
 
+static int mmc_mmc_hw_reset(struct mmc_host *host, int test)
+{
+	struct mmc_card *card = host->card;
+	u8 rst_n_function;
+
+	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
+		return -EOPNOTSUPP;
+
+	rst_n_function = card->ext_csd.rst_n_function;
+	if ((rst_n_function & EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED)
+		return -EOPNOTSUPP;
+
+	if (test == MMC_HW_RESET_TEST)
+		return 0;
+
+	mmc_host_clk_hold(host);
+	mmc_set_clock(host, host->f_init);
+
+	host->ops->hw_reset(host);
+
+	if (test == MMC_HW_RESET_CHECK) {
+		u32 status;
+
+		if (!mmc_send_status(card, &status)) {
+			mmc_host_clk_release(host);
+			return -ENOSYS;
+		}
+	}
+
+	mmc_set_initial_state(host);
+
+	mmc_host_clk_release(host);
+
+	return 0;
+}
+
 static const struct mmc_bus_ops mmc_ops = {
 	.remove = mmc_remove,
 	.detect = mmc_detect,

@@ -1783,6 +1819,7 @@ static const struct mmc_bus_ops mmc_ops = {
 	.power_restore = mmc_power_restore,
 	.alive = mmc_alive,
 	.shutdown = mmc_shutdown,
+	.hw_reset = mmc_mmc_hw_reset,
 };
 
 /*

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

index f206e29..d7e02a7 100644
@@ -182,7 +182,6 @@ extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
 			      bool is_rel_write);
 extern int mmc_hw_reset(struct mmc_host *host);
 extern int mmc_hw_reset_check(struct mmc_host *host);
-extern int mmc_can_reset(struct mmc_card *card);
 
 extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *);
 extern unsigned int mmc_align_data_size(struct mmc_card *, unsigned int);

Signature · 1 line

1.7.2.5

Adrian Hunter

2014-11-05 12:11 UTC #9 lore raw

Re: [PATCH v2 3/4] mmc: core: turn hw_reset into a bus_ops

Quoted 18 lines · Johan Rudholm

Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for reseting SD cards as well.
Signed-off-by Johan Rudholm <[email protected]>
---
 drivers/mmc/card/mmc_test.c |    3 --
 drivers/mmc/core/core.c     |   48 ++++++++----------------------------------
 drivers/mmc/core/core.h     |    4 +++
 drivers/mmc/core/mmc.c      |   37 +++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h    |    1 -
 5 files changed, 50 insertions(+), 43 deletions(-)
drivers/mmc/card/mmc_test.c +0 -0
index 0c0fc52..2cd3385 100644

drivers/mmc/card/mmc_test.c static int mmc_test_hw_reset(struct mmc_test_card *test) · line 2354

Quoted 6 lines

drivers/mmc/card/mmc_test.c +0 -3

@@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test)
 	if (err != -EOPNOTSUPP)
 		return err;
 
-	if (!mmc_can_reset(card))
-		return RESULT_UNSUP_CARD;
-

I would like to retain the ability for mmc_test to distinguish RESULT_UNSUP_CARD from RESULT_UNSUP_HOST

Quoted 164 lines

drivers/mmc/card/mmc_test.c +0 -0

 	return RESULT_UNSUP_HOST;
 }

drivers/mmc/core/core.c +9 ~~-39~~

index 2c39d26..b35f205 100644
@@ -2239,64 +2239,34 @@ static void mmc_hw_reset_for_init(struct mmc_host *host)
 	mmc_host_clk_release(host);
 }
 
-int mmc_can_reset(struct mmc_card *card)
-{
-	u8 rst_n_function;
-
-	if (!mmc_card_mmc(card))
-		return 0;
-	rst_n_function = card->ext_csd.rst_n_function;
-	if ((rst_n_function & EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED)
-		return 0;
-	return 1;
-}
-EXPORT_SYMBOL(mmc_can_reset);
-
+/* Reset card in a bus-specific way */
 static int mmc_do_hw_reset(struct mmc_host *host, int check)
 {
-	struct mmc_card *card = host->card;
-
-	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
-		return -EOPNOTSUPP;
+	int ret;
 
-	if (!card)
+	if (!host->card)
 		return -EINVAL;
 
-	if (!mmc_can_reset(card))
+	if (!host->bus_ops || !host->bus_ops->hw_reset ||
+			host->bus_ops->hw_reset(host, MMC_HW_RESET_TEST))
 		return -EOPNOTSUPP;
 
-	mmc_host_clk_hold(host);
-	mmc_set_clock(host, host->f_init);
-
-	host->ops->hw_reset(host);
-
-	/* If the reset has happened, then a status command will fail */
-	if (check) {
-		u32 status;
-
-		if (!mmc_send_status(card, &status)) {
-			mmc_host_clk_release(host);
-			return -ENOSYS;
-		}
-	}
-
-	/* Set initial state and call mmc_set_ios */
-	mmc_set_initial_state(host);
+	ret = host->bus_ops->hw_reset(host, check);
 
-	mmc_host_clk_release(host);
+	pr_warn("%s: tried to reset card (%d)\n", mmc_hostname(host), ret);
 
 	return host->bus_ops->power_restore(host);
 }
 
 int mmc_hw_reset(struct mmc_host *host)
 {
-	return mmc_do_hw_reset(host, 0);
+	return mmc_do_hw_reset(host, MMC_HW_RESET_RESET);
 }
 EXPORT_SYMBOL(mmc_hw_reset);
 
 int mmc_hw_reset_check(struct mmc_host *host)
 {
-	return mmc_do_hw_reset(host, 1);
+	return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK);
 }
 EXPORT_SYMBOL(mmc_hw_reset_check);

drivers/mmc/core/core.h +4 -0

index d76597c..918bbe8 100644
@@ -27,6 +27,10 @@ struct mmc_bus_ops {
 	int (*power_restore)(struct mmc_host *);
 	int (*alive)(struct mmc_host *);
 	int (*shutdown)(struct mmc_host *);
+	int (*hw_reset)(struct mmc_host *, int);
+#define MMC_HW_RESET_RESET	0
+#define MMC_HW_RESET_TEST	1
+#define MMC_HW_RESET_CHECK	2
 };
 
 void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);

drivers/mmc/core/mmc.c +37 -0

index 793c6f7..55e1a97 100644
@@ -1773,6 +1773,42 @@ static int mmc_power_restore(struct mmc_host *host)
 	return ret;
 }
 
+static int mmc_mmc_hw_reset(struct mmc_host *host, int test)
+{
+	struct mmc_card *card = host->card;
+	u8 rst_n_function;
+
+	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
+		return -EOPNOTSUPP;
+
+	rst_n_function = card->ext_csd.rst_n_function;
+	if ((rst_n_function & EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED)
+		return -EOPNOTSUPP;
+
+	if (test == MMC_HW_RESET_TEST)
+		return 0;
+
+	mmc_host_clk_hold(host);
+	mmc_set_clock(host, host->f_init);
+
+	host->ops->hw_reset(host);
+
+	if (test == MMC_HW_RESET_CHECK) {
+		u32 status;
+
+		if (!mmc_send_status(card, &status)) {
+			mmc_host_clk_release(host);
+			return -ENOSYS;
+		}
+	}
+
+	mmc_set_initial_state(host);
+
+	mmc_host_clk_release(host);
+
+	return 0;
+}
+
 static const struct mmc_bus_ops mmc_ops = {
 	.remove = mmc_remove,
 	.detect = mmc_detect,

@@ -1783,6 +1819,7 @@ static const struct mmc_bus_ops mmc_ops = {
 	.power_restore = mmc_power_restore,
 	.alive = mmc_alive,
 	.shutdown = mmc_shutdown,
+	.hw_reset = mmc_mmc_hw_reset,
 };
 
 /*

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

index f206e29..d7e02a7 100644
@@ -182,7 +182,6 @@ extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
 			      bool is_rel_write);
 extern int mmc_hw_reset(struct mmc_host *host);
 extern int mmc_hw_reset_check(struct mmc_host *host);
-extern int mmc_can_reset(struct mmc_card *card);
 
 extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *);
 extern unsigned int mmc_align_data_size(struct mmc_card *, unsigned int);

Johan Rudholm

author

2014-11-05 14:41 UTC #10 lore raw

Re: [PATCH v2 3/4] mmc: core: turn hw_reset into a bus_ops

Hi Adrian,

2014-11-05 13:10 GMT+01:00 Adrian Hunter <[email protected]>:

Quoted 22 lines · Johan Rudholm

Quoted 21 lines
Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for reseting SD cards as well.
Signed-off-by Johan Rudholm <[email protected]>
---
 drivers/mmc/card/mmc_test.c |    3 --
 drivers/mmc/core/core.c     |   48 ++++++++----------------------------------
 drivers/mmc/core/core.h     |    4 +++
 drivers/mmc/core/mmc.c      |   37 +++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h    |    1 -
 5 files changed, 50 insertions(+), 43 deletions(-)
drivers/mmc/card/mmc_test.c +0 -0
index 0c0fc52..2cd3385 100644
@@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test)
      if (err != -EOPNOTSUPP)
              return err;
 

Quoted 6 lines

drivers/mmc/card/mmc_test.c static int mmc_test_hw_reset(struct mmc_test_card *test) · line 2354
Quoted 3 lines
drivers/mmc/card/mmc_test.c +0 -3
-     if (!mmc_can_reset(card))
-             return RESULT_UNSUP_CARD;
-
I would like to retain the ability for mmc_test to distinguish RESULT_UNSUP_CARD from RESULT_UNSUP_HOST

Ok, coming up in v3, thanks.

//Johan

Johan Rudholm

author

2014-11-04 15:07 UTC #11 lore raw

PATCH v2 4/4 mmc: sd: add hw_reset callback

In reply to Johan Rudholm

Enable power cycle and re-initialization of SD cards via the hw_reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track.

Signed-off-by Johan Rudholm <[email protected]>

---
 drivers/mmc/core/sd.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

drivers/mmc/core/sd.c +13 -0

index 0c44510..383320e 100644
@@ -1177,6 +1177,18 @@ static int mmc_sd_runtime_resume(struct mmc_host *host)
 	return 0;
 }
 
+static int mmc_sd_hw_reset(struct mmc_host *host, int test)
+{
+	struct mmc_card *card = host->card;
+
+	if (test == MMC_HW_RESET_TEST)
+		return 0;
+
+	mmc_power_cycle(host, card->ocr);
+
+	return 0;
+}
+
 static int mmc_sd_power_restore(struct mmc_host *host)
 {
 	int ret;

@@ -1198,6 +1210,7 @@ static const struct mmc_bus_ops mmc_sd_ops = {
 	.power_restore = mmc_sd_power_restore,
 	.alive = mmc_sd_alive,
 	.shutdown = mmc_sd_suspend,
+	.hw_reset = mmc_sd_hw_reset,
 };
 
 /*

Signature · 1 line

1.7.2.5