On May 17, 2011, at 10:49 PM, Nath, Arindam wrote: > Hi Philip, > > >> -----Original Message----- >> From: Philip Rakity [mailto:prakity@xxxxxxxxxxx] >> Sent: Wednesday, May 18, 2011 2:29 AM >> To: linux-mmc@xxxxxxxxxxxxxxx >> Cc: Nath, Arindam >> Subject: [RFC] mmc: Non Default UHS Drive Strength must use board >> specific code >> >> >> Note: This is being send out for comment. We are still in the process >> of testing this change >> but we would like to have community review at this time. >> >> >> SD 3.0 introduced additional drive strengths for UHS. >> The card and the host can indicate 4 drive strengths as a bit >> mask. Without local design knowledge of the board it is not >> possible to select the correct drive strength. Unfortunately >> there is not the equivalent of ethernet auto-negotiate in the >> SD 3.0 Physical Spec at this time. >> >> We punt the problem by asking any platform specific code to >> handle the drive strength problem. If non is defined we default >> the drive strength to the manadory TYPE_B value. >> >> Signed-off-by: Philip Rakity <prakity@xxxxxxxxxxx> >> --- >> drivers/mmc/core/core.c | 15 ++++++++++ >> drivers/mmc/core/core.h | 5 +++ >> drivers/mmc/core/sd.c | 65 ++++++++++++++++++++++++-------------- >> -------- >> drivers/mmc/host/sdhci.c | 16 +++++++++++ >> drivers/mmc/host/sdhci.h | 5 +++ >> include/linux/mmc/host.h | 13 ++++++--- >> 6 files changed, 84 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 68091dd..49df27b 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -964,6 +964,21 @@ int mmc_set_signal_voltage(struct mmc_host *host, >> int signal_voltage, bool cmd11 >> return err; >> } >> >> +unsigned int mmc_select_drive_strength(struct mmc_host *host, >> + unsigned int bus_mode, >> + unsigned int max_dtr, >> + unsigned int host_drv_type, >> + unsigned int card_drv_type) >> +{ >> + if (host->ops->select_drive_strength) >> + return host->ops->select_drive_strength(host, >> + bus_mode, max_dtr, >> + host_drv_type, card_drv_type); >> + >> + /* return default strength if no handler in driver */ >> + return MMC_SET_DRIVER_TYPE_B; >> +} >> + >> /* >> * Select timing parameters for host. >> */ >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index d9411ed..8bc289c 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -43,6 +43,11 @@ int mmc_set_signal_voltage(struct mmc_host *host, >> int signal_voltage, >> bool cmd11); >> void mmc_set_timing(struct mmc_host *host, unsigned int timing); >> void mmc_set_driver_type(struct mmc_host *host, unsigned int >> drv_type); >> +unsigned int mmc_select_drive_strength(struct mmc_host *host, >> + unsigned int bus_mode, >> + unsigned int max_dtr, >> + unsigned int host_drv_type, >> + unsigned int card_drv_type); >> >> static inline void mmc_delay(unsigned int ms) >> { >> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> index 596d0b9..a268ead 100644 >> --- a/drivers/mmc/core/sd.c >> +++ b/drivers/mmc/core/sd.c >> @@ -405,54 +405,57 @@ out: >> return err; >> } >> >> + >> static int sd_select_driver_type(struct mmc_card *card, u8 *status) >> { >> - int host_drv_type = 0, card_drv_type = 0; >> + unsigned int host_drv_type = MMC_SET_DRIVER_TYPE_B; >> + unsigned int card_drv_type = MMC_SET_DRIVER_TYPE_B; >> int err; >> + unsigned char drive_strength; >> >> /* >> * If the host doesn't support any of the Driver Types A,C or D, >> - * default Driver Type B is used. >> + * or there is no board specific handler then default Driver >> + * Type B is used. >> */ >> if (!(card->host->caps & (MMC_CAP_DRIVER_TYPE_A | >> MMC_CAP_DRIVER_TYPE_C >> | MMC_CAP_DRIVER_TYPE_D))) >> return 0; >> >> - if (card->host->caps & MMC_CAP_DRIVER_TYPE_A) { >> - host_drv_type = MMC_SET_DRIVER_TYPE_A; >> - if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_A) >> - card_drv_type = MMC_SET_DRIVER_TYPE_A; >> - else if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_B) >> - card_drv_type = MMC_SET_DRIVER_TYPE_B; >> - else if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_C) >> - card_drv_type = MMC_SET_DRIVER_TYPE_C; >> - } else if (card->host->caps & MMC_CAP_DRIVER_TYPE_C) { >> - host_drv_type = MMC_SET_DRIVER_TYPE_C; >> - if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_C) >> - card_drv_type = MMC_SET_DRIVER_TYPE_C; >> - } else if (!(card->host->caps & MMC_CAP_DRIVER_TYPE_D)) { >> - /* >> - * If we are here, that means only the default driver type >> - * B is supported by the host. >> - */ >> - host_drv_type = MMC_SET_DRIVER_TYPE_B; >> - if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_B) >> - card_drv_type = MMC_SET_DRIVER_TYPE_B; >> - else if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_C) >> - card_drv_type = MMC_SET_DRIVER_TYPE_C; >> - } >> + if (card->host->caps & MMC_CAP_DRIVER_TYPE_A) >> + host_drv_type |= MMC_SET_DRIVER_TYPE_A; >> + >> + if (card->host->caps & MMC_CAP_DRIVER_TYPE_C) >> + host_drv_type |= MMC_SET_DRIVER_TYPE_C; >> + >> + if (card->host->caps & MMC_CAP_DRIVER_TYPE_D) >> + host_drv_type |= MMC_SET_DRIVER_TYPE_D; >> >> - err = mmc_sd_switch(card, 1, 2, card_drv_type, status); >> + if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_A) >> + card_drv_type |= MMC_SET_DRIVER_TYPE_A; >> + >> + if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_C) >> + card_drv_type |= MMC_SET_DRIVER_TYPE_C; >> + >> + if (card->sw_caps.sd3_drv_type & SD_DRIVER_TYPE_D) >> + card_drv_type |= MMC_SET_DRIVER_TYPE_D; >> + >> + drive_strength = mmc_select_drive_strength(card->host, >> + card->sw_caps.sd3_bus_mode, >> + card->sw_caps.uhs_max_dtr, >> + host_drv_type, card_drv_type); >> + >> + err = mmc_sd_switch(card, 1, 2, drive_strength, status); >> if (err) >> return err; >> >> - if ((status[15] & 0xF) != card_drv_type) { >> - printk(KERN_WARNING "%s: Problem setting driver >> strength!\n", >> - mmc_hostname(card->host)); >> + if ((status[15] & 0xF) != drive_strength) { >> + printk(KERN_WARNING "%s: Problem setting driver strength >> %d\n", >> + mmc_hostname(card->host), drive_strength); >> return 0; >> } >> >> - mmc_set_driver_type(card->host, host_drv_type); >> + mmc_set_driver_type(card->host, drive_strength); >> >> return 0; >> } >> @@ -488,7 +491,7 @@ static int sd_set_bus_speed_mode(struct mmc_card >> *card, u8 *status) >> card->sw_caps.uhs_max_dtr = UHS_SDR50_MAX_DTR; >> } else if ((card->host->caps & (MMC_CAP_UHS_SDR104 | >> MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR25)) && >> - (card->sw_caps.sd3_bus_mode & SD_MODE_UHS_SDR25)) { >> + (card->sw_caps.uhs_max_dtr & SD_MODE_UHS_SDR25)) { > > I think this line should not be changed, otherwise it will lose its purpose. typo -- will fix > >> bus_speed = UHS_SDR25_BUS_SPEED; >> timing = MMC_TIMING_UHS_SDR25; >> card->sw_caps.uhs_max_dtr = UHS_SDR25_MAX_DTR; >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index cc63f5e..ebeb986 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -1766,6 +1766,21 @@ static void sdhci_enable_preset_value(struct >> mmc_host *mmc, bool enable) >> spin_unlock_irqrestore(&host->lock, flags); >> } >> >> +static unsigned int sdhci_select_drive_strength(struct mmc_host *host, >> + unsigned int bus_mode, >> + unsigned int max_dtr, >> + unsigned int host_drv_type, >> + unsigned int card_drv_type) >> +{ >> + if (host->ops->select_drive_strength) >> + return host->ops->select_drive_strength(host, >> + bus_mode, max_dtr, >> + host_drv_type, card_drv_type); >> + >> + /* return default strength if no handler in driver */ >> + return MMC_SET_DRIVER_TYPE_B; >> +} >> + >> static const struct mmc_host_ops sdhci_ops = { >> .request = sdhci_request, >> .set_ios = sdhci_set_ios, >> @@ -1774,6 +1789,7 @@ static const struct mmc_host_ops sdhci_ops = { >> .start_signal_voltage_switch = >> sdhci_start_signal_voltage_switch, >> .execute_tuning = sdhci_execute_tuning, >> .enable_preset_value = sdhci_enable_preset_value, >> + .select_drive_strength = sdhci_select_drive_strength, > > Do we need this here? Host Controllers who want to support this should provide their own sdhci_ops function in their respective mmc/host files. For other controllers, mmc_select_drive_strength() will always return TYPE_B as you mentioned above. > without this there is no hook into the platform specific code to handle the setting of drive strength. >> }; >> >> >> /********************************************************************** >> *******\ >> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >> index 7e28eec..8f48aca 100644 >> --- a/drivers/mmc/host/sdhci.h >> +++ b/drivers/mmc/host/sdhci.h >> @@ -271,6 +271,11 @@ struct sdhci_ops { >> void (*platform_reset_enter)(struct sdhci_host *host, u8 mask); >> void (*platform_reset_exit)(struct sdhci_host *host, u8 mask); >> int (*set_uhs_signaling)(struct sdhci_host *host, unsigned int >> uhs); >> + unsigned int (*select_drive_strength)(struct sdhci_host >> *host, >> + unsigned int bus_mode, >> + unsigned int max_dtr, >> + unsigned int host_drv_type, >> + unsigned int card_drv_type); >> >> }; >> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index de32e6a..6c2aeab 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -70,10 +70,10 @@ struct mmc_ios { >> >> unsigned char drv_type; /* driver type (A, B, C, D) >> */ >> >> -#define MMC_SET_DRIVER_TYPE_B 0 >> -#define MMC_SET_DRIVER_TYPE_A 1 >> -#define MMC_SET_DRIVER_TYPE_C 2 >> -#define MMC_SET_DRIVER_TYPE_D 3 >> +#define MMC_SET_DRIVER_TYPE_B (1<<0) >> +#define MMC_SET_DRIVER_TYPE_A (1<<1) >> +#define MMC_SET_DRIVER_TYPE_C (1<<2) >> +#define MMC_SET_DRIVER_TYPE_D (1<<3) > > These defines if changed will have a different meaning altogether. The defines actually correspond to the "Function Name" column of Table 4-11 of the Physical Layer spec v3.01, which is used by mmc_sd_switch() to set the driver type for the card. will add the comment about Function Name and rework code. > > Thanks, > Arindam > >> }; >> >> struct mmc_host_ops { >> @@ -139,6 +139,11 @@ struct mmc_host_ops { >> int (*start_signal_voltage_switch)(struct mmc_host *host, >> struct mmc_ios *ios); >> int (*execute_tuning)(struct mmc_host *host); >> void (*enable_preset_value)(struct mmc_host *host, bool enable); >> + unsigned int (*select_drive_strength)(struct mmc_host *host, >> + unsigned int bus_mode, >> + unsigned int max_dtr, >> + unsigned int host_drv_type, >> + unsigned int card_drv_type); >> }; >> >> struct mmc_card; >> -- >> 1.7.0.4 >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
