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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 20 Dec 2019 15:33:56 +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 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?


Kind regards,
Niklas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ