Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property

[Rex-BC Chen <rex-bc.chen@mediatek.com>]

On Fri, 2022-04-22 at 19:34 +0200, Krzysztof Kozlowski wrote:
> On 22/04/2022 19:26, Krzysztof Kozlowski wrote:
> > On 22/04/2022 09:52, Rex-BC Chen wrote:
> > > MediaTek Cache Coherent Interconnect (CCI) uses software devfreq
> > > module
> > > for scaling clock frequency and adjust voltage.
> > > The phandle could be linked between CPU and MediaTek CCI for some
> > > MediaTek SoCs, like MT8183 and MT8186.
> > > Therefore, we add this property in cpufreq-mediatek.txt.
> > > 
> > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > > ---
> > >  .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5
> > > +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > index b8233ec91d3d..3387e1e2a2df 100644
> > > --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > @@ -20,6 +20,11 @@ Optional properties:
> > >  	       Vsram to fit SoC specific needs. When absent, the
> > > voltage scaling
> > >  	       flow is handled by hardware, hence no software "voltage
> > > tracking" is
> > >  	       needed.
> > > +- mediatek,cci:
> > > +	MediaTek Cache Coherent Interconnect (CCI) uses the software
> > > devfreq module to
> > > +	scale the clock frequency and adjust the voltage.
> > 
> > Devfreq is a SW mechanism, it should not be part of bindings
> > description.

Hello Krzysztof,

The reason we want to get the "mediatek,cci":
We need to check the mediatek cci is ready and probed done.
Because cpufreq and mediatek cci are sharing the same regulator in
little core cpus.
Therefore, to prevent high frequency low voltage issue, we need to make
sure the mediatek cci is ready.

If mediatek cci is ready, cpufreq and mediatek cci will register the
same regulator and from regulator's implementation, if there are two
device using the same regulator, the framwork will make sure it's using
the max voltage.
For example:
mediatek cci set 1.2V originally. When cpufreq want to adjust lower
frequency adn set voltage to 1.0V.
The framework will remain using 1.2V to prevent crash of mediatek cci.

Therefore, we need to confirm the mediatek cci is ready and register
the regulator.

> > 
> > > +	For details, please refer to
> > > +	Documentation/devicetree/bindings/interconnect/mediatek,cci.yam
> > > l
> > 
> > Since the file does not exist, I have troubles reviewing it. First
> > of
> > all, you already have "mediatek,cci-control" property in DT, so why
> > using different name?

I am not sure where is "mediatek,cci-control". I think this name is not
used before.

> > 
> > Second, it looks like you want to put devfreq into bindings instead
> > of
> > using proper interconnect bindings.
> 
> Actually judging by the driver this looks like some
> device-boot-time-ordering, so I wonder whether this is a proper way
> to
> express it.

Yes, we need to get the mediatek cci node and let cpufreq and mediatek
cci link succefully. In that case, we can know the mediatek cci is
ready. And we can set the voltage using the regulator framwork.

[1]: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220422075239.16437-11-rex-bc.chen@mediatek.com/

BRs,
Rex
> 
> 
> Best regards,
> Krzysztof