* [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
@ 2018-01-10 15:32 Andy Shevchenko
2018-01-10 17:01 ` Ulf Hansson
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-01-10 15:32 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 supply 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
supply on Intel Merrifield platforms by enforcing 1.8v power choice.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- address comments given by Adrian
drivers/mmc/host/sdhci-pci-core.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 3e4f04fd5175..24c2b2504b3f 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -778,6 +778,8 @@ 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:
+ /* 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 +791,35 @@ 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)
+{
+ /*
+ * 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.
+ */
+ if (IS_ERR(host->mmc->supply.vmmc) && vdd == ilog2(MMC_VDD_20_21))
+ vdd = ilog2(MMC_VDD_165_195);
+
+ 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] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-10 15:32 [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface Andy Shevchenko
@ 2018-01-10 17:01 ` Ulf Hansson
2018-01-10 18:04 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Ulf Hansson @ 2018-01-10 17:01 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Adrian Hunter, linux-mmc
On 10 January 2018 at 16:32, 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 supply 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
> supply on Intel Merrifield platforms by enforcing 1.8v power choice.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - address comments given by Adrian
> drivers/mmc/host/sdhci-pci-core.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 3e4f04fd5175..24c2b2504b3f 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -778,6 +778,8 @@ 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:
> + /* 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 +791,35 @@ 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)
> +{
> + /*
> + * 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.
> + */
> + if (IS_ERR(host->mmc->supply.vmmc) && vdd == ilog2(MMC_VDD_20_21))
> + vdd = ilog2(MMC_VDD_165_195);
Why not instead extend the range in sdhci_set_power_noreg() to also
check for MMC_VDD_20_21?
Or is there a problem with that?
> +
> + sdhci_set_power(host, mode, vdd);
> +}
> +
Kind regards
Uffe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-10 17:01 ` Ulf Hansson
@ 2018-01-10 18:04 ` Andy Shevchenko
2018-01-10 20:06 ` Ulf Hansson
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-01-10 18:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc
On Wed, 2018-01-10 at 18:01 +0100, Ulf Hansson wrote:
> On 10 January 2018 at 16:32, 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 supply only.
> > + /*
> > + * 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.
> > + */
> > + if (IS_ERR(host->mmc->supply.vmmc) && vdd ==
> > ilog2(MMC_VDD_20_21))
> > + vdd = ilog2(MMC_VDD_165_195);
>
> Why not instead extend the range in sdhci_set_power_noreg() to also
> check for MMC_VDD_20_21?
>
> Or is there a problem with that?
Do we have any grounds to do this in generic code?
Moreover, if we check for 2.0v what should we do in generic code?
For my understanding
case _20_21:
pwr = _180;
is absolutely wrong in generic code (see how it looked in v1 of this
patch).
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-10 18:04 ` Andy Shevchenko
@ 2018-01-10 20:06 ` Ulf Hansson
2018-01-10 20:16 ` Andy Shevchenko
2018-01-11 7:57 ` Adrian Hunter
0 siblings, 2 replies; 7+ messages in thread
From: Ulf Hansson @ 2018-01-10 20:06 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Adrian Hunter, linux-mmc
On 10 January 2018 at 19:04, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, 2018-01-10 at 18:01 +0100, Ulf Hansson wrote:
>> On 10 January 2018 at 16:32, 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 supply only.
>
>> > + /*
>> > + * 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.
>> > + */
>> > + if (IS_ERR(host->mmc->supply.vmmc) && vdd ==
>> > ilog2(MMC_VDD_20_21))
>> > + vdd = ilog2(MMC_VDD_165_195);
>>
>> Why not instead extend the range in sdhci_set_power_noreg() to also
>> check for MMC_VDD_20_21?
>>
>> Or is there a problem with that?
>
> Do we have any grounds to do this in generic code?
>
> Moreover, if we check for 2.0v what should we do in generic code?
> For my understanding
>
> case _20_21:
> pwr = _180;
Yeah, why is that a problem?
You should never reach this point, unless the host announce support
via the ocr mask, for the _20_21 VDD.
What does other variants do in this regards?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-10 20:06 ` Ulf Hansson
@ 2018-01-10 20:16 ` Andy Shevchenko
2018-01-11 7:57 ` Adrian Hunter
1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-01-10 20:16 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc
On Wed, 2018-01-10 at 21:06 +0100, Ulf Hansson wrote:
> On 10 January 2018 at 19:04, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, 2018-01-10 at 18:01 +0100, Ulf Hansson wrote:
> > > On 10 January 2018 at 16:32, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > Why not instead extend the range in sdhci_set_power_noreg() to
> > > also
> > > check for MMC_VDD_20_21?
> > >
> > > Or is there a problem with that?
> >
> > Do we have any grounds to do this in generic code?
> >
> > Moreover, if we check for 2.0v what should we do in generic code?
> > For my understanding
> >
> > case _20_21:
> > pwr = _180;
>
> Yeah, why is that a problem?
For me there is none. I assume that there might be hardware which we do
not possess and which would be broken by this change.
If you think this is okay, the resulting patch can be just two liner:
1) add custom ocr_mask in sdhci-pci-core;
2) add additional case in sdhci generic code.
> You should never reach this point, unless the host announce support
> via the ocr mask, for the _20_21 VDD.
>
> What does other variants do in this regards?
I'm not sure I understand what "variants" you meant. Hardware? Host
controllers? Cards?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-10 20:06 ` Ulf Hansson
2018-01-10 20:16 ` Andy Shevchenko
@ 2018-01-11 7:57 ` Adrian Hunter
2018-01-11 12:48 ` Andy Shevchenko
1 sibling, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2018-01-11 7:57 UTC (permalink / raw)
To: Ulf Hansson, Andy Shevchenko; +Cc: linux-mmc
On 10/01/18 22:06, Ulf Hansson wrote:
> On 10 January 2018 at 19:04, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> On Wed, 2018-01-10 at 18:01 +0100, Ulf Hansson wrote:
>>> On 10 January 2018 at 16:32, 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 supply only.
>>
>>>> + /*
>>>> + * 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.
>>>> + */
>>>> + if (IS_ERR(host->mmc->supply.vmmc) && vdd ==
>>>> ilog2(MMC_VDD_20_21))
>>>> + vdd = ilog2(MMC_VDD_165_195);
>>>
>>> Why not instead extend the range in sdhci_set_power_noreg() to also
>>> check for MMC_VDD_20_21?
>>>
>>> Or is there a problem with that?
>>
>> Do we have any grounds to do this in generic code?
>>
>> Moreover, if we check for 2.0v what should we do in generic code?
>> For my understanding
>>
>> case _20_21:
>> pwr = _180;
>
> Yeah, why is that a problem?
Shouldn't be a problem. Just add a comment:
/*
* Without a regulator, SDHCI does not support 2.0v so we only
* get here if the driver deliberately added the 2.0v range to
* ocr_avail. Map it to 1.8V for the purpose of turning on the
* power.
*/
case MMC_VDD_20_21:
pwr = SDHCI_POWER_180;
break;
>
> You should never reach this point, unless the host announce support
> via the ocr mask, for the _20_21 VDD.
>
> What does other variants do in this regards?
Use regulators or 3V I expect.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface
2018-01-11 7:57 ` Adrian Hunter
@ 2018-01-11 12:48 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-01-11 12:48 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson; +Cc: linux-mmc
On Thu, 2018-01-11 at 09:57 +0200, Adrian Hunter wrote:
> On 10/01/18 22:06, Ulf Hansson wrote:
> > On 10 January 2018 at 19:04, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Wed, 2018-01-10 at 18:01 +0100, Ulf Hansson wrote:
> > > > On 10 January 2018 at 16:32, 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 supply only.
> > > > > + /*
> > > > > + * 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.
> > > > > + */
> > > > > + if (IS_ERR(host->mmc->supply.vmmc) && vdd ==
> > > > > ilog2(MMC_VDD_20_21))
> > > > > + vdd = ilog2(MMC_VDD_165_195);
> > > >
> > > > Why not instead extend the range in sdhci_set_power_noreg() to
> > > > also
> > > > check for MMC_VDD_20_21?
> > > >
> > > > Or is there a problem with that?
> > >
> > > Do we have any grounds to do this in generic code?
> > >
> > > Moreover, if we check for 2.0v what should we do in generic code?
> > > For my understanding
> > >
> > > case _20_21:
> > > pwr = _180;
> >
> > Yeah, why is that a problem?
>
> Shouldn't be a problem. Just add a comment:
>
> /*
> * Without a regulator, SDHCI does not support 2.0v so
> we only
> * get here if the driver deliberately added the 2.0v
> range to
> * ocr_avail. Map it to 1.8V for the purpose of turning
> on the
> * power.
> */
> case MMC_VDD_20_21:
> pwr = SDHCI_POWER_180;
> break;
OK.
I will send v4 soon after giving some testing.
> > What does other variants do in this regards?
>
> Use regulators or 3V I expect.
Yep, correct.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-01-11 12:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 15:32 [PATCH v3] sdhci: Advertise 2.0v supply on SDIO host interface Andy Shevchenko
2018-01-10 17:01 ` Ulf Hansson
2018-01-10 18:04 ` Andy Shevchenko
2018-01-10 20:06 ` Ulf Hansson
2018-01-10 20:16 ` Andy Shevchenko
2018-01-11 7:57 ` Adrian Hunter
2018-01-11 12:48 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.