On Tue, Apr 30, 2019 at 01:31:32PM -0700, Stephen Boyd wrote:
> Quoting Roger Lu (2019-04-30 04:20:10)
> > Document the binding for enabling mtk svs on MediaTek SoC.
> >
> > Signed-off-by: Roger Lu <roger.lu@xxxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/power/mtk-svs.txt | 70 +++++++++++++++++++
> > 1 file changed, 70 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/mtk-svs.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/mtk-svs.txt b/Documentation/devicetree/bindings/power/mtk-svs.txt
> > new file mode 100644
> > index 000000000000..355329db74ba
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/mtk-svs.txt
> > @@ -0,0 +1,70 @@
> > +* Mediatek Smart Voltage Scaling (MTK SVS)
> > +
> > +This describes the device tree binding for the MTK SVS controller
> > +which helps provide the optimized CPU/GPU/CCI voltages. This device also
> > +needs thermal data to calculate thermal slope for accurately compensate
> > +the voltages when temperature change.
> > +
> > +Required properties:
> > +- compatible:
> > + - "mediatek,mt8183-svs" : For MT8183 family of SoCs
> > +- reg: Address range of the MTK SVS controller.
> > +- interrupts: IRQ for the MTK SVS controller.
> > +- clocks, clock-names: Clocks needed for the svs controller. required
> > + clocks are:
> > + "main_clk": Main clock needed for register access
> > +- nvmem-cells: Phandle to the calibration data provided by a nvmem device.
> > +- nvmem-cell-names: Should be "svs-calibration-data" and "calibration-data"
> > +- svs_xxx: Phandle of svs_bank device for controlling corresponding opp
>
> Properties shouldn't have underscores in them. Use dashes?
It's also a node, not a property.
>
> > + table and power-domains.
> > +- vxxx-supply: Phandle to each regulator. vxxx can be "vcpu_little",
> > + "vcpu_big", "vcci" and "vgpu".
Just list each property instead of the indirection with 'xxx'. Though
here to, these should be in the nodes actually getting the power.
> > +
> > +Example:
> > +
> > + svs: svs@1100b000 {
> > + compatible = "mediatek,mt8183-svs";
> > + reg = <0 0x1100b000 0 0x1000>;
> > + interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW 0>;
> > + clocks = <&infracfg CLK_INFRA_THERM>;
> > + clock-names = "main_clk";
> > + nvmem-cells = <&svs_calibration>, <&thermal_calibration>;
> > + nvmem-cell-names = "svs-calibration-data", "calibration-data";
> > +
> > + svs_cpu_little: svs_cpu_little {
> > + compatible = "mediatek,mt8183-svs-cpu-little";
Not documented. Though I think the child nodes should be removed if you
do as Stephen suggests below.
> > + operating-points-v2 = <&cluster0_opp>;
> > + };
> > +
> > + svs_cpu_big: svs_cpu_big {
> > + compatible = "mediatek,mt8183-svs-cpu-big";
> > + operating-points-v2 = <&cluster1_opp>;
> > + };
> > +
> > + svs_cci: svs_cci {
> > + compatible = "mediatek,mt8183-svs-cci";
> > + operating-points-v2 = <&cluster2_opp>;
> > + };
> > +
> > + svs_gpu: svs_gpu {
> > + compatible = "mediatek,mt8183-svs-gpu";
> > + power-domains = <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
> > + operating-points-v2 = <&gpu_opp_table>;
> > + };
>
> It looks like you need multiple OPPs for a single device, because it has
> different independent power supplies it wants to associate the OPP
> tables with? Why can't these OPP tables be attached to the devices that
> use them, i.e. CPU, GPU, CCI, etc.? Seems odd that those devices don't
> have OPP tables that this hardware block can look up somehow. Similarly,
> the power domains should probably be part of the devices that are using
> them and not these sub-nodes that are mirroring the other hardware
> blocks in the system?
>
> > + };
> > +
> > + &svs_cpu_little {
