1. Patchew
  2. linux
  3. [PATCH V2 00/15] cpufreq: mediatek: Cleanup and support MT8183 and MT8186
  4. View patch

[PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional

Rex-BC Chen posted 15 patches 2 years, 5 months ago
Only 14 patches received!
There is a newer version of this series
[PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional
Posted by Rex-BC Chen 2 years, 5 months ago 
From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

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 <jia-wei.chang@mediatek.com>
---
 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");
 	if (IS_ERR(info->sram_reg))
 		info->sram_reg = NULL;
 	else {
-- 
2.18.0
Re: [PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional
Posted by Kevin Hilman 2 years, 5 months ago 
Rex-BC Chen <rex-bc.chen@mediatek.com> writes:

> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
>
> 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 <jia-wei.chang@mediatek.com>

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
Re: [PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional
Posted by Rex-BC Chen 2 years, 5 months ago 
On Fri, 2022-04-08 at 13:32 -0700, Kevin Hilman wrote:
> Rex-BC Chen <rex-bc.chen@mediatek.com> writes:
> 
> > From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> > 
> > 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 <jia-wei.chang@mediatek.com>
> 
> 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.

Thanks.

BRs,
Rex
Re: [PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional
Posted by Kevin Hilman 2 years, 5 months ago 
Rex-BC Chen <rex-bc.chen@mediatek.com> writes:

> On Fri, 2022-04-08 at 13:32 -0700, Kevin Hilman wrote:
>> Rex-BC Chen <rex-bc.chen@mediatek.com> writes:
>> 
>> > From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
>> > 
>> > 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 <jia-wei.chang@mediatek.com>
>> 
>> 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
Re: [PATCH V2 10/15] cpufreq: mediatek: Make sram regulator optional
Posted by AngeloGioacchino Del Regno 2 years, 5 months ago 
Il 08/04/22 06:59, Rex-BC Chen ha scritto:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> 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 <jia-wei.chang@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.