| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191223145853.eaw23k2c5oyh7j44@flawful.org>
Date: Mon, 23 Dec 2019 15:58:53 +0100
From: Niklas Cassel <nks@...wful.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Niklas Cassel <niklas.cassel@...aro.org>,
linux-arm-msm@...r.kernel.org, amit.kucheria@...aro.org,
sboyd@...nel.org, vireshk@...nel.org, bjorn.andersson@...aro.org,
ulf.hansson@...aro.org, Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 1/5] dt-bindings: power: avs: Add support for CPR
(Core Power Reduction)
On Fri, Dec 20, 2019 at 03:33:56PM +0100, Niklas Cassel wrote:
> On Fri, Dec 20, 2019 at 10:31:53AM +0100, Rafael J. Wysocki wrote:
> > On Friday, November 29, 2019 10:39:11 PM CET Niklas Cassel wrote:
> > > Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
> > >
> > > Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>
> > > Signed-off-by: Niklas Cassel <niklas.cassel@...aro.org>
> > > Reviewed-by: Rob Herring <robh@...nel.org>
> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> > > Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
> > > ---
> > > Changes since v6:
> > > -Picked up Bjorn's and Ulf's Reviewed-by.
> > >
> > > .../bindings/power/avs/qcom,cpr.txt | 130 ++++++++++++++++++
> > > 1 file changed, 130 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > > new file mode 100644
> > > index 000000000000..ab0d5ebbad4e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > > @@ -0,0 +1,130 @@
> > > +QCOM CPR (Core Power Reduction)
> > > +
> > > +CPR (Core Power Reduction) is a technology to reduce core power on a CPU
> > > +or other device. Each OPP of a device corresponds to a "corner" that has
> > > +a range of valid voltages for a particular frequency. While the device is
> > > +running at a particular frequency, CPR monitors dynamic factors such as
> > > +temperature, etc. and suggests adjustments to the voltage to save power
> > > +and meet silicon characteristic requirements.
> > > +
> > > +- compatible:
> > > + Usage: required
> > > + Value type: <string>
> > > + Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
> > > +
> > > +- reg:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: base address and size of the rbcpr register region
> > > +
> > > +- interrupts:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: should specify the CPR interrupt
> > > +
> > > +- clocks:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: phandle to the reference clock
> > > +
> > > +- clock-names:
> > > + Usage: required
> > > + Value type: <stringlist>
> > > + Definition: must be "ref"
> > > +
> > > +- vdd-apc-supply:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: phandle to the vdd-apc-supply regulator
> > > +
> > > +- #power-domain-cells:
> > > + Usage: required
> > > + Value type: <u32>
> > > + Definition: should be 0
> > > +
> > > +- operating-points-v2:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: A phandle to the OPP table containing the
> > > + performance states supported by the CPR
> > > + power domain
> > > +
> > > +- acc-syscon:
> > > + Usage: optional
> > > + Value type: <phandle>
> > > + Definition: phandle to syscon for writing ACC settings
> > > +
> > > +- nvmem-cells:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: phandle to nvmem cells containing the data
> > > + that makes up a fuse corner, for each fuse corner.
> > > + As well as the CPR fuse revision.
> > > +
> > > +- nvmem-cell-names:
> > > + Usage: required
> > > + Value type: <stringlist>
> > > + Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
> > > + "cpr_quotient_offset3", "cpr_init_voltage1",
> > > + "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
> > > + "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
> > > + "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
> > > + for qcs404.
> > > +
> > > +Example:
> > > +
> > > + cpr_opp_table: cpr-opp-table {
> > > + compatible = "operating-points-v2-qcom-level";
> > > +
> > > + cpr_opp1: opp1 {
> > > + opp-level = <1>;
> > > + qcom,opp-fuse-level = <1>;
> > > + };
> > > + cpr_opp2: opp2 {
> > > + opp-level = <2>;
> > > + qcom,opp-fuse-level = <2>;
> > > + };
> > > + cpr_opp3: opp3 {
> > > + opp-level = <3>;
> > > + qcom,opp-fuse-level = <3>;
> > > + };
> > > + };
> > > +
> > > + power-controller@...8000 {
> > > + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> > > + reg = <0x0b018000 0x1000>;
> > > + interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
> > > + clocks = <&xo_board>;
> > > + clock-names = "ref";
> > > + vdd-apc-supply = <&pms405_s3>;
> > > + #power-domain-cells = <0>;
> > > + operating-points-v2 = <&cpr_opp_table>;
> > > + acc-syscon = <&tcsr>;
> > > +
> > > + nvmem-cells = <&cpr_efuse_quot_offset1>,
> > > + <&cpr_efuse_quot_offset2>,
> > > + <&cpr_efuse_quot_offset3>,
> > > + <&cpr_efuse_init_voltage1>,
> > > + <&cpr_efuse_init_voltage2>,
> > > + <&cpr_efuse_init_voltage3>,
> > > + <&cpr_efuse_quot1>,
> > > + <&cpr_efuse_quot2>,
> > > + <&cpr_efuse_quot3>,
> > > + <&cpr_efuse_ring1>,
> > > + <&cpr_efuse_ring2>,
> > > + <&cpr_efuse_ring3>,
> > > + <&cpr_efuse_revision>;
> > > + nvmem-cell-names = "cpr_quotient_offset1",
> > > + "cpr_quotient_offset2",
> > > + "cpr_quotient_offset3",
> > > + "cpr_init_voltage1",
> > > + "cpr_init_voltage2",
> > > + "cpr_init_voltage3",
> > > + "cpr_quotient1",
> > > + "cpr_quotient2",
> > > + "cpr_quotient3",
> > > + "cpr_ring_osc1",
> > > + "cpr_ring_osc2",
> > > + "cpr_ring_osc3",
> > > + "cpr_fuse_revision";
> > > + };
> > >
> >
> > I have queued up this one and the [2/5] for 5.6, but if you'd rather want them
> > to go in via a different patch, please let me know and I'll drop them.
> >
>
> Thanks a lot Rafael!
>
> I would very much prefer them to go via your tree.
>
> Unfortunately it seems like kbuild test robot
> found an incorrect printk format specifier in
> one of the debug prints.
>
> Line 838
> dev_dbg(dev, "efuse read(%s) = %x, bytes %ld\n", cname, *data, len);
>
> should be
> dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len);
>
> So %zd rather than %ld.
>
> This was obviously an error, but didn't show when
> compiling on arm64 or x86_64.
>
> Sorry for this inconvenience.
>
> Could you fix up the commit or do I need to do a respin?
Hello Rafael,
Since the intel test robot found another problem,
I decided to cook up a series that can be applied
(or squashed) on top of your bleeding-edge branch
here instead:
https://patchwork.kernel.org/project/linux-pm/list/?series=220955
Again, sorry for the inconvenience.
Kind regards,
Niklas
Powered by blists - more mailing lists