[PATCH] mmc: msm_sdhci: enable vqmmc at probe if available

[Caleb Connolly]

On 24/10/2024 16:16, Neil Armstrong wrote:
> On 24/10/2024 15:11, Caleb Connolly wrote:
>> Hi Neil,
>>
>> On 16/10/2024 11:17, Neil Armstrong wrote:
>>> On earlier platforms, the vqmmc regulator was enabled by the
>>> previous bootloader, but on the newest (SM8650) it's not
>>> and we need vqmmc to be enabled in order to have the card
>>> to respond.
>>
>> Isn't/shouldn't this be handled in mmc_power_init() ?
> 
> So, yes and no, the device_get_supply_regulator(vqmmc-supply) in
> mmc_power_init is
> done to be used by sdhci_set_voltage() but .... but ... Qualcomm doesn't
> use
> MMC_SIGNAL_VOLTAGE_330 but MMC_SIGNAL_VOLTAGE_300 which is implemented
> in Linux
> but not in Uboot, so it would require a significant mmc/sdhci driver update
> but since we 're not even far from supporting any card state requiring
> MMC_SIGNAL_VOLTAGE_180 we don't even declare the sdhci_ops and neither
> any of the config_dll/set_enhanced_strobe/execute_tuning so in our current
> case we just need to enable vqmmc.

Ahhh, welp this presumably explains a bunch of my sdcard woes ;( I
thought the generic code was handling this (and clearly did a bad job at
checking the regulators themselves)

Reviewed-by: Caleb Connolly <caleb.connolly at linaro.org>

Kind regards,
> 
> Neil
>>
>> Kind regards,
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
>>> ---
>>>   drivers/mmc/msm_sdhci.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c
>>> index 4e5c932c071..27bb7052fca 100644
>>> --- a/drivers/mmc/msm_sdhci.c
>>> +++ b/drivers/mmc/msm_sdhci.c
>>> @@ -15,6 +15,7 @@
>>>   #include <asm/global_data.h>
>>>   #include <asm/io.h>
>>>   #include <linux/bitops.h>
>>> +#include <power/regulator.h>
>>>   /* Non-standard registers needed for SDHCI startup */
>>>   #define SDCC_MCI_POWER   0x0
>>> @@ -43,6 +44,7 @@ struct msm_sdhc {
>>>       struct sdhci_host host;
>>>       void *base;
>>>       struct clk_bulk clks;
>>> +    struct udevice *vqmmc;
>>>   };
>>>   struct msm_sdhc_variant_info {
>>> @@ -163,6 +165,16 @@ static int msm_sdc_probe(struct udevice *dev)
>>>       if (ret)
>>>           return ret;
>>> +    /* Get the vqmmc regulator and enable it if available */
>>> +    device_get_supply_regulator(dev, "vqmmc-supply", &prv->vqmmc);
>>> +    if (prv->vqmmc) {
>>> +        ret = regulator_set_enable_if_allowed(prv->vqmmc, true);
>>> +        if (ret) {
>>> +            printf("Failed to enable the VQMMC regulator\n");
>>> +            return ret;
>>> +        }
>>> +    }
>>> +
>>>       var_info = (void *)dev_get_driver_data(dev);
>>>       if (!var_info->mci_removed) {
>>>           ret = msm_sdc_mci_init(prv);
>>>
>>> ---
>>> base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae
>>> change-id: 20241016-topic-sm8x50-mmc-vqmmc-b7cf8176ec51
>>>
>>> Best regards,
>>
> 

-- 
// Caleb (they/them)