| Message ID | 20180105203743.35201-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5 January 2018 at 21:37, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Intel Edison the Broadcom WiFi card, which is connected to SDIO, > requires 2.0v, while the host, according to Intel Merrifield TRM, > supports 1.8v I/O only. > > The card announces itself as > > mmc2: new ultra high speed DDR50 SDIO card at address 0001 > > Introduce a custom OCR mask and ->set_power() callback to override 2.0v > signaling on Intel Merrifield platforms by enforcing 1.8v power choice. This seems to be about VDD (vmmc) rather than about signaling voltage? Could you verify that is the case? I think we have had too many cases historically, which mixing the two voltages together and thus we end up by luck getting things to work. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > - quirk free approach > drivers/mmc/host/sdhci-pci-core.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > index 3e4f04fd5175..029fcb19eb91 100644 > --- a/drivers/mmc/host/sdhci-pci-core.c > +++ b/drivers/mmc/host/sdhci-pci-core.c > @@ -778,6 +778,7 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) > slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > break; > case INTEL_MRFLD_SDIO: > + slot->host->ocr_mask = MMC_VDD_20_21 | MMC_VDD_165_195; So this is about VDD and what VDD voltage levels the host support. If there a way to find out these values dynamically, that should be done instead. > slot->host->mmc->caps |= MMC_CAP_NONREMOVABLE | > MMC_CAP_POWER_OFF_CARD; > break; > @@ -789,10 +790,37 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) > return 0; > } > > +static void intel_mrfld_sdhci_set_power(struct sdhci_host *host, > + unsigned char mode, unsigned short vdd) > +{ > + if (IS_ERR(host->mmc->supply.vmmc)) { > + switch (1 << vdd) { > + case MMC_VDD_20_21: > + sdhci_set_power_noreg(host, mode, ilog2(MMC_VDD_165_195)); This looks weird. I don't think you should need to treat the MMC_VDD_20_21 in a special way. Or perhaps this is because this is about signal voltage after all? > + break; > + default: > + sdhci_set_power_noreg(host, mode, vdd); > + break; > + } > + } else { > + sdhci_set_power(host, mode, vdd); > + } > +} > + > +static const struct sdhci_ops intel_mrfld_sdhci_pci_ops = { > + .set_clock = sdhci_set_clock, > + .set_power = intel_mrfld_sdhci_set_power, > + .enable_dma = sdhci_pci_enable_dma, > + .set_bus_width = sdhci_set_bus_width, > + .reset = sdhci_reset, > + .set_uhs_signaling = sdhci_set_uhs_signaling, > +}; > + > static const struct sdhci_pci_fixes sdhci_intel_mrfld_mmc = { > .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, > .quirks2 = SDHCI_QUIRK2_BROKEN_HS200 | > SDHCI_QUIRK2_PRESET_VALUE_BROKEN, > + .ops = &intel_mrfld_sdhci_pci_ops, > .allow_runtime_pm = true, > .probe_slot = intel_mrfld_mmc_probe_slot, > }; > -- > 2.15.1 > 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 Sat, 2018-01-06 at 13:44 +0100, Ulf Hansson wrote: > On 5 January 2018 at 21:37, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Intel Edison the Broadcom WiFi card, which is connected to SDIO, > > requires 2.0v, while the host, according to Intel Merrifield TRM, > > supports 1.8v I/O only. > > > > The card announces itself as > > > > mmc2: new ultra high speed DDR50 SDIO card at address 0001 > > > > Introduce a custom OCR mask and ->set_power() callback to override > > 2.0v > > signaling on Intel Merrifield platforms by enforcing 1.8v power > > choice. > > This seems to be about VDD (vmmc) rather than about signaling voltage? I'm not sure the case is the following: 1) SDHCI host reports (via OCR) that supported voltage is 1.8v (only) 2) BCM Wi-Fi card tells to the driver that it supports 2.0v (only) In the result there is no OCR which has at least one supported voltage by both. Keep in mind that there is no regulator case. > > Could you verify that is the case? I think we have had too many cases > historically, which mixing the two voltages together and thus we end > up by luck getting things to work. If you tell me what to add to debug print this or alike, I will provide you the answer. > > case INTEL_MRFLD_SDIO: > > + slot->host->ocr_mask = MMC_VDD_20_21 | > > MMC_VDD_165_195; > > So this is about VDD and what VDD voltage levels the host support. If > there a way to find out these values dynamically, that should be done > instead. Then we can't find a compatible voltage. > > + if (IS_ERR(host->mmc->supply.vmmc)) { > > + switch (1 << vdd) { > > + case MMC_VDD_20_21: > > + sdhci_set_power_noreg(host, mode, > > ilog2(MMC_VDD_165_195)); > > This looks weird. Yes, it does. I have no idea how to do this better. I can't fix neither hardware nor firmware to do something about this. > I don't think you should need to treat the MMC_VDD_20_21 in a special > way. > > Or perhaps this is because this is about signal voltage after all? See above.
On 05/01/18 22:37, Andy Shevchenko wrote: > On Intel Edison the Broadcom WiFi card, which is connected to SDIO, > requires 2.0v, while the host, according to Intel Merrifield TRM, > supports 1.8v I/O only. I/O -> supply > > The card announces itself as > > mmc2: new ultra high speed DDR50 SDIO card at address 0001 > > Introduce a custom OCR mask and ->set_power() callback to override 2.0v > signaling on Intel Merrifield platforms by enforcing 1.8v power choice. signaling -> supply > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > - quirk free approach > drivers/mmc/host/sdhci-pci-core.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > index 3e4f04fd5175..029fcb19eb91 100644 > --- a/drivers/mmc/host/sdhci-pci-core.c > +++ b/drivers/mmc/host/sdhci-pci-core.c > @@ -778,6 +778,7 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) > slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > break; > case INTEL_MRFLD_SDIO: Maybe add a comment here: /* Advertise 2.0v for compatibility with the SDIO card's OCR */ > + slot->host->ocr_mask = MMC_VDD_20_21 | MMC_VDD_165_195; > slot->host->mmc->caps |= MMC_CAP_NONREMOVABLE | > MMC_CAP_POWER_OFF_CARD; > break; > @@ -789,10 +790,37 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) > return 0; > } > > +static void intel_mrfld_sdhci_set_power(struct sdhci_host *host, > + unsigned char mode, unsigned short vdd) > +{ > + if (IS_ERR(host->mmc->supply.vmmc)) { > + switch (1 << vdd) { Maybe add a comment: /* * Without a regulator, SDHCI does not support 2.0v but we get here * because we advertised 2.0v support for compatibility with the SDIO * card's OCR. Map it to 1.8v for the purpose of turning on the power. */ > + case MMC_VDD_20_21: > + sdhci_set_power_noreg(host, mode, ilog2(MMC_VDD_165_195)); > + break; > + default: > + sdhci_set_power_noreg(host, mode, vdd); > + break; > + } > + } else { > + sdhci_set_power(host, mode, vdd); > + } How about this instead: if (IS_ERR(host->mmc->supply.vmmc) && vdd == ilog2(MMC_VDD_20_21)) { sdhci_set_power_noreg(host, mode, ilog2(MMC_VDD_165_195)); return; } sdhci_set_power(host, mode, vdd); > +} > + > +static const struct sdhci_ops intel_mrfld_sdhci_pci_ops = { > + .set_clock = sdhci_set_clock, > + .set_power = intel_mrfld_sdhci_set_power, > + .enable_dma = sdhci_pci_enable_dma, > + .set_bus_width = sdhci_set_bus_width, > + .reset = sdhci_reset, > + .set_uhs_signaling = sdhci_set_uhs_signaling, > +}; > + > static const struct sdhci_pci_fixes sdhci_intel_mrfld_mmc = { > .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, > .quirks2 = SDHCI_QUIRK2_BROKEN_HS200 | > SDHCI_QUIRK2_PRESET_VALUE_BROKEN, > + .ops = &intel_mrfld_sdhci_pci_ops, > .allow_runtime_pm = true, > .probe_slot = intel_mrfld_mmc_probe_slot, > }; > -- 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/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c index 3e4f04fd5175..029fcb19eb91 100644 --- a/drivers/mmc/host/sdhci-pci-core.c +++ b/drivers/mmc/host/sdhci-pci-core.c @@ -778,6 +778,7 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; break; case INTEL_MRFLD_SDIO: + slot->host->ocr_mask = MMC_VDD_20_21 | MMC_VDD_165_195; slot->host->mmc->caps |= MMC_CAP_NONREMOVABLE | MMC_CAP_POWER_OFF_CARD; break; @@ -789,10 +790,37 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot) return 0; } +static void intel_mrfld_sdhci_set_power(struct sdhci_host *host, + unsigned char mode, unsigned short vdd) +{ + if (IS_ERR(host->mmc->supply.vmmc)) { + switch (1 << vdd) { + case MMC_VDD_20_21: + sdhci_set_power_noreg(host, mode, ilog2(MMC_VDD_165_195)); + break; + default: + sdhci_set_power_noreg(host, mode, vdd); + break; + } + } else { + sdhci_set_power(host, mode, vdd); + } +} + +static const struct sdhci_ops intel_mrfld_sdhci_pci_ops = { + .set_clock = sdhci_set_clock, + .set_power = intel_mrfld_sdhci_set_power, + .enable_dma = sdhci_pci_enable_dma, + .set_bus_width = sdhci_set_bus_width, + .reset = sdhci_reset, + .set_uhs_signaling = sdhci_set_uhs_signaling, +}; + static const struct sdhci_pci_fixes sdhci_intel_mrfld_mmc = { .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, .quirks2 = SDHCI_QUIRK2_BROKEN_HS200 | SDHCI_QUIRK2_PRESET_VALUE_BROKEN, + .ops = &intel_mrfld_sdhci_pci_ops, .allow_runtime_pm = true, .probe_slot = intel_mrfld_mmc_probe_slot, };
On Intel Edison the Broadcom WiFi card, which is connected to SDIO, requires 2.0v, while the host, according to Intel Merrifield TRM, supports 1.8v I/O only. The card announces itself as mmc2: new ultra high speed DDR50 SDIO card at address 0001 Introduce a custom OCR mask and ->set_power() callback to override 2.0v signaling on Intel Merrifield platforms by enforcing 1.8v power choice. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- - quirk free approach drivers/mmc/host/sdhci-pci-core.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)