| Message ID | ABE44455-4F1B-45A2-AED9-150C8E5DBED0@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Arindam, On Apr 25, 2011, at 11:45 PM, Nath, Arindam wrote: > Hi Philip, > > >> -----Original Message----- >> From: Philip Rakity [mailto:prakity@marvell.com] >> Sent: Saturday, April 23, 2011 1:56 AM >> To: linux-mmc@vger.kernel.org >> Cc: Nath, Arindam >> Subject: [PATCH] mmc: eMMC signal voltage does not use CMD11 >> >> >> eMMC chips do not use CMD11 when changing voltage. Add extra >> argument to call to indicate if CMD11 needs to be sent. >> >> Signed-off-by: Philip Rakity <prakity@marvell.com> >> --- >> drivers/mmc/core/sd.c | 4 ++-- >> drivers/mmc/core/sd_ops.c | 4 ++-- >> drivers/mmc/core/sd_ops.h | 3 ++- >> 3 files changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> index ed6b11b..362716c 100644 >> --- a/drivers/mmc/core/sd.c >> +++ b/drivers/mmc/core/sd.c >> @@ -734,7 +734,7 @@ try_again: >> */ >> if (!mmc_host_is_spi(host) && rocr && >> ((*rocr & 0x41000000) == 0x41000000)) { >> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); >> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, >> 1); >> if (err) { >> ocr &= ~(1 << 24); >> goto try_again; >> @@ -1115,7 +1115,7 @@ int mmc_attach_sd(struct mmc_host *host) >> host->ocr_avail = host->ocr_avail_sd; >> >> /* Make sure we are at 3.3V signalling voltage */ >> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); >> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0); >> if (err) >> return err; >> >> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c >> index fc7edc4..6fd35f6 100644 >> --- a/drivers/mmc/core/sd_ops.c >> +++ b/drivers/mmc/core/sd_ops.c >> @@ -149,7 +149,7 @@ int mmc_app_set_bus_width(struct mmc_card *card, >> int width) >> return 0; >> } >> >> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) >> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, >> int cmd11) >> { >> struct mmc_command cmd; >> int err = 0; >> @@ -160,7 +160,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, >> int signal_voltage) >> * Send CMD11 only if the request is to switch the card to >> * 1.8V signalling. >> */ >> - if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { >> + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330 && cmd11) { > > Since we are going to send CMD11 only to change to 1.8V signaling, and when cmd11 is set, isn't it better to have > > if ((signal_voltage == MMC_SIGNAL_VOLTAGE_180) && cmd11) { I am okay with this. Wanted to cover the possible future case for 1.2V support and CMD11. I know eMMC does not do CMD11 but incase SD or SDIO ever did. If you feel strongly will change code. > > Thanks, > Arindam > >> memset(&cmd, 0, sizeof(struct mmc_command)); >> >> cmd.opcode = SD_SWITCH_VOLTAGE; >> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h >> index d5bdda5..2670e71 100644 >> --- a/drivers/mmc/core/sd_ops.h >> +++ b/drivers/mmc/core/sd_ops.h >> @@ -20,7 +20,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 >> *scr); >> int mmc_sd_switch(struct mmc_card *card, int mode, int group, >> u8 value, u8 *resp); >> int mmc_app_sd_status(struct mmc_card *card, void *ssr); >> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); >> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, >> + int cmd11); >> >> #endif >> >> -- >> 1.7.0.4 >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philip, [...] > >> - if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > >> + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330 && cmd11) { > > > > Since we are going to send CMD11 only to change to 1.8V signaling, > and when cmd11 is set, isn't it better to have > > > > if ((signal_voltage == MMC_SIGNAL_VOLTAGE_180) && cmd11) { > > I am okay with this. Wanted to cover the possible future case for 1.2V > support and CMD11. I know eMMC does not do > CMD11 but incase SD or SDIO ever did. If you feel strongly will > change code. If you think that 1.2V might need CMD11, I am okay with your change. Thanks, Arindam -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arindam, Since this mmc_set_signal_voltage() routine will be used by SDIO and MMC should the code be moved the code core.c ? I am not sure about drive strength or driver type since SDIO handles these differently then SD. Philip On Apr 26, 2011, at 8:26 AM, Philip Rakity wrote: > > Hi Arindam, > > On Apr 25, 2011, at 11:45 PM, Nath, Arindam wrote: > >> Hi Philip, >> >> >>> -----Original Message----- >>> From: Philip Rakity [mailto:prakity@marvell.com] >>> Sent: Saturday, April 23, 2011 1:56 AM >>> To: linux-mmc@vger.kernel.org >>> Cc: Nath, Arindam >>> Subject: [PATCH] mmc: eMMC signal voltage does not use CMD11 >>> >>> >>> eMMC chips do not use CMD11 when changing voltage. Add extra >>> argument to call to indicate if CMD11 needs to be sent. >>> >>> Signed-off-by: Philip Rakity <prakity@marvell.com> >>> --- >>> drivers/mmc/core/sd.c | 4 ++-- >>> drivers/mmc/core/sd_ops.c | 4 ++-- >>> drivers/mmc/core/sd_ops.h | 3 ++- >>> 3 files changed, 6 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >>> index ed6b11b..362716c 100644 >>> --- a/drivers/mmc/core/sd.c >>> +++ b/drivers/mmc/core/sd.c >>> @@ -734,7 +734,7 @@ try_again: >>> */ >>> if (!mmc_host_is_spi(host) && rocr && >>> ((*rocr & 0x41000000) == 0x41000000)) { >>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); >>> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, >>> 1); >>> if (err) { >>> ocr &= ~(1 << 24); >>> goto try_again; >>> @@ -1115,7 +1115,7 @@ int mmc_attach_sd(struct mmc_host *host) >>> host->ocr_avail = host->ocr_avail_sd; >>> >>> /* Make sure we are at 3.3V signalling voltage */ >>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); >>> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0); >>> if (err) >>> return err; >>> >>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c >>> index fc7edc4..6fd35f6 100644 >>> --- a/drivers/mmc/core/sd_ops.c >>> +++ b/drivers/mmc/core/sd_ops.c >>> @@ -149,7 +149,7 @@ int mmc_app_set_bus_width(struct mmc_card *card, >>> int width) >>> return 0; >>> } >>> >>> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) >>> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, >>> int cmd11) >>> { >>> struct mmc_command cmd; >>> int err = 0; >>> @@ -160,7 +160,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, >>> int signal_voltage) >>> * Send CMD11 only if the request is to switch the card to >>> * 1.8V signalling. >>> */ >>> - if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { >>> + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330 && cmd11) { >> >> Since we are going to send CMD11 only to change to 1.8V signaling, and when cmd11 is set, isn't it better to have >> >> if ((signal_voltage == MMC_SIGNAL_VOLTAGE_180) && cmd11) { > > I am okay with this. Wanted to cover the possible future case for 1.2V support and CMD11. I know eMMC does not do > CMD11 but incase SD or SDIO ever did. If you feel strongly will change code. >> >> Thanks, >> Arindam >> >>> memset(&cmd, 0, sizeof(struct mmc_command)); >>> >>> cmd.opcode = SD_SWITCH_VOLTAGE; >>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h >>> index d5bdda5..2670e71 100644 >>> --- a/drivers/mmc/core/sd_ops.h >>> +++ b/drivers/mmc/core/sd_ops.h >>> @@ -20,7 +20,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 >>> *scr); >>> int mmc_sd_switch(struct mmc_card *card, int mode, int group, >>> u8 value, u8 *resp); >>> int mmc_app_sd_status(struct mmc_card *card, void *ssr); >>> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); >>> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, >>> + int cmd11); >>> >>> #endif >>> >>> -- >>> 1.7.0.4 >>> >>> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philip, > -----Original Message----- > From: Philip Rakity [mailto:prakity@marvell.com] > Sent: Friday, April 29, 2011 4:16 AM > To: Nath, Arindam > Cc: linux-mmc@vger.kernel.org > Subject: Re: [PATCH] mmc: eMMC signal voltage does not use CMD11 > > > Hi Arindam, > > Since this mmc_set_signal_voltage() routine will be used by SDIO and > MMC should the code be moved > the code core.c ? Yes, mmc_set_signal_voltage() should be moved to core.c to support SDIO and MMC. Do you want me to do it, or you want to make the changes when posting eMMC patches? > > I am not sure about drive strength or driver type since SDIO handles > these differently then SD. I don't think there is much difference in setting Bus Speed Mode and Driver Strength for SDIO3.0 cards, but seems like setting the Current Limit will be a little different from SD3.0. We can work on it after the base support is in the kernel, if that's okay with you. Thanks, Arindam -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index ed6b11b..362716c 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -734,7 +734,7 @@ try_again: */ if (!mmc_host_is_spi(host) && rocr && ((*rocr & 0x41000000) == 0x41000000)) { - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, 1); if (err) { ocr &= ~(1 << 24); goto try_again; @@ -1115,7 +1115,7 @@ int mmc_attach_sd(struct mmc_host *host) host->ocr_avail = host->ocr_avail_sd; /* Make sure we are at 3.3V signalling voltage */ - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, 0); if (err) return err; diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index fc7edc4..6fd35f6 100644 --- a/drivers/mmc/core/sd_ops.c +++ b/drivers/mmc/core/sd_ops.c @@ -149,7 +149,7 @@ int mmc_app_set_bus_width(struct mmc_card *card, int width) return 0; } -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, int cmd11) { struct mmc_command cmd; int err = 0; @@ -160,7 +160,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330 && cmd11) { memset(&cmd, 0, sizeof(struct mmc_command)); cmd.opcode = SD_SWITCH_VOLTAGE; diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h index d5bdda5..2670e71 100644 --- a/drivers/mmc/core/sd_ops.h +++ b/drivers/mmc/core/sd_ops.h @@ -20,7 +20,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr); int mmc_sd_switch(struct mmc_card *card, int mode, int group, u8 value, u8 *resp); int mmc_app_sd_status(struct mmc_card *card, void *ssr); -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, + int cmd11); #endif
eMMC chips do not use CMD11 when changing voltage. Add extra argument to call to indicate if CMD11 needs to be sent. Signed-off-by: Philip Rakity <prakity@marvell.com> --- drivers/mmc/core/sd.c | 4 ++-- drivers/mmc/core/sd_ops.c | 4 ++-- drivers/mmc/core/sd_ops.h | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-)