2022-04-16 00:17:18

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional

Rex-BC Chen <[email protected]> writes:

> On Fri, 2022-04-08 at 13:32 -0700, Kevin Hilman wrote:
>> Rex-BC Chen <[email protected]> writes:
>>
>> > From: Jia-Wei Chang <[email protected]>
>> >
>> > For some MediaTek SoCs, like MT8186, it's possible that the sram
>> > regulator
>> > is shared between CPU and CCI.
>> >
>> > Signed-off-by: Jia-Wei Chang <[email protected]>
>>
>> nit: missing your sign-off.
>>
>> > ---
>> > drivers/cpufreq/mediatek-cpufreq.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/cpufreq/mediatek-cpufreq.c
>> > b/drivers/cpufreq/mediatek-cpufreq.c
>> > index 9e9bce0ff235..8f688d47e64b 100644
>> > --- a/drivers/cpufreq/mediatek-cpufreq.c
>> > +++ b/drivers/cpufreq/mediatek-cpufreq.c
>> > @@ -435,7 +435,7 @@ static int mtk_cpu_dvfs_info_init(struct
>> > mtk_cpu_dvfs_info *info, int cpu)
>> > }
>> >
>> > /* Both presence and absence of sram regulator are valid cases.
>> > */
>> > - info->sram_reg = regulator_get_exclusive(cpu_dev, "sram");
>> > + info->sram_reg = regulator_get_optional(cpu_dev, "sram");
>>
>> The changelog says that this regulator may be shared with CCI, so I
>> understand it's no longer exclusive. But here you make it optional,
>> which should be explained in the changelog. If it's not actually
>> optional, then it should just be normal "get".
>>
>> Kevin
>
> Hello Kevin,
>
> Since cpufreq and cci devfreq might share the same sram regulator in
> MediaTek SoC, it is no longer exclusive as you mentioned.
>
> The reason to use regulator_get_optional is we hope regulator framework
> can return error for error handling rather than a dummy handler from
> regulator_get api.
>
> I will add this to commit message in next version.

OK, sounds good.

Thanks,

Kevin