On 17.02.2023 11:47, Ulf Hansson wrote:
> On Wed, 15 Feb 2023 at 21:14, Heiner Kallweit <hkallweit1@xxxxxxxxx> wrote:
>>
>> A lot of drivers use this code, therefore let's factor it out to
>> helpers.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> include/linux/mmc/host.h | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 812e6b583..f93fb8c7d 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -597,6 +597,23 @@ static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
>> }
>> #endif
>>
>> +static inline int mmc_regulator_set_ocr_vmmc_up(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + if (IS_ERR(mmc->supply.vmmc))
>> + return 0;
>
> Rather than adding these two new helper functions, how about adding
> the similar check in mmc_regulator_set_ocr() instead?
>
There's a number of drivers having 3 paths here:
1. IS_ERR() is true -> do nothing and go one
2. mmc_regulator_set_ocr() returns 0 -> some action and go on
3. mmc_regulator_set_ocr() returns an error -> bail out
So the question is: what should mmc_regulator_set_ocr_vmmc_up return
if IS_ERR() is true:
1. An errno? Then this errno would have to be different from the
error codes the function can normally return.
2. A positive value? Seems to be the best option
Then we could write:
ret = mmc_regulator_set_ocr()
if (ret < 0)
return ret;
if (!ret) {
some_action();
}
...
Works but I'm not sure whether it's very intuitive.
The other benefit of the proposed helpers is that they hide the
complexity of using mmc->supply.vmmc and ios->vdd.
Mileage may vary here. Do you have any preference?
> That should allow us to simplify some code in the host drivers too, right?
>
>> +
>> + return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd);
>> +}
>> +
>> +static inline int mmc_regulator_set_ocr_vmmc_off(struct mmc_host *mmc)
>> +{
>> + if (IS_ERR(mmc->supply.vmmc))
>> + return 0;
>> +
>> + return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
>> +}
>> +
>> int mmc_regulator_get_supply(struct mmc_host *mmc);
>>
>> static inline int mmc_card_is_removable(struct mmc_host *host)
>> --
>> 2.39.1
>>
>>
>
> Kind regards
> Uffe
