LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Stephen Boyd <sboyd@kernel.org>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Roger Lu <roger.lu@mediatek.com>,
	Viresh Kumar <vireshk@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>, Nishanth Menon <nm@ti.com>,
	Kevin Hilman <khilman@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [RFC v1 1/3] dt-bindings: soc: add mtk svs dt-bindings
Date: Thu, 2 May 2019 16:06:47 -0500	[thread overview]
Message-ID: <20190502210647.GA32209@bogus> (raw)
In-Reply-To: <155665629219.168659.8221738507474891604@swboyd.mtv.corp.google.com>

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@mediatek.com>
> > ---
> >  .../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 {

  parent reply	other threads:[~2019-05-02 21:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 11:20 PM / AVS: SVS: Introduce SVS engine Roger Lu
2019-04-30 11:20 ` [RFC v1 1/3] dt-bindings: soc: add mtk svs dt-bindings Roger Lu
2019-04-30 20:31   ` Stephen Boyd
2019-05-02  6:19     ` Roger Lu
2019-05-03 21:08       ` Stephen Boyd
2019-05-07  7:50         ` Roger Lu
2019-05-07 20:49           ` Stephen Boyd
2019-05-13  0:34             ` Roger Lu
2019-05-02 21:06     ` Rob Herring [this message]
2019-04-30 11:20 ` [RFC v1 2/3] arm64: dts: mt8183: add svs device information Roger Lu
2019-04-30 11:20 ` [RFC v1 3/3] PM / AVS: SVS: Introduce SVS engine Roger Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190502210647.GA32209@bogus \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=khilman@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=roger.lu@mediatek.com \
    --cc=sboyd@kernel.org \
    --cc=vireshk@kernel.org \
    --subject='Re: [RFC v1 1/3] dt-bindings: soc: add mtk svs dt-bindings' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).