| Message ID | 1408715272-13833-4-git-send-email-yuvaraj.cd@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: > Exynos 5250 and 5420 based boards uses built-in CD# line for card > detection.But unfortunately CD# line is on the same voltage rails > as of I/O voltage rails. When we cut off vqmmc,the consequent card > detection will break in these boards. I am not sure I follow here. Is the card detect mechanism handled internally by the dw_mmc controller? I thought HW engineers long time ago realized that this should be done separately on a GPIO line to be able to save power while waiting for a card to be inserted. But that's not case then? > > These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the > time, even when the mmc core tells them to power off. However, one > problem is that these cards won't properly handle mmc_power_cycle(). > That's needed to handle error cases when trying to switch voltages > (see 0797e5f mmc:core: Fixup signal voltage switch). > > This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power > cycle. This mode differs from the normal MMC_POWER_OFF mode in that > the mmc core will promise to power the slot back on before it expects > the host to detect card insertion or removal. > > Also if we let alone the vqmmc turned on when vmmc turned off, the > card could have half way powered and this can damage the card. So > this patch adds a check so that, if the board used the built-in > card detection mechanism i.e through CDETECT, it will not turned > down vqmmc and vmmc both. Why does vmmc needs to be enabled when there are no card inserted? That can't be right? Kind regards Uffe > > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com> > Signed-off-by: Doug Anderson <dianders@chromium.org> > --- > changes from v1: > 1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's suggestion. > 2.added dw_mci_exynos_post_init() to perform the host specific post > initialisation. > 3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host->caps2. > > drivers/mmc/core/core.c | 16 ++++++++++++++-- > drivers/mmc/core/debugfs.c | 3 +++ > drivers/mmc/host/dw_mmc-exynos.c | 12 ++++++++++++ > drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++ > drivers/mmc/host/dw_mmc.h | 2 ++ > include/linux/mmc/host.h | 2 ++ > 6 files changed, 58 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 68f5f4b..79ced36 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) > mmc_host_clk_release(host); > } > > -void mmc_power_off(struct mmc_host *host) > +void _mmc_power_off(struct mmc_host *host, unsigned char power_mode) > { > - if (host->ios.power_mode == MMC_POWER_OFF) > + if (host->ios.power_mode == power_mode) > return; > > mmc_host_clk_hold(host); > @@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host) > host->ios.chip_select = MMC_CS_DONTCARE; > } > host->ios.power_mode = MMC_POWER_OFF; > + host->ios.power_mode = power_mode; > host->ios.bus_width = MMC_BUS_WIDTH_1; > host->ios.timing = MMC_TIMING_LEGACY; > mmc_set_ios(host); > @@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host) > mmc_host_clk_release(host); > } > > +void mmc_power_off(struct mmc_host *host) > +{ > + _mmc_power_off(host, MMC_POWER_OFF); > +} > + > void mmc_power_cycle(struct mmc_host *host, u32 ocr) > { > mmc_power_off(host); > + /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */ > + if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER) > + _mmc_power_off(host, MMC_POWER_OFF_HARD); > + else > + _mmc_power_off(host, MMC_POWER_OFF); > + > /* Wait at least 1 ms according to SD spec */ > mmc_delay(1); > mmc_power_up(host, ocr); > diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c > index 91eb162..3d9c5a3 100644 > --- a/drivers/mmc/core/debugfs.c > +++ b/drivers/mmc/core/debugfs.c > @@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data) > case MMC_POWER_ON: > str = "on"; > break; > + case MMC_POWER_OFF_HARD: > + str = "hard off"; > + break; > default: > str = "invalid"; > break; > diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c > index 0fbc53a..4e26049 100644 > --- a/drivers/mmc/host/dw_mmc-exynos.c > +++ b/drivers/mmc/host/dw_mmc-exynos.c > @@ -17,6 +17,7 @@ > #include <linux/mmc/mmc.h> > #include <linux/of.h> > #include <linux/of_gpio.h> > +#include <linux/mmc/slot-gpio.h> > #include <linux/slab.h> > > #include "dw_mmc.h" > @@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios) > } > } > > +static void dw_mci_exynos_post_init(struct dw_mci_slot *slot) > +{ > + struct dw_mci_board *brd = slot->host->pdata; > + struct mmc_host *mmc = slot->mmc; > + > + if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && > + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) > + mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER; > +} > + > static int dw_mci_exynos_parse_dt(struct dw_mci *host) > { > struct dw_mci_exynos_priv_data *priv; > @@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = { > .prepare_command = dw_mci_exynos_prepare_command, > .set_ios = dw_mci_exynos_set_ios, > .parse_dt = dw_mci_exynos_parse_dt, > + .post_init = dw_mci_exynos_post_init, > .execute_tuning = dw_mci_exynos_execute_tuning, > }; > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index f20b4b8..6f2c681 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) > spin_unlock_bh(&host->lock); > } > > +/* > + * some of the boards use controller CD line for card detection.Unfortunately > + * CD line is bind to the same volatge domain as of the IO lines.If we turn off > + * IO voltage domain, CD line wont work. > + * Return true when controller CD line is used for card detection or return > + * false. > + */ > +static bool dw_mci_builtin_cd(struct dw_mci_slot *slot) > +{ > + struct dw_mci_board *brd = slot->host->pdata; > + struct mmc_host *mmc = slot->mmc; > + > + return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && > + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0; > +} > + > static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > { > struct dw_mci_slot *slot = mmc_priv(mmc); > @@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > mci_writel(slot->host, PWREN, regs); > break; > case MMC_POWER_OFF: > + if (dw_mci_builtin_cd(slot) && > + !test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) > + return; > + > if (!IS_ERR(mmc->supply.vmmc)) > mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0); > > @@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > regs &= ~(1 << slot->id); > mci_writel(slot->host, PWREN, regs); > break; > + case MMC_POWER_OFF_HARD: > + break; > default: > break; > } > @@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) > else > clear_bit(DW_MMC_CARD_PRESENT, &slot->flags); > > + if (drv_data && drv_data->post_init) > + drv_data->post_init(slot); > + > ret = mmc_add_host(mmc); > if (ret) > goto err_setup_bus; > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > index 01b99e8..a3c2628 100644 > --- a/drivers/mmc/host/dw_mmc.h > +++ b/drivers/mmc/host/dw_mmc.h > @@ -250,6 +250,7 @@ struct dw_mci_tuning_data { > * @prepare_command: handle CMD register extensions. > * @set_ios: handle bus specific extensions. > * @parse_dt: parse implementation specific device tree properties. > + * @post_init: implementation specific post initialization. > * @execute_tuning: implementation specific tuning procedure. > * > * Provide controller implementation specific extensions. The usage of this > @@ -263,6 +264,7 @@ struct dw_mci_drv_data { > void (*prepare_command)(struct dw_mci *host, u32 *cmdr); > void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios); > int (*parse_dt)(struct dw_mci *host); > + void (*post_init)(struct dw_mci_slot *slot); > int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode, > struct dw_mci_tuning_data *tuning_data); > }; > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 4cbf614..5eb24ff 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -42,6 +42,7 @@ struct mmc_ios { > #define MMC_POWER_OFF 0 > #define MMC_POWER_UP 1 > #define MMC_POWER_ON 2 > +#define MMC_POWER_OFF_HARD 3 > > unsigned char bus_width; /* data bus width */ > > @@ -283,6 +284,7 @@ struct mmc_host { > #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \ > MMC_CAP2_HS400_1_2V) > #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17) > +#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */ > > mmc_pm_flag_t pm_caps; /* supported pm features */ > > -- > 1.7.10.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
On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >> Exynos 5250 and 5420 based boards uses built-in CD# line for card >> detection.But unfortunately CD# line is on the same voltage rails >> as of I/O voltage rails. When we cut off vqmmc,the consequent card >> detection will break in these boards. > > I am not sure I follow here. > > Is the card detect mechanism handled internally by the dw_mmc controller? Yes > > I thought HW engineers long time ago realized that this should be done > separately on a GPIO line to be able to save power while waiting for a > card to be inserted. But that's not case then? At least in my limited experience, this seems to be common among SoC vendors who are using dw_mmc, as we've seen this elsewhere as well and after seeing it here we know that we need to ignore the CD pin that's routed to dw_mmc and use a separately powered GPIO on the board, but still there are probably many SoCs/boards which are doing it this way. >> >> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >> time, even when the mmc core tells them to power off. However, one >> problem is that these cards won't properly handle mmc_power_cycle(). >> That's needed to handle error cases when trying to switch voltages >> (see 0797e5f mmc:core: Fixup signal voltage switch). >> >> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >> the mmc core will promise to power the slot back on before it expects >> the host to detect card insertion or removal. This patch is based off of one that Doug wrote (sent privately to Yuvaraj) which just modifies the MMC core, and should be split into two patches. One that modifies the mmc core and one that implements this in dw_mmc. >> >> Also if we let alone the vqmmc turned on when vmmc turned off, the >> card could have half way powered and this can damage the card. So >> this patch adds a check so that, if the board used the built-in >> card detection mechanism i.e through CDETECT, it will not turned >> down vqmmc and vmmc both. > > Why does vmmc needs to be enabled when there are no card inserted? > That can't be right? I think this is because we don't want to send power to a card over the I/O pins but not the vcc pin, even for a little while. We also asked some SD card manufacturers about whether it was okay to leave vqmmc on and they recommended not doing this, because there's a risk of damage to the card. So, in this (broken) environment, we basically just keep both vmmc and vqmmc on all the time unless mmc core is asking the driver to power cycle the card through MMC_POWER_OFF and MMC_POWER_ON modes. Does that help? > > Kind regards > Uffe > >> >> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com> >> Signed-off-by: Doug Anderson <dianders@chromium.org> >> --- >> changes from v1: >> 1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's suggestion. >> 2.added dw_mci_exynos_post_init() to perform the host specific post >> initialisation. >> 3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host->caps2. >> >> drivers/mmc/core/core.c | 16 ++++++++++++++-- >> drivers/mmc/core/debugfs.c | 3 +++ >> drivers/mmc/host/dw_mmc-exynos.c | 12 ++++++++++++ >> drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++ >> drivers/mmc/host/dw_mmc.h | 2 ++ >> include/linux/mmc/host.h | 2 ++ >> 6 files changed, 58 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 68f5f4b..79ced36 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) >> mmc_host_clk_release(host); >> } >> >> -void mmc_power_off(struct mmc_host *host) >> +void _mmc_power_off(struct mmc_host *host, unsigned char power_mode) >> { >> - if (host->ios.power_mode == MMC_POWER_OFF) >> + if (host->ios.power_mode == power_mode) >> return; >> >> mmc_host_clk_hold(host); >> @@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host) >> host->ios.chip_select = MMC_CS_DONTCARE; >> } >> host->ios.power_mode = MMC_POWER_OFF; >> + host->ios.power_mode = power_mode; >> host->ios.bus_width = MMC_BUS_WIDTH_1; >> host->ios.timing = MMC_TIMING_LEGACY; >> mmc_set_ios(host); >> @@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host) >> mmc_host_clk_release(host); >> } >> >> +void mmc_power_off(struct mmc_host *host) >> +{ >> + _mmc_power_off(host, MMC_POWER_OFF); >> +} >> + >> void mmc_power_cycle(struct mmc_host *host, u32 ocr) >> { >> mmc_power_off(host); >> + /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */ >> + if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER) >> + _mmc_power_off(host, MMC_POWER_OFF_HARD); >> + else >> + _mmc_power_off(host, MMC_POWER_OFF); >> + >> /* Wait at least 1 ms according to SD spec */ >> mmc_delay(1); >> mmc_power_up(host, ocr); >> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c >> index 91eb162..3d9c5a3 100644 >> --- a/drivers/mmc/core/debugfs.c >> +++ b/drivers/mmc/core/debugfs.c >> @@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data) >> case MMC_POWER_ON: >> str = "on"; >> break; >> + case MMC_POWER_OFF_HARD: >> + str = "hard off"; >> + break; >> default: >> str = "invalid"; >> break; >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c >> index 0fbc53a..4e26049 100644 >> --- a/drivers/mmc/host/dw_mmc-exynos.c >> +++ b/drivers/mmc/host/dw_mmc-exynos.c >> @@ -17,6 +17,7 @@ >> #include <linux/mmc/mmc.h> >> #include <linux/of.h> >> #include <linux/of_gpio.h> >> +#include <linux/mmc/slot-gpio.h> >> #include <linux/slab.h> >> >> #include "dw_mmc.h" >> @@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios) >> } >> } >> >> +static void dw_mci_exynos_post_init(struct dw_mci_slot *slot) >> +{ >> + struct dw_mci_board *brd = slot->host->pdata; >> + struct mmc_host *mmc = slot->mmc; >> + >> + if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && >> + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) >> + mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER; >> +} >> + >> static int dw_mci_exynos_parse_dt(struct dw_mci *host) >> { >> struct dw_mci_exynos_priv_data *priv; >> @@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = { >> .prepare_command = dw_mci_exynos_prepare_command, >> .set_ios = dw_mci_exynos_set_ios, >> .parse_dt = dw_mci_exynos_parse_dt, >> + .post_init = dw_mci_exynos_post_init, >> .execute_tuning = dw_mci_exynos_execute_tuning, >> }; >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index f20b4b8..6f2c681 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) >> spin_unlock_bh(&host->lock); >> } >> >> +/* >> + * some of the boards use controller CD line for card detection.Unfortunately >> + * CD line is bind to the same volatge domain as of the IO lines.If we turn off >> + * IO voltage domain, CD line wont work. >> + * Return true when controller CD line is used for card detection or return >> + * false. >> + */ >> +static bool dw_mci_builtin_cd(struct dw_mci_slot *slot) >> +{ >> + struct dw_mci_board *brd = slot->host->pdata; >> + struct mmc_host *mmc = slot->mmc; >> + >> + return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && >> + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0; >> +} >> + >> static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> { >> struct dw_mci_slot *slot = mmc_priv(mmc); >> @@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> mci_writel(slot->host, PWREN, regs); >> break; >> case MMC_POWER_OFF: >> + if (dw_mci_builtin_cd(slot) && >> + !test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) >> + return; >> + >> if (!IS_ERR(mmc->supply.vmmc)) >> mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0); >> >> @@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> regs &= ~(1 << slot->id); >> mci_writel(slot->host, PWREN, regs); >> break; >> + case MMC_POWER_OFF_HARD: >> + break; >> default: >> break; >> } >> @@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) >> else >> clear_bit(DW_MMC_CARD_PRESENT, &slot->flags); >> >> + if (drv_data && drv_data->post_init) >> + drv_data->post_init(slot); >> + >> ret = mmc_add_host(mmc); >> if (ret) >> goto err_setup_bus; >> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h >> index 01b99e8..a3c2628 100644 >> --- a/drivers/mmc/host/dw_mmc.h >> +++ b/drivers/mmc/host/dw_mmc.h >> @@ -250,6 +250,7 @@ struct dw_mci_tuning_data { >> * @prepare_command: handle CMD register extensions. >> * @set_ios: handle bus specific extensions. >> * @parse_dt: parse implementation specific device tree properties. >> + * @post_init: implementation specific post initialization. >> * @execute_tuning: implementation specific tuning procedure. >> * >> * Provide controller implementation specific extensions. The usage of this >> @@ -263,6 +264,7 @@ struct dw_mci_drv_data { >> void (*prepare_command)(struct dw_mci *host, u32 *cmdr); >> void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios); >> int (*parse_dt)(struct dw_mci *host); >> + void (*post_init)(struct dw_mci_slot *slot); >> int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode, >> struct dw_mci_tuning_data *tuning_data); >> }; >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 4cbf614..5eb24ff 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -42,6 +42,7 @@ struct mmc_ios { >> #define MMC_POWER_OFF 0 >> #define MMC_POWER_UP 1 >> #define MMC_POWER_ON 2 >> +#define MMC_POWER_OFF_HARD 3 >> >> unsigned char bus_width; /* data bus width */ >> >> @@ -283,6 +284,7 @@ struct mmc_host { >> #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \ >> MMC_CAP2_HS400_1_2V) >> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17) >> +#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */ >> >> mmc_pm_flag_t pm_caps; /* supported pm features */ >> >> -- >> 1.7.10.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
On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: > On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>> detection.But unfortunately CD# line is on the same voltage rails >>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>> detection will break in these boards. >> >> I am not sure I follow here. >> >> Is the card detect mechanism handled internally by the dw_mmc controller? > > Yes Just out of curiosity. Do you know how the power to the actual dw_mmc controller is handled? I expect it to be SoC specific and I am guessing power domain regulators may be involved!? > >> >> I thought HW engineers long time ago realized that this should be done >> separately on a GPIO line to be able to save power while waiting for a >> card to be inserted. But that's not case then? > > At least in my limited experience, this seems to be common among SoC > vendors who are using dw_mmc, as we've seen this elsewhere as well and > after seeing it here we know that we need to ignore the CD pin that's > routed to dw_mmc and use a separately powered GPIO on the board, but > still there are probably many SoCs/boards which are doing it this way. > >>> >>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>> time, even when the mmc core tells them to power off. However, one >>> problem is that these cards won't properly handle mmc_power_cycle(). >>> That's needed to handle error cases when trying to switch voltages >>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>> >>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>> the mmc core will promise to power the slot back on before it expects >>> the host to detect card insertion or removal. > > This patch is based off of one that Doug wrote (sent privately to > Yuvaraj) which just modifies the MMC core, and should be split into > two patches. > One that modifies the mmc core and one that implements this in dw_mmc. I looked at the mmc core parts, it seems like the wrong approach. I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this broken card detect mechanism. We even have a DT binding for that, "broken-cd". Kind regards Uffe -- 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
On 08/25/2014 05:13 PM, Ulf Hansson wrote: > On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>> detection.But unfortunately CD# line is on the same voltage rails >>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>> detection will break in these boards. I didn't know that use CD# line for card detect. And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. Which card is used with same voltages? (eMMC? SD? SDIO?) Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>> >>> I am not sure I follow here. >>> >>> Is the card detect mechanism handled internally by the dw_mmc controller? >> >> Yes What card detect mechanism? > > Just out of curiosity. > > Do you know how the power to the actual dw_mmc controller is handled? > I expect it to be SoC specific and I am guessing power domain > regulators may be involved!? > >> >>> >>> I thought HW engineers long time ago realized that this should be done >>> separately on a GPIO line to be able to save power while waiting for a >>> card to be inserted. But that's not case then? >> >> At least in my limited experience, this seems to be common among SoC >> vendors who are using dw_mmc, as we've seen this elsewhere as well and >> after seeing it here we know that we need to ignore the CD pin that's >> routed to dw_mmc and use a separately powered GPIO on the board, but >> still there are probably many SoCs/boards which are doing it this way. >> >>>> >>>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>>> time, even when the mmc core tells them to power off. However, one >>>> problem is that these cards won't properly handle mmc_power_cycle(). >>>> That's needed to handle error cases when trying to switch voltages >>>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>>> >>>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>>> the mmc core will promise to power the slot back on before it expects >>>> the host to detect card insertion or removal. >> >> This patch is based off of one that Doug wrote (sent privately to >> Yuvaraj) which just modifies the MMC core, and should be split into >> two patches. >> One that modifies the mmc core and one that implements this in dw_mmc. > > I looked at the mmc core parts, it seems like the wrong approach. > > I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this > broken card detect mechanism. We even have a DT binding for that, > "broken-cd". I agreed with Ulf's opinion. Best Regards, Jaehoon Chung > > Kind regards > Uffe > -- 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
Ulf, On Mon, Aug 25, 2014 at 1:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>> detection.But unfortunately CD# line is on the same voltage rails >>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>> detection will break in these boards. >>> >>> I am not sure I follow here. >>> >>> Is the card detect mechanism handled internally by the dw_mmc controller? >> >> Yes > > Just out of curiosity. > > Do you know how the power to the actual dw_mmc controller is handled? > I expect it to be SoC specific and I am guessing power domain > regulators may be involved!? You can likely read the dw_mmc registers when vqmmc is off. Is that what you're asking? Certainly if vqmmc is not powered then the lines themselves will be useless, won't they? The "vqmmc" supply goes to the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that "clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same voltage (VDDQ_MMC2) domain. Can you really read a pin without powering that part of the SoC? >>> I thought HW engineers long time ago realized that this should be done >>> separately on a GPIO line to be able to save power while waiting for a >>> card to be inserted. But that's not case then? >> >> At least in my limited experience, this seems to be common among SoC >> vendors who are using dw_mmc, as we've seen this elsewhere as well and >> after seeing it here we know that we need to ignore the CD pin that's >> routed to dw_mmc and use a separately powered GPIO on the board, but >> still there are probably many SoCs/boards which are doing it this way. >> >>>> >>>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>>> time, even when the mmc core tells them to power off. However, one >>>> problem is that these cards won't properly handle mmc_power_cycle(). >>>> That's needed to handle error cases when trying to switch voltages >>>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>>> >>>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>>> the mmc core will promise to power the slot back on before it expects >>>> the host to detect card insertion or removal. >> >> This patch is based off of one that Doug wrote (sent privately to >> Yuvaraj) which just modifies the MMC core, and should be split into >> two patches. >> One that modifies the mmc core and one that implements this in dw_mmc. > > I looked at the mmc core parts, it seems like the wrong approach. > > I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this > broken card detect mechanism. We even have a DT binding for that, > "broken-cd". I don't think this is possible, but let me explain why I think so and you can correct me. The voltage domain of the "card detect" pin on the SoC is vqmmc, right? That means that you won't be able to read the pin without turning on vqmmc. Even if you could read the pin without turning on vqmmc, the pullup on this line is connected to vqmmc too. ...so if vqmmc is off then there's no pulup and you can't use card detect. Are you suggesting that we should flip the voltage of vqmmc (and thus vmmc to prevent damaging the card) during polling? That seems ugly. One other thing to mention: we didn't find any power savings by actually turning off vmmc and vqmmc when there was no card inserted. There's no current running through the lines when there is no card inserted and apparently everything is efficient enough that there was no problem. -Doug -- 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
Jaehoon, On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > On 08/25/2014 05:13 PM, Ulf Hansson wrote: >> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>> detection will break in these boards. > > I didn't know that use CD# line for card detect. > And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. > Which card is used with same voltages? (eMMC? SD? SDIO?) > > Well, I have checked Exynos5250 and 5420, but it looks like not same rails. I'm not sure I totally understood what you said. In my manual I have a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and you'll see the power domain. For both it shows VDDQ_MMC2. If that doesn't mean that the two are in the same voltage domain then I don't know what does. Can you point to any examples where they have different voltage domains? I agree that what exynos does is not sensible, but that's what it is. >>>> I am not sure I follow here. >>>> >>>> Is the card detect mechanism handled internally by the dw_mmc controller? >>> >>> Yes > > What card detect mechanism? The dw_mmc controller has a way to read the card detect, right? That's internal to the controller. The alternative would be to use a generic GPIO for card detect. -- 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
On 25 August 2014 17:20, Doug Anderson <dianders@google.com> wrote: > Ulf, > > On Mon, Aug 25, 2014 at 1:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>> detection will break in these boards. >>>> >>>> I am not sure I follow here. >>>> >>>> Is the card detect mechanism handled internally by the dw_mmc controller? >>> >>> Yes >> >> Just out of curiosity. >> >> Do you know how the power to the actual dw_mmc controller is handled? >> I expect it to be SoC specific and I am guessing power domain >> regulators may be involved!? > > You can likely read the dw_mmc registers when vqmmc is off. Is that > what you're asking? Certainly if vqmmc is not powered then the lines > themselves will be useless, won't they? The "vqmmc" supply goes to > the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that > "clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same > voltage (VDDQ_MMC2) domain. Can you really read a pin without > powering that part of the SoC? Can you verify that you can't re-route these signals to GPIOs on your Exynos board, especially for wp and cd? That would mean, you don't need the internal logic of the dw_mmc to get either card detect or write protect, which would solve your problem. > > >>>> I thought HW engineers long time ago realized that this should be done >>>> separately on a GPIO line to be able to save power while waiting for a >>>> card to be inserted. But that's not case then? >>> >>> At least in my limited experience, this seems to be common among SoC >>> vendors who are using dw_mmc, as we've seen this elsewhere as well and >>> after seeing it here we know that we need to ignore the CD pin that's >>> routed to dw_mmc and use a separately powered GPIO on the board, but >>> still there are probably many SoCs/boards which are doing it this way. >>> >>>>> >>>>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>>>> time, even when the mmc core tells them to power off. However, one >>>>> problem is that these cards won't properly handle mmc_power_cycle(). >>>>> That's needed to handle error cases when trying to switch voltages >>>>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>>>> >>>>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>>>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>>>> the mmc core will promise to power the slot back on before it expects >>>>> the host to detect card insertion or removal. >>> >>> This patch is based off of one that Doug wrote (sent privately to >>> Yuvaraj) which just modifies the MMC core, and should be split into >>> two patches. >>> One that modifies the mmc core and one that implements this in dw_mmc. >> >> I looked at the mmc core parts, it seems like the wrong approach. >> >> I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this >> broken card detect mechanism. We even have a DT binding for that, >> "broken-cd". > > I don't think this is possible, but let me explain why I think so and > you can correct me. > > The voltage domain of the "card detect" pin on the SoC is vqmmc, > right? That means that you won't be able to read the pin without > turning on vqmmc. Even if you could read the pin without turning on > vqmmc, the pullup on this line is connected to vqmmc too. ...so if > vqmmc is off then there's no pulup and you can't use card detect. > > Are you suggesting that we should flip the voltage of vqmmc (and thus > vmmc to prevent damaging the card) during polling? That seems ugly. I am not sure I follow to understand the problem. All I am saying is: From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq. From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc. The rest is taken care of from mmc core, when trying to initialize the card. > > > One other thing to mention: we didn't find any power savings by > actually turning off vmmc and vqmmc when there was no card inserted. > There's no current running through the lines when there is no card > inserted and apparently everything is efficient enough that there was > no problem. On what level of current leakage did you measure? In an system PM suspend state we are chasing uA; I find it hard that no leakage is added when the power is enabled. Anyway, let's leave that as a separate discussion. :-) Kind regards Uffe -- 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
Ulf, On Tue, Aug 26, 2014 at 12:37 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 25 August 2014 17:20, Doug Anderson <dianders@google.com> wrote: >> Ulf, >> >> On Mon, Aug 25, 2014 at 1:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>> detection will break in these boards. >>>>> >>>>> I am not sure I follow here. >>>>> >>>>> Is the card detect mechanism handled internally by the dw_mmc controller? >>>> >>>> Yes >>> >>> Just out of curiosity. >>> >>> Do you know how the power to the actual dw_mmc controller is handled? >>> I expect it to be SoC specific and I am guessing power domain >>> regulators may be involved!? >> >> You can likely read the dw_mmc registers when vqmmc is off. Is that >> what you're asking? Certainly if vqmmc is not powered then the lines >> themselves will be useless, won't they? The "vqmmc" supply goes to >> the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that >> "clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same >> voltage (VDDQ_MMC2) domain. Can you really read a pin without >> powering that part of the SoC? > > Can you verify that you can't re-route these signals to GPIOs on your > Exynos board, especially for wp and cd? Verified. > That would mean, you don't need the internal logic of the dw_mmc to > get either card detect or write protect, which would solve your > problem. Right. ...but since I can't re-route, I can't use this to solve my problem. >>>>> I thought HW engineers long time ago realized that this should be done >>>>> separately on a GPIO line to be able to save power while waiting for a >>>>> card to be inserted. But that's not case then? >>>> >>>> At least in my limited experience, this seems to be common among SoC >>>> vendors who are using dw_mmc, as we've seen this elsewhere as well and >>>> after seeing it here we know that we need to ignore the CD pin that's >>>> routed to dw_mmc and use a separately powered GPIO on the board, but >>>> still there are probably many SoCs/boards which are doing it this way. >>>> >>>>>> >>>>>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>>>>> time, even when the mmc core tells them to power off. However, one >>>>>> problem is that these cards won't properly handle mmc_power_cycle(). >>>>>> That's needed to handle error cases when trying to switch voltages >>>>>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>>>>> >>>>>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>>>>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>>>>> the mmc core will promise to power the slot back on before it expects >>>>>> the host to detect card insertion or removal. >>>> >>>> This patch is based off of one that Doug wrote (sent privately to >>>> Yuvaraj) which just modifies the MMC core, and should be split into >>>> two patches. >>>> One that modifies the mmc core and one that implements this in dw_mmc. >>> >>> I looked at the mmc core parts, it seems like the wrong approach. >>> >>> I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this >>> broken card detect mechanism. We even have a DT binding for that, >>> "broken-cd". >> >> I don't think this is possible, but let me explain why I think so and >> you can correct me. >> >> The voltage domain of the "card detect" pin on the SoC is vqmmc, >> right? That means that you won't be able to read the pin without >> turning on vqmmc. Even if you could read the pin without turning on >> vqmmc, the pullup on this line is connected to vqmmc too. ...so if >> vqmmc is off then there's no pulup and you can't use card detect. >> >> Are you suggesting that we should flip the voltage of vqmmc (and thus >> vmmc to prevent damaging the card) during polling? That seems ugly. > > I am not sure I follow to understand the problem. All I am saying is: > From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq. > From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc. > > The rest is taken care of from mmc core, when trying to initialize the card. We must not be on the same page, so I'll put all my assumptions in super detail and we can see what's different... So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things, right? ...and you are suggesting that I could solve my vqmmc vs. card detect problem by using MMC_CAP_NEEDS_POLL, right? Let's think about the case when no card is inserted. When no card is inserted the core will call set_ios() with MMC_POWER_OFF, right? ...and we want that to turn off vmmc. ...and if we turn off vmmc, we should turn off vqmmc. Now we've got vmmc off and vqmmc off and on this board we can't read the card detect line, right? Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be called to check the card detect line, but with vmmc and vqmmc off. It will be unable to return a sensible value without actually turning on vmmc and vqmmc. ...so with that context, I'll ask my questoin again: Are you suggesting that we should flip the voltage of vqmmc (and thus vmmc to prevent damaging the card) during polling? That seems ugly. >> One other thing to mention: we didn't find any power savings by >> actually turning off vmmc and vqmmc when there was no card inserted. >> There's no current running through the lines when there is no card >> inserted and apparently everything is efficient enough that there was >> no problem. > > On what level of current leakage did you measure? It was not measurable during normal running (S0). > In an system PM suspend state we are chasing uA; I find it hard that > no leakage is added when the power is enabled. Anyway, let's leave > that as a separate discussion. :-) During system PM state we actually _can_ turn power off. We don't need to detect card insertions at that time (card detect is not a wakeup source on this board). -Doug -- 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, Doug, On 08/26/2014 12:25 AM, Doug Anderson wrote: > Jaehoon, > > On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>> detection will break in these boards. >> >> I didn't know that use CD# line for card detect. >> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >> Which card is used with same voltages? (eMMC? SD? SDIO?) >> >> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. > > I'm not sure I totally understood what you said. In my manual I have > a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table > for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and > you'll see the power domain. For both it shows VDDQ_MMC2. If that > doesn't mean that the two are in the same voltage domain then I don't > know what does. Can you point to any examples where they have > different voltage domains? I think you're mis-understanding for it. Right, It's described at exynos5420, but it's not connected. Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers. Because Card-detection rail need to enable as "always-on". We don't need to consider this. I checked the circuit, this patch didn't need. exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API. Best Regards, Jaehoon Chung > > I agree that what exynos does is not sensible, but that's what it is. > > >>>>> I am not sure I follow here. >>>>> >>>>> Is the card detect mechanism handled internally by the dw_mmc controller? >>>> >>>> Yes >> >> What card detect mechanism? > > The dw_mmc controller has a way to read the card detect, right? > That's internal to the controller. The alternative would be to use a > generic GPIO for card detect. > -- 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. On 08/26/2014 12:20 AM, Doug Anderson wrote: > Ulf, > > On Mon, Aug 25, 2014 at 1:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>> detection will break in these boards. >>>> >>>> I am not sure I follow here. >>>> >>>> Is the card detect mechanism handled internally by the dw_mmc controller? >>> >>> Yes >> >> Just out of curiosity. >> >> Do you know how the power to the actual dw_mmc controller is handled? >> I expect it to be SoC specific and I am guessing power domain >> regulators may be involved!? > > You can likely read the dw_mmc registers when vqmmc is off. Is that > what you're asking? Certainly if vqmmc is not powered then the lines > themselves will be useless, won't they? The "vqmmc" supply goes to > the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that > "clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same > voltage (VDDQ_MMC2) domain. Can you really read a pin without > powering that part of the SoC? It's not correct. At TRM, described as same voltage domain. But CD-pin is used with "always-on" power. In circuit, CD# pin is disconnected. > > >>>> I thought HW engineers long time ago realized that this should be done >>>> separately on a GPIO line to be able to save power while waiting for a >>>> card to be inserted. But that's not case then? >>> >>> At least in my limited experience, this seems to be common among SoC >>> vendors who are using dw_mmc, as we've seen this elsewhere as well and >>> after seeing it here we know that we need to ignore the CD pin that's >>> routed to dw_mmc and use a separately powered GPIO on the board, but >>> still there are probably many SoCs/boards which are doing it this way. >>> >>>>> >>>>> These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the >>>>> time, even when the mmc core tells them to power off. However, one >>>>> problem is that these cards won't properly handle mmc_power_cycle(). >>>>> That's needed to handle error cases when trying to switch voltages >>>>> (see 0797e5f mmc:core: Fixup signal voltage switch). >>>>> >>>>> This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power >>>>> cycle. This mode differs from the normal MMC_POWER_OFF mode in that >>>>> the mmc core will promise to power the slot back on before it expects >>>>> the host to detect card insertion or removal. >>> >>> This patch is based off of one that Doug wrote (sent privately to >>> Yuvaraj) which just modifies the MMC core, and should be split into >>> two patches. >>> One that modifies the mmc core and one that implements this in dw_mmc. >> >> I looked at the mmc core parts, it seems like the wrong approach. >> >> I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this >> broken card detect mechanism. We even have a DT binding for that, >> "broken-cd". > > I don't think this is possible, but let me explain why I think so and > you can correct me. Exynos series is using the external gpio-cd concept. So it need not to use MMC_CAP_NEEDS_POLL. Can use the slot-gpio API. In my exynos5 board, it's working fine with the slot-gpio API. Best Regards, Jaehoon Chung > > The voltage domain of the "card detect" pin on the SoC is vqmmc, > right? That means that you won't be able to read the pin without > turning on vqmmc. Even if you could read the pin without turning on > vqmmc, the pullup on this line is connected to vqmmc too. ...so if > vqmmc is off then there's no pulup and you can't use card detect. > > Are you suggesting that we should flip the voltage of vqmmc (and thus > vmmc to prevent damaging the card) during polling? That seems ugly. > > > One other thing to mention: we didn't find any power savings by > actually turning off vmmc and vqmmc when there was no card inserted. > There's no current running through the lines when there is no card > inserted and apparently everything is efficient enough that there was > no problem. > > -Doug > -- 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
Jaehoon, On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi, Doug, > > On 08/26/2014 12:25 AM, Doug Anderson wrote: >> Jaehoon, >> >> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>> detection will break in these boards. >>> >>> I didn't know that use CD# line for card detect. >>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>> >>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >> >> I'm not sure I totally understood what you said. In my manual I have >> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >> you'll see the power domain. For both it shows VDDQ_MMC2. If that >> doesn't mean that the two are in the same voltage domain then I don't >> know what does. Can you point to any examples where they have >> different voltage domains? > I think you're mis-understanding for it. > Right, It's described at exynos5420, but it's not connected. "It's not connected". What do you mean? If I were to guess I'd say that on some particular board you're looking at they don't happen to use the "CD" pin for card detect. If this is what you mean, it doesn't help me. exynos5420-peach-pit does use the CD pin for card detect. You can look at the DTS file and confirm it. ...or are you saying that the CD pin somehow changes voltage domains when configured as a GPIO? I find that very hard to believe. What voltage domain does it go to? If it goes to a 1.8V voltage domain then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V voltage domain then that would be bad when vqmmc was 1.8V. Remember that externally we've got a pull up to vqmmc. Even if somehow magically we can read the card detect pin with vqmmc off, we have an external pullup on this line that goes directly to the "vqmmc" power rail. If the vqmmc power rail is off then this external pull up would not work and would actually act as an external pull down if you could somehow configure the internal line to be a pullup. > Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers. > Because Card-detection rail need to enable as "always-on". > > We don't need to consider this. I checked the circuit, this patch didn't need. > > exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API. When you say "exynos5", what do you mean here? Do you mean the smdk for 5250, or something else? -- 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
Doug, On 08/27/2014 01:14 PM, Doug Anderson wrote: > Jaehoon, > > On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> Hi, Doug, >> >> On 08/26/2014 12:25 AM, Doug Anderson wrote: >>> Jaehoon, >>> >>> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>>> detection will break in these boards. >>>> >>>> I didn't know that use CD# line for card detect. >>>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>>> >>>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>> >>> I'm not sure I totally understood what you said. In my manual I have >>> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >>> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >>> you'll see the power domain. For both it shows VDDQ_MMC2. If that >>> doesn't mean that the two are in the same voltage domain then I don't >>> know what does. Can you point to any examples where they have >>> different voltage domains? >> I think you're mis-understanding for it. >> Right, It's described at exynos5420, but it's not connected. > > "It's not connected". What do you mean? If I were to guess I'd say > that on some particular board you're looking at they don't happen to > use the "CD" pin for card detect. If this is what you mean, it > doesn't help me. exynos5420-peach-pit does use the CD pin for card > detect. You can look at the DTS file and confirm it. I didn't know how exynos5420-peach-pit's circuit is configured. But i guess that almost all exynos5 boards are configured with the similar circuit. At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. (CD-pin is external card-detect pin. - like XEINT_# pin) You mentioned CD# and DATA# lines is used the same power domain, right? In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. Not use the CD# pin, used the XEINT_# pin. So i think we don't need to consider the CD#. If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. Your commit message looks like all exynos5250/5420 board are used CD# line. > > ...or are you saying that the CD pin somehow changes voltage domains > when configured as a GPIO? I find that very hard to believe. What > voltage domain does it go to? If it goes to a 1.8V voltage domain > then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V > voltage domain then that would be bad when vqmmc was 1.8V. Remember > that externally we've got a pull up to vqmmc. It is used with XEINT_# pin instead of CD# pin. As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin, my point is meaningless. :) Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin? Best Regards, Jaehoon Chung > > Even if somehow magically we can read the card detect pin with vqmmc > off, we have an external pullup on this line that goes directly to the > "vqmmc" power rail. If the vqmmc power rail is off then this external > pull up would not work and would actually act as an external pull down > if you could somehow configure the internal line to be a pullup. > > >> Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers. >> Because Card-detection rail need to enable as "always-on". >> >> We don't need to consider this. I checked the circuit, this patch didn't need. >> >> exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API. > > When you say "exynos5", what do you mean here? Do you mean the smdk > for 5250, or something else? > -- 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 Doug, [snip] >>>> I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this >>>> broken card detect mechanism. We even have a DT binding for that, >>>> "broken-cd". >>> >>> I don't think this is possible, but let me explain why I think so and >>> you can correct me. >>> >>> The voltage domain of the "card detect" pin on the SoC is vqmmc, >>> right? That means that you won't be able to read the pin without >>> turning on vqmmc. Even if you could read the pin without turning on >>> vqmmc, the pullup on this line is connected to vqmmc too. ...so if >>> vqmmc is off then there's no pulup and you can't use card detect. >>> >>> Are you suggesting that we should flip the voltage of vqmmc (and thus >>> vmmc to prevent damaging the card) during polling? That seems ugly. >> >> I am not sure I follow to understand the problem. All I am saying is: >> From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq. >> From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc. >> >> The rest is taken care of from mmc core, when trying to initialize the card. > > We must not be on the same page, so I'll put all my assumptions in > super detail and we can see what's different... > > > So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things, > right? ...and you are suggesting that I could solve my vqmmc vs. card > detect problem by using MMC_CAP_NEEDS_POLL, right? Yes. > > > Let's think about the case when no card is inserted. When no card is > inserted the core will call set_ios() with MMC_POWER_OFF, right? > > ...and we want that to turn off vmmc. > ...and if we turn off vmmc, we should turn off vqmmc. Correct. At MMC_POWER_OFF, all host drivers shall turn off all the possible powers that goes to the card. I am just telling this to make sure, we don't think this is a dw_mmc specific thing. > > Now we've got vmmc off and vqmmc off and on this board we can't read > the card detect line, right? Got it. :-) > > Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be > called to check the card detect line, but with vmmc and vqmmc off. It > will be unable to return a sensible value without actually turning on > vmmc and vqmmc. Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule itself with HZ interval. This to check for card insert/removal. Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback to check whether it's worth to continue initialization sequence or if it should re-schedule itself again. If your host driver can distinguish whether a card is inserted, which in this the are when vccq is turned off, your ->get_cd() callback need to return 0. That will allow mmc_rescan() to continue it's initialization sequence and do mmc_power_up(). > > > ...so with that context, I'll ask my questoin again: > > Are you suggesting that we should flip the voltage of vqmmc (and thus > vmmc to prevent damaging the card) during polling? That seems ugly. Nope. Kind regards Uffe -- 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
On 27 August 2014 13:17, Ulf Hansson <ulf.hansson@linaro.org> wrote: > Hi Doug, > > [snip] > >>>>> I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this >>>>> broken card detect mechanism. We even have a DT binding for that, >>>>> "broken-cd". >>>> >>>> I don't think this is possible, but let me explain why I think so and >>>> you can correct me. >>>> >>>> The voltage domain of the "card detect" pin on the SoC is vqmmc, >>>> right? That means that you won't be able to read the pin without >>>> turning on vqmmc. Even if you could read the pin without turning on >>>> vqmmc, the pullup on this line is connected to vqmmc too. ...so if >>>> vqmmc is off then there's no pulup and you can't use card detect. >>>> >>>> Are you suggesting that we should flip the voltage of vqmmc (and thus >>>> vmmc to prevent damaging the card) during polling? That seems ugly. >>> >>> I am not sure I follow to understand the problem. All I am saying is: >>> From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq. >>> From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc. >>> >>> The rest is taken care of from mmc core, when trying to initialize the card. >> >> We must not be on the same page, so I'll put all my assumptions in >> super detail and we can see what's different... >> >> >> So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things, >> right? ...and you are suggesting that I could solve my vqmmc vs. card >> detect problem by using MMC_CAP_NEEDS_POLL, right? > > Yes. > >> >> >> Let's think about the case when no card is inserted. When no card is >> inserted the core will call set_ios() with MMC_POWER_OFF, right? >> >> ...and we want that to turn off vmmc. >> ...and if we turn off vmmc, we should turn off vqmmc. > > Correct. At MMC_POWER_OFF, all host drivers shall turn off all the > possible powers that goes to the card. I am just telling this to make > sure, we don't think this is a dw_mmc specific thing. > >> >> Now we've got vmmc off and vqmmc off and on this board we can't read >> the card detect line, right? > > Got it. :-) > >> >> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >> called to check the card detect line, but with vmmc and vqmmc off. It >> will be unable to return a sensible value without actually turning on >> vmmc and vqmmc. > > Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule > itself with HZ interval. This to check for card insert/removal. > > Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback > to check whether it's worth to continue initialization sequence or if > it should re-schedule itself again. > > If your host driver can distinguish whether a card is inserted, which > in this the are when vccq is turned off, your ->get_cd() callback need /s /the /case > to return 0. That will allow mmc_rescan() to continue it's /s /return 0 /return 1 > initialization sequence and do mmc_power_up(). > >> >> >> ...so with that context, I'll ask my questoin again: >> >> Are you suggesting that we should flip the voltage of vqmmc (and thus >> vmmc to prevent damaging the card) during polling? That seems ugly. > > Nope. > > Kind regards > Uffe -- 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
Jaehoon, On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Doug, > > On 08/27/2014 01:14 PM, Doug Anderson wrote: >> Jaehoon, >> >> On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> Hi, Doug, >>> >>> On 08/26/2014 12:25 AM, Doug Anderson wrote: >>>> Jaehoon, >>>> >>>> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>>>> detection will break in these boards. >>>>> >>>>> I didn't know that use CD# line for card detect. >>>>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>>>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>>>> >>>>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>>> >>>> I'm not sure I totally understood what you said. In my manual I have >>>> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >>>> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >>>> you'll see the power domain. For both it shows VDDQ_MMC2. If that >>>> doesn't mean that the two are in the same voltage domain then I don't >>>> know what does. Can you point to any examples where they have >>>> different voltage domains? >>> I think you're mis-understanding for it. >>> Right, It's described at exynos5420, but it's not connected. >> >> "It's not connected". What do you mean? If I were to guess I'd say >> that on some particular board you're looking at they don't happen to >> use the "CD" pin for card detect. If this is what you mean, it >> doesn't help me. exynos5420-peach-pit does use the CD pin for card >> detect. You can look at the DTS file and confirm it. > > I didn't know how exynos5420-peach-pit's circuit is configured. > But i guess that almost all exynos5 boards are configured with the similar circuit. > > At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. > (CD-pin is external card-detect pin. - like XEINT_# pin) > You mentioned CD# and DATA# lines is used the same power domain, right? > In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. > Not use the CD# pin, used the XEINT_# pin. > So i think we don't need to consider the CD#. > If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. Maybe on your board you have CD connected to a "gpx" line. ...but not mine. The guys who designed our hardware followed the SMDK5420 reference schematics which connect the SD card slot card detect to "gpc2_2", which is the card detect pin. See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting the lack of a GPIO card detect and the inclusion of "sd2_cd" mmc@12220000 { status = "okay"; card-detect-delay = <200>; samsung,dw-mshc-ciu-div = <3>; samsung,dw-mshc-sdr-timing = <2 3>; samsung,dw-mshc-ddr-timing = <1 2>; pinctrl-names = "default"; pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; bus-width = <4>; cap-sd-highspeed; }; See "arch/arm/boot/dts/exynos5420-peach-pit.dts" too: &mmc_2 { status = "okay"; num-slots = <1>; cap-sd-highspeed; card-detect-delay = <200>; clock-frequency = <400000000>; samsung,dw-mshc-ciu-div = <3>; samsung,dw-mshc-sdr-timing = <2 3>; samsung,dw-mshc-ddr-timing = <1 2>; pinctrl-names = "default"; pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; bus-width = <4>; }; Here, see this ASCII art that shows how some lines are hooked up on peach-pit. You might need to paste this into something with a fixed-width font. +-------------------- | Exynos 5420 | | P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7) | | | | +-+- 10K res -+----|- XMMC2CDN (AK6) | | | | | | | | | | Ext CD | | | +-- 10K res-+--+---|- XMMC2CMD (AK8) | | Ext CMD You can see from the above that the external card detect signal (that goes to the connector) named "Ext CD" is pulled up to the same voltage as the external CMD signal (that also goes to the connector). This is vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act as a pull down, or in the best case it will be floating. Said another way: we can't read card detect when vqmmc is off. > Your commit message looks like all exynos5250/5420 board are used CD# line. The commit message should be clearer, agreed. I think I asked Yuvaraj to make sure that the code only invoked this quirk on exynos and only if a GPIO was not used for card detect. Yuvaraj: can you make it more obvious that not all exynos5250/5420 boards need this, only those that use the "official" card detect line? >> ...or are you saying that the CD pin somehow changes voltage domains >> when configured as a GPIO? I find that very hard to believe. What >> voltage domain does it go to? If it goes to a 1.8V voltage domain >> then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V >> voltage domain then that would be bad when vqmmc was 1.8V. Remember >> that externally we've got a pull up to vqmmc. > > It is used with XEINT_# pin instead of CD# pin. > As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin, > my point is meaningless. :) > > Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin? Yes. It is using CD#. Do you remove your objections to this patch, then (once the commit message is clearer)? -Doug -- 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
Ulf, On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >> called to check the card detect line, but with vmmc and vqmmc off. It >> will be unable to return a sensible value without actually turning on >> vmmc and vqmmc. > > Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule > itself with HZ interval. This to check for card insert/removal. > > Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback > to check whether it's worth to continue initialization sequence or if > it should re-schedule itself again. > > If your host driver can distinguish whether a card is inserted, which > in this case are when vccq is turned off, your ->get_cd() callback need > to return 1. That will allow mmc_rescan() to continue it's > initialization sequence and do mmc_power_up(). As per my other email, we can't tell whether a card is inserted when vqmmc is off. Does this mean that you have removed your objections to a solution like Yuvaraj has posted? We still need to break it into two patches (the core part and the dwmmc part), but otherwise is it OK? I can post the original patch I sent Yuvaraj if it's helpful (or he can just include my patch as part 1 of his series). -Doug -- 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
On Wed, Aug 27, 2014 at 9:19 PM, Doug Anderson <dianders@google.com> wrote: > Jaehoon, > > On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> Doug, >> >> On 08/27/2014 01:14 PM, Doug Anderson wrote: >>> Jaehoon, >>> >>> On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>> Hi, Doug, >>>> >>>> On 08/26/2014 12:25 AM, Doug Anderson wrote: >>>>> Jaehoon, >>>>> >>>>> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>>>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>>>>> detection will break in these boards. >>>>>> >>>>>> I didn't know that use CD# line for card detect. >>>>>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>>>>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>>>>> >>>>>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>>>> >>>>> I'm not sure I totally understood what you said. In my manual I have >>>>> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >>>>> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >>>>> you'll see the power domain. For both it shows VDDQ_MMC2. If that >>>>> doesn't mean that the two are in the same voltage domain then I don't >>>>> know what does. Can you point to any examples where they have >>>>> different voltage domains? >>>> I think you're mis-understanding for it. >>>> Right, It's described at exynos5420, but it's not connected. >>> >>> "It's not connected". What do you mean? If I were to guess I'd say >>> that on some particular board you're looking at they don't happen to >>> use the "CD" pin for card detect. If this is what you mean, it >>> doesn't help me. exynos5420-peach-pit does use the CD pin for card >>> detect. You can look at the DTS file and confirm it. >> >> I didn't know how exynos5420-peach-pit's circuit is configured. >> But i guess that almost all exynos5 boards are configured with the similar circuit. >> >> At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. >> (CD-pin is external card-detect pin. - like XEINT_# pin) >> You mentioned CD# and DATA# lines is used the same power domain, right? >> In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. >> Not use the CD# pin, used the XEINT_# pin. >> So i think we don't need to consider the CD#. >> If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. > > Maybe on your board you have CD connected to a "gpx" line. ...but not > mine. The guys who designed our hardware followed the SMDK5420 > reference schematics which connect the SD card slot card detect to > "gpc2_2", which is the card detect pin. > > See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting > the lack of a GPIO card detect and the inclusion of "sd2_cd" > > mmc@12220000 { > status = "okay"; > card-detect-delay = <200>; > samsung,dw-mshc-ciu-div = <3>; > samsung,dw-mshc-sdr-timing = <2 3>; > samsung,dw-mshc-ddr-timing = <1 2>; > pinctrl-names = "default"; > pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; > bus-width = <4>; > cap-sd-highspeed; > }; > > See "arch/arm/boot/dts/exynos5420-peach-pit.dts" too: > > &mmc_2 { > status = "okay"; > num-slots = <1>; > cap-sd-highspeed; > card-detect-delay = <200>; > clock-frequency = <400000000>; > samsung,dw-mshc-ciu-div = <3>; > samsung,dw-mshc-sdr-timing = <2 3>; > samsung,dw-mshc-ddr-timing = <1 2>; > pinctrl-names = "default"; > pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; > bus-width = <4>; > }; > > > Here, see this ASCII art that shows how some lines are hooked up on > peach-pit. You might need to paste this into something with a > fixed-width font. > > +-------------------- > | Exynos 5420 > | > | > P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7) > | | > | | > +-+- 10K res -+----|- XMMC2CDN (AK6) > | | | > | | | > | | | > | Ext CD | > | | > +-- 10K res-+--+---|- XMMC2CMD (AK8) > | > | > Ext CMD > > You can see from the above that the external card detect signal (that > goes to the connector) named "Ext CD" is pulled up to the same voltage > as the external CMD signal (that also goes to the connector). This is > vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act > as a pull down, or in the best case it will be floating. > > Said another way: we can't read card detect when vqmmc is off. > >> Your commit message looks like all exynos5250/5420 board are used CD# line. > > The commit message should be clearer, agreed. I think I asked Yuvaraj > to make sure that the code only invoked this quirk on exynos and only > if a GPIO was not used for card detect. Yuvaraj: can you make it more > obvious that not all exynos5250/5420 boards need this, only those that > use the "official" card detect line? if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) This check will not enable the quirk, if a GPIO was used for the card detect.Did I miss something? I will change the commit message to exclude "all exynos5250/5420 boards". > > >>> ...or are you saying that the CD pin somehow changes voltage domains >>> when configured as a GPIO? I find that very hard to believe. What >>> voltage domain does it go to? If it goes to a 1.8V voltage domain >>> then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V >>> voltage domain then that would be bad when vqmmc was 1.8V. Remember >>> that externally we've got a pull up to vqmmc. >> >> It is used with XEINT_# pin instead of CD# pin. >> As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin, >> my point is meaningless. :) >> >> Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin? > > Yes. It is using CD#. Do you remove your objections to this patch, > then (once the commit message is clearer)? > > -Doug -- 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
On 27 August 2014 17:52, Doug Anderson <dianders@google.com> wrote: > Ulf, > > On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >>> called to check the card detect line, but with vmmc and vqmmc off. It >>> will be unable to return a sensible value without actually turning on >>> vmmc and vqmmc. >> >> Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule >> itself with HZ interval. This to check for card insert/removal. >> >> Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback >> to check whether it's worth to continue initialization sequence or if >> it should re-schedule itself again. >> >> If your host driver can distinguish whether a card is inserted, which >> in this case are when vccq is turned off, your ->get_cd() callback need >> to return 1. That will allow mmc_rescan() to continue it's >> initialization sequence and do mmc_power_up(). > > As per my other email, we can't tell whether a card is inserted when > vqmmc is off. I understand. The solution I proposed above, is: 1) Enable MMC_CAP_NEEDS_POLL. 2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted. That will solve this issue and without messing up the mmc core. > > Does this mean that you have removed your objections to a solution > like Yuvaraj has posted? We still need to break it into two patches > (the core part and the dwmmc part), but otherwise is it OK? I can > post the original patch I sent Yuvaraj if it's helpful (or he can just > include my patch as part 1 of his series). No. This can entirely be fixed in the host driver. As suggested above. Kind regards Uffe -- 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
On 08/28/2014 12:49 AM, Doug Anderson wrote: > Jaehoon, > > On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> Doug, >> >> On 08/27/2014 01:14 PM, Doug Anderson wrote: >>> Jaehoon, >>> >>> On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>> Hi, Doug, >>>> >>>> On 08/26/2014 12:25 AM, Doug Anderson wrote: >>>>> Jaehoon, >>>>> >>>>> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>>>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>>>>> detection will break in these boards. >>>>>> >>>>>> I didn't know that use CD# line for card detect. >>>>>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>>>>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>>>>> >>>>>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>>>> >>>>> I'm not sure I totally understood what you said. In my manual I have >>>>> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >>>>> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >>>>> you'll see the power domain. For both it shows VDDQ_MMC2. If that >>>>> doesn't mean that the two are in the same voltage domain then I don't >>>>> know what does. Can you point to any examples where they have >>>>> different voltage domains? >>>> I think you're mis-understanding for it. >>>> Right, It's described at exynos5420, but it's not connected. >>> >>> "It's not connected". What do you mean? If I were to guess I'd say >>> that on some particular board you're looking at they don't happen to >>> use the "CD" pin for card detect. If this is what you mean, it >>> doesn't help me. exynos5420-peach-pit does use the CD pin for card >>> detect. You can look at the DTS file and confirm it. >> >> I didn't know how exynos5420-peach-pit's circuit is configured. >> But i guess that almost all exynos5 boards are configured with the similar circuit. >> >> At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. >> (CD-pin is external card-detect pin. - like XEINT_# pin) >> You mentioned CD# and DATA# lines is used the same power domain, right? >> In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. >> Not use the CD# pin, used the XEINT_# pin. >> So i think we don't need to consider the CD#. >> If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. > > Maybe on your board you have CD connected to a "gpx" line. ...but not > mine. The guys who designed our hardware followed the SMDK5420 > reference schematics which connect the SD card slot card detect to > "gpc2_2", which is the card detect pin. > > See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting > the lack of a GPIO card detect and the inclusion of "sd2_cd" > > mmc@12220000 { > status = "okay"; > card-detect-delay = <200>; > samsung,dw-mshc-ciu-div = <3>; > samsung,dw-mshc-sdr-timing = <2 3>; > samsung,dw-mshc-ddr-timing = <1 2>; > pinctrl-names = "default"; > pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; > bus-width = <4>; > cap-sd-highspeed; > }; > > See "arch/arm/boot/dts/exynos5420-peach-pit.dts" too: > > &mmc_2 { > status = "okay"; > num-slots = <1>; > cap-sd-highspeed; > card-detect-delay = <200>; > clock-frequency = <400000000>; > samsung,dw-mshc-ciu-div = <3>; > samsung,dw-mshc-sdr-timing = <2 3>; > samsung,dw-mshc-ddr-timing = <1 2>; > pinctrl-names = "default"; > pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; > bus-width = <4>; > }; > > > Here, see this ASCII art that shows how some lines are hooked up on > peach-pit. You might need to paste this into something with a > fixed-width font. > > +-------------------- > | Exynos 5420 > | > | > P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7) > | | > | | > +-+- 10K res -+----|- XMMC2CDN (AK6) > | | | > | | | > | | | > | Ext CD | > | | > +-- 10K res-+--+---|- XMMC2CMD (AK8) > | > | > Ext CMD > > You can see from the above that the external card detect signal (that > goes to the connector) named "Ext CD" is pulled up to the same voltage > as the external CMD signal (that also goes to the connector). This is > vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act > as a pull down, or in the best case it will be floating. > > Said another way: we can't read card detect when vqmmc is off. If that's the case, it makes sense. But i wonder why designed like that. Anyway, then we need to consider that controls the vqmmc power for card-detection. But if polling system uses, it seems to detect the card. Polling is the method that sends the status command. At that time, we can notice whether card is insert/remove. Is it impossible? > >> Your commit message looks like all exynos5250/5420 board are used CD# line. > > The commit message should be clearer, agreed. I think I asked Yuvaraj > to make sure that the code only invoked this quirk on exynos and only > if a GPIO was not used for card detect. Yuvaraj: can you make it more > obvious that not all exynos5250/5420 boards need this, only those that > use the "official" card detect line? > > >>> ...or are you saying that the CD pin somehow changes voltage domains >>> when configured as a GPIO? I find that very hard to believe. What >>> voltage domain does it go to? If it goes to a 1.8V voltage domain >>> then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V >>> voltage domain then that would be bad when vqmmc was 1.8V. Remember >>> that externally we've got a pull up to vqmmc. >> >> It is used with XEINT_# pin instead of CD# pin. >> As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin, >> my point is meaningless. :) >> >> Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin? > > Yes. It is using CD#. Do you remove your objections to this patch, > then (once the commit message is clearer)? Sure. And I will also effort to find it, if we can find the more generic approach. Best Regards, Jaehoon Chung > > -Doug > -- > 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
Ulf, On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 27 August 2014 17:52, Doug Anderson <dianders@google.com> wrote: >> Ulf, >> >> On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >>>> called to check the card detect line, but with vmmc and vqmmc off. It >>>> will be unable to return a sensible value without actually turning on >>>> vmmc and vqmmc. >>> >>> Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule >>> itself with HZ interval. This to check for card insert/removal. >>> >>> Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback >>> to check whether it's worth to continue initialization sequence or if >>> it should re-schedule itself again. >>> >>> If your host driver can distinguish whether a card is inserted, which >>> in this case are when vccq is turned off, your ->get_cd() callback need >>> to return 1. That will allow mmc_rescan() to continue it's >>> initialization sequence and do mmc_power_up(). >> >> As per my other email, we can't tell whether a card is inserted when >> vqmmc is off. > > I understand. > > The solution I proposed above, is: > > 1) Enable MMC_CAP_NEEDS_POLL. > 2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted. > > That will solve this issue and without messing up the mmc core. Ah, I see! So every 1 second, we'll do the following: 1. Call get_cd(), which returns 1. 2. MMC core will power everything up (turn on all regulators) for MMC. 3. We'll start to initialize a card. 4. We'll notice that, oops, there's no card there. 5. MMC core will shut things down again. That seems awfully expensive to do every second when the card detect line actually does work (as long as you keep power lines). Is the patch to separate out the concepts of "power off because no card is inserted" and "power off because we're power cycling the card" really bad enough to warrant forcing us to use the above? I'm not an EE, but cycling regulators on and off every second doesn't seem super ideal, but maybe they're designed for it? Personally, I'd be tempted to just leave the power on all the time and if a card somehow needs to be power cycled (because UHS negotiation failed?) then that card just isn't supported. This could be done in the dts by saying that the regulator is "always on" and no need to pollute any source files. -Doug -- 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
Jaehoon, On Thu, Aug 28, 2014 at 1:43 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > On 08/28/2014 12:49 AM, Doug Anderson wrote: >> Jaehoon, >> >> On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> Doug, >>> >>> On 08/27/2014 01:14 PM, Doug Anderson wrote: >>>> Jaehoon, >>>> >>>> On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>> Hi, Doug, >>>>> >>>>> On 08/26/2014 12:25 AM, Doug Anderson wrote: >>>>>> Jaehoon, >>>>>> >>>>>> On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>>>> On 08/25/2014 05:13 PM, Ulf Hansson wrote: >>>>>>>> On 22 August 2014 20:27, Sonny Rao <sonnyrao@chromium.org> wrote: >>>>>>>>> On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>>>>>> On 22 August 2014 15:47, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: >>>>>>>>>>> Exynos 5250 and 5420 based boards uses built-in CD# line for card >>>>>>>>>>> detection.But unfortunately CD# line is on the same voltage rails >>>>>>>>>>> as of I/O voltage rails. When we cut off vqmmc,the consequent card >>>>>>>>>>> detection will break in these boards. >>>>>>> >>>>>>> I didn't know that use CD# line for card detect. >>>>>>> And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. >>>>>>> Which card is used with same voltages? (eMMC? SD? SDIO?) >>>>>>> >>>>>>> Well, I have checked Exynos5250 and 5420, but it looks like not same rails. >>>>>> >>>>>> I'm not sure I totally understood what you said. In my manual I have >>>>>> a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table >>>>>> for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and >>>>>> you'll see the power domain. For both it shows VDDQ_MMC2. If that >>>>>> doesn't mean that the two are in the same voltage domain then I don't >>>>>> know what does. Can you point to any examples where they have >>>>>> different voltage domains? >>>>> I think you're mis-understanding for it. >>>>> Right, It's described at exynos5420, but it's not connected. >>>> >>>> "It's not connected". What do you mean? If I were to guess I'd say >>>> that on some particular board you're looking at they don't happen to >>>> use the "CD" pin for card detect. If this is what you mean, it >>>> doesn't help me. exynos5420-peach-pit does use the CD pin for card >>>> detect. You can look at the DTS file and confirm it. >>> >>> I didn't know how exynos5420-peach-pit's circuit is configured. >>> But i guess that almost all exynos5 boards are configured with the similar circuit. >>> >>> At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. >>> (CD-pin is external card-detect pin. - like XEINT_# pin) >>> You mentioned CD# and DATA# lines is used the same power domain, right? >>> In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. >>> Not use the CD# pin, used the XEINT_# pin. >>> So i think we don't need to consider the CD#. >>> If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. >> >> Maybe on your board you have CD connected to a "gpx" line. ...but not >> mine. The guys who designed our hardware followed the SMDK5420 >> reference schematics which connect the SD card slot card detect to >> "gpc2_2", which is the card detect pin. >> >> See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting >> the lack of a GPIO card detect and the inclusion of "sd2_cd" >> >> mmc@12220000 { >> status = "okay"; >> card-detect-delay = <200>; >> samsung,dw-mshc-ciu-div = <3>; >> samsung,dw-mshc-sdr-timing = <2 3>; >> samsung,dw-mshc-ddr-timing = <1 2>; >> pinctrl-names = "default"; >> pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; >> bus-width = <4>; >> cap-sd-highspeed; >> }; >> >> See "arch/arm/boot/dts/exynos5420-peach-pit.dts" too: >> >> &mmc_2 { >> status = "okay"; >> num-slots = <1>; >> cap-sd-highspeed; >> card-detect-delay = <200>; >> clock-frequency = <400000000>; >> samsung,dw-mshc-ciu-div = <3>; >> samsung,dw-mshc-sdr-timing = <2 3>; >> samsung,dw-mshc-ddr-timing = <1 2>; >> pinctrl-names = "default"; >> pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; >> bus-width = <4>; >> }; >> >> >> Here, see this ASCII art that shows how some lines are hooked up on >> peach-pit. You might need to paste this into something with a >> fixed-width font. >> >> +-------------------- >> | Exynos 5420 >> | >> | >> P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7) >> | | >> | | >> +-+- 10K res -+----|- XMMC2CDN (AK6) >> | | | >> | | | >> | | | >> | Ext CD | >> | | >> +-- 10K res-+--+---|- XMMC2CMD (AK8) >> | >> | >> Ext CMD >> >> You can see from the above that the external card detect signal (that >> goes to the connector) named "Ext CD" is pulled up to the same voltage >> as the external CMD signal (that also goes to the connector). This is >> vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act >> as a pull down, or in the best case it will be floating. >> >> Said another way: we can't read card detect when vqmmc is off. > > If that's the case, it makes sense. But i wonder why designed like that. > Anyway, then we need to consider that controls the vqmmc power for card-detection. Maybe you can send a message to the SoC designers at Samsung not to design the chip incorrectly in the future? -- 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
On Thu, Aug 28, 2014 at 8:50 AM, Doug Anderson <dianders@google.com> wrote: > Ulf, > > On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 27 August 2014 17:52, Doug Anderson <dianders@google.com> wrote: >>> Ulf, >>> >>> On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >>>>> called to check the card detect line, but with vmmc and vqmmc off. It >>>>> will be unable to return a sensible value without actually turning on >>>>> vmmc and vqmmc. >>>> >>>> Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule >>>> itself with HZ interval. This to check for card insert/removal. >>>> >>>> Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback >>>> to check whether it's worth to continue initialization sequence or if >>>> it should re-schedule itself again. >>>> >>>> If your host driver can distinguish whether a card is inserted, which >>>> in this case are when vccq is turned off, your ->get_cd() callback need >>>> to return 1. That will allow mmc_rescan() to continue it's >>>> initialization sequence and do mmc_power_up(). >>> >>> As per my other email, we can't tell whether a card is inserted when >>> vqmmc is off. >> >> I understand. >> >> The solution I proposed above, is: >> >> 1) Enable MMC_CAP_NEEDS_POLL. >> 2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted. >> >> That will solve this issue and without messing up the mmc core. > > Ah, I see! So every 1 second, we'll do the following: > > 1. Call get_cd(), which returns 1. > > 2. MMC core will power everything up (turn on all regulators) for MMC. > > 3. We'll start to initialize a card. > > 4. We'll notice that, oops, there's no card there. > > 5. MMC core will shut things down again. > > > That seems awfully expensive to do every second when the card detect > line actually does work (as long as you keep power lines). Is the > patch to separate out the concepts of "power off because no card is > inserted" and "power off because we're power cycling the card" really > bad enough to warrant forcing us to use the above? > > I'm not an EE, but cycling regulators on and off every second doesn't > seem super ideal, but maybe they're designed for it? > > > Personally, I'd be tempted to just leave the power on all the time and > if a card somehow needs to be power cycled (because UHS negotiation > failed?) then that card just isn't supported. This could be done in > the dts by saying that the regulator is "always on" and no need to > pollute any source files. Yeah, power cycling the regulators constantly doesn't seem like a great idea, we can ask the EEs what they think. This scheme of leaving them on all the time would prevent us from being able to actually power cycle the card, which I think is required by the spec in case UHS negotiation fails. > > > -Doug -- 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, On Thu, Aug 28, 2014 at 10:50 AM, Sonny Rao <sonnyrao@chromium.org> wrote: > On Thu, Aug 28, 2014 at 8:50 AM, Doug Anderson <dianders@google.com> wrote: >> Ulf, >> >> On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 27 August 2014 17:52, Doug Anderson <dianders@google.com> wrote: >>>> Ulf, >>>> >>>> On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>>>> Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be >>>>>> called to check the card detect line, but with vmmc and vqmmc off. It >>>>>> will be unable to return a sensible value without actually turning on >>>>>> vmmc and vqmmc. >>>>> >>>>> Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule >>>>> itself with HZ interval. This to check for card insert/removal. >>>>> >>>>> Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback >>>>> to check whether it's worth to continue initialization sequence or if >>>>> it should re-schedule itself again. >>>>> >>>>> If your host driver can distinguish whether a card is inserted, which >>>>> in this case are when vccq is turned off, your ->get_cd() callback need >>>>> to return 1. That will allow mmc_rescan() to continue it's >>>>> initialization sequence and do mmc_power_up(). >>>> >>>> As per my other email, we can't tell whether a card is inserted when >>>> vqmmc is off. >>> >>> I understand. >>> >>> The solution I proposed above, is: >>> >>> 1) Enable MMC_CAP_NEEDS_POLL. >>> 2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted. >>> >>> That will solve this issue and without messing up the mmc core. >> >> Ah, I see! So every 1 second, we'll do the following: >> >> 1. Call get_cd(), which returns 1. >> >> 2. MMC core will power everything up (turn on all regulators) for MMC. >> >> 3. We'll start to initialize a card. >> >> 4. We'll notice that, oops, there's no card there. >> >> 5. MMC core will shut things down again. >> >> >> That seems awfully expensive to do every second when the card detect >> line actually does work (as long as you keep power lines). Is the >> patch to separate out the concepts of "power off because no card is >> inserted" and "power off because we're power cycling the card" really >> bad enough to warrant forcing us to use the above? >> >> I'm not an EE, but cycling regulators on and off every second doesn't >> seem super ideal, but maybe they're designed for it? >> >> >> Personally, I'd be tempted to just leave the power on all the time and >> if a card somehow needs to be power cycled (because UHS negotiation >> failed?) then that card just isn't supported. This could be done in >> the dts by saying that the regulator is "always on" and no need to >> pollute any source files. > > Yeah, power cycling the regulators constantly doesn't seem like a > great idea, we can ask the EEs what they think. > > This scheme of leaving them on all the time would prevent us from > being able to actually power cycle the card, which I think is required > by the spec in case UHS negotiation fails. OK, fair enough. I guess polling is less bad than nor supporting card power cycling. ...it still feels like adding this quirk to the core isn't super ugly, but if everyone is against it... -Doug -- 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/core.c b/drivers/mmc/core/core.c index 68f5f4b..79ced36 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) mmc_host_clk_release(host); } -void mmc_power_off(struct mmc_host *host) +void _mmc_power_off(struct mmc_host *host, unsigned char power_mode) { - if (host->ios.power_mode == MMC_POWER_OFF) + if (host->ios.power_mode == power_mode) return; mmc_host_clk_hold(host); @@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host) host->ios.chip_select = MMC_CS_DONTCARE; } host->ios.power_mode = MMC_POWER_OFF; + host->ios.power_mode = power_mode; host->ios.bus_width = MMC_BUS_WIDTH_1; host->ios.timing = MMC_TIMING_LEGACY; mmc_set_ios(host); @@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_off(struct mmc_host *host) +{ + _mmc_power_off(host, MMC_POWER_OFF); +} + void mmc_power_cycle(struct mmc_host *host, u32 ocr) { mmc_power_off(host); + /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */ + if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER) + _mmc_power_off(host, MMC_POWER_OFF_HARD); + else + _mmc_power_off(host, MMC_POWER_OFF); + /* Wait at least 1 ms according to SD spec */ mmc_delay(1); mmc_power_up(host, ocr); diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 91eb162..3d9c5a3 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data) case MMC_POWER_ON: str = "on"; break; + case MMC_POWER_OFF_HARD: + str = "hard off"; + break; default: str = "invalid"; break; diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c index 0fbc53a..4e26049 100644 --- a/drivers/mmc/host/dw_mmc-exynos.c +++ b/drivers/mmc/host/dw_mmc-exynos.c @@ -17,6 +17,7 @@ #include <linux/mmc/mmc.h> #include <linux/of.h> #include <linux/of_gpio.h> +#include <linux/mmc/slot-gpio.h> #include <linux/slab.h> #include "dw_mmc.h" @@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios) } } +static void dw_mci_exynos_post_init(struct dw_mci_slot *slot) +{ + struct dw_mci_board *brd = slot->host->pdata; + struct mmc_host *mmc = slot->mmc; + + if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) + mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER; +} + static int dw_mci_exynos_parse_dt(struct dw_mci *host) { struct dw_mci_exynos_priv_data *priv; @@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = { .prepare_command = dw_mci_exynos_prepare_command, .set_ios = dw_mci_exynos_set_ios, .parse_dt = dw_mci_exynos_parse_dt, + .post_init = dw_mci_exynos_post_init, .execute_tuning = dw_mci_exynos_execute_tuning, }; diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index f20b4b8..6f2c681 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) spin_unlock_bh(&host->lock); } +/* + * some of the boards use controller CD line for card detection.Unfortunately + * CD line is bind to the same volatge domain as of the IO lines.If we turn off + * IO voltage domain, CD line wont work. + * Return true when controller CD line is used for card detection or return + * false. + */ +static bool dw_mci_builtin_cd(struct dw_mci_slot *slot) +{ + struct dw_mci_board *brd = slot->host->pdata; + struct mmc_host *mmc = slot->mmc; + + return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) && + IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0; +} + static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) { struct dw_mci_slot *slot = mmc_priv(mmc); @@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) mci_writel(slot->host, PWREN, regs); break; case MMC_POWER_OFF: + if (dw_mci_builtin_cd(slot) && + !test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) + return; + if (!IS_ERR(mmc->supply.vmmc)) mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0); @@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) regs &= ~(1 << slot->id); mci_writel(slot->host, PWREN, regs); break; + case MMC_POWER_OFF_HARD: + break; default: break; } @@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) else clear_bit(DW_MMC_CARD_PRESENT, &slot->flags); + if (drv_data && drv_data->post_init) + drv_data->post_init(slot); + ret = mmc_add_host(mmc); if (ret) goto err_setup_bus; diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h index 01b99e8..a3c2628 100644 --- a/drivers/mmc/host/dw_mmc.h +++ b/drivers/mmc/host/dw_mmc.h @@ -250,6 +250,7 @@ struct dw_mci_tuning_data { * @prepare_command: handle CMD register extensions. * @set_ios: handle bus specific extensions. * @parse_dt: parse implementation specific device tree properties. + * @post_init: implementation specific post initialization. * @execute_tuning: implementation specific tuning procedure. * * Provide controller implementation specific extensions. The usage of this @@ -263,6 +264,7 @@ struct dw_mci_drv_data { void (*prepare_command)(struct dw_mci *host, u32 *cmdr); void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios); int (*parse_dt)(struct dw_mci *host); + void (*post_init)(struct dw_mci_slot *slot); int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode, struct dw_mci_tuning_data *tuning_data); }; diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 4cbf614..5eb24ff 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -42,6 +42,7 @@ struct mmc_ios { #define MMC_POWER_OFF 0 #define MMC_POWER_UP 1 #define MMC_POWER_ON 2 +#define MMC_POWER_OFF_HARD 3 unsigned char bus_width; /* data bus width */ @@ -283,6 +284,7 @@ struct mmc_host { #define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \ MMC_CAP2_HS400_1_2V) #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17) +#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */ mmc_pm_flag_t pm_caps; /* supported pm features */