On 4/15/20 10:40 AM, Ulf Hansson wrote:
> On Wed, 1 Apr 2020 at 21:57, Marek Vasut <marex@xxxxxxx> wrote:
>>
>> Patch all drivers and core code which uses mmc_set_signal_voltage()
>> and prepare it for the fact that mmc_set_signal_voltage() can return
>> a value > 0, which would happen if the signal voltage switch did NOT
>> happen, because the voltage was already set correctly.
>
> I am not sure why you want to change mmc_set_signal_voltage(), can you
> elaborate on that?
>
> I thought we discussed changing mmc_regulator_set_vqmmc(), what am I missing?
Because mmc_set_signal_voltage() optionally calls
host->ops_>start_signal_voltage_switch() , which can now return value >
0 , so the rest of the core needs to be patched to cater for that.
[...]
>> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
>> index 23b6f65b3785..50977ff18074 100644
>> --- a/drivers/mmc/host/dw_mmc-k3.c
>> +++ b/drivers/mmc/host/dw_mmc-k3.c
>> @@ -424,7 +424,7 @@ static int dw_mci_hi3660_switch_voltage(struct mmc_host *mmc,
>>
>> if (!IS_ERR(mmc->supply.vqmmc)) {
>> ret = mmc_regulator_set_vqmmc(mmc, ios);
>> - if (ret) {
>> + if (ret < 0) {
>
> This change makes sense to me, however it's also a bit confusing, as
> the changelog refers to changes for mmc_set_signal_voltage().
>
> As I understand it, we want mmc_regulator_set_vqmmc() to return 1, in
> case the current voltage level is the same as the requested "new"
> target".
Yes. So a failure is now only a return value < 0.
>> dev_err(host->dev, "Regulator set error %d\n", ret);
>> return ret;
>> }
>
> [...]
>
> So, to conclude, it seems like $subject patch needs to be reworked a
> bit - just keep the changes you have made to the host drivers, then
> throw away the other parts in core.
I think the core parts are necessary as well though , see above.
