* [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface
@ 2018-01-05 20:37 Andy Shevchenko
2018-01-06 12:44 ` Ulf Hansson
2018-01-10 14:56 ` Adrian Hunter
0 siblings, 2 replies; 4+ messages in thread
From: Andy Shevchenko @ 2018-01-05 20:37 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson, linux-mmc; +Cc: Andy Shevchenko
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(+)
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,
};
--
2.15.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface
2018-01-05 20:37 [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface Andy Shevchenko
@ 2018-01-06 12:44 ` Ulf Hansson
2018-01-08 13:05 ` Andy Shevchenko
2018-01-10 14:56 ` Adrian Hunter
1 sibling, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2018-01-06 12:44 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Adrian Hunter, linux-mmc@vger.kernel.org
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface
2018-01-06 12:44 ` Ulf Hansson
@ 2018-01-08 13:05 ` Andy Shevchenko
0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2018-01-08 13:05 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc@vger.kernel.org
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.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface
2018-01-05 20:37 [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface Andy Shevchenko
2018-01-06 12:44 ` Ulf Hansson
@ 2018-01-10 14:56 ` Adrian Hunter
1 sibling, 0 replies; 4+ messages in thread
From: Adrian Hunter @ 2018-01-10 14:56 UTC (permalink / raw)
To: Andy Shevchenko, Ulf Hansson, linux-mmc
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,
> };
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-10 14:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-05 20:37 [PATCH v2] sdhci: Advertise 2.0v on SDIO host interface Andy Shevchenko
2018-01-06 12:44 ` Ulf Hansson
2018-01-08 13:05 ` Andy Shevchenko
2018-01-10 14:56 ` Adrian Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox