Re: [PATCH 1/2] mmc: sdhci-acpi: Disable 1.8V modes on external microSD on Lenovo Miix 320

[Hans de Goede <hdegoede@redhat.com>]

HI,

On 16-01-2020 08:59, Adrian Hunter wrote:
> On 15/01/20 5:31 pm, Hans de Goede wrote:

<snip>

>>>> Note that the suspend/resume handling is broken also in the sense that
>>>> it does not disable the signal voltage during suspend.
>>>
>>> The bus power gets switched off if the card is runtime suspended.  The host
>>> controller should go to D3cold which means everything off.
>>
>> Right, what I mean is that the _PS3 method is broken in that it does
>> not turn off the voltage-regulator providing the signal voltage, as
>> it does do on other machines with a non buggy implementation.
> 
> Is that different to what you would get with Windows?

No Windows has the same problem.

> Also, you could possibly build a custom DSDT and fix the _PS0 and _PS3
> yourself.  That requires building it into a custom kernel also though.

I have not tried, but yes that should work, but until we get some generic
mechanism (*) in Linux / distro-s to provide DSDT overrides, that is not
helpful for regular Linux users.

*) which also has copyright issues, so the chances of this ever happening
are slim

<snip>

>>>>>> +static int quirks = -1;
>>>>>> +module_param(quirks, int, 0444);
>>>>>> +MODULE_PARM_DESC(quirks, "Override sdhci-acpi specific quirks");
>>>>>
>>>>> Why is a module parameter needed?
>>>>
>>>> The module parameter is purely to make testing if the same quirk(s)
>>>> help on other devices easier. Like the debug_quirks[2] params in sdhci.c
>>>
>>> Mmm, but we already have SDHCI_QUIRK2_NO_1_8_V
>>
>> True, but this only applies to the sdcard slot and not to the eMMC,
>> also you are asking for this to be changed to:
>>
>> SDHCI_ACPI_QUIRK_SD_SET_SIGNAL_3_3V_ON_SUSPEND
>>
>> Which is not duplicate. Anyways if you dislike the module parameter
>> bits I can drop them and make this only available through the DMI quirks.
>>
> 
> It isn't dislike, it is whether it will ever be needed.

For this specific issue, chances are not that big we will need it
on another device. The quirk added by the second patch, to disable
(broken) read-only detection OTOH might very well be useful on some
other devices.

And adding the option to override the quirks from the kernel commandline
requires very little extra code.

Anyways, it is your call. Please let me know if you want to drop the
module parameter for v2, or if you are ok with keeping it.

Regards,

Hans