| 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: <25c285d4-4348-8bca-ba12-26464e13ed30@lechnology.com>
Date: Wed, 7 Feb 2018 16:21:58 -0600
From: David Lechner <david@...hnology.com>
To: Bartosz Golaszewski <brgl@...ev.pl>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Sekhar Nori <nsekhar@...com>,
Kevin Hilman <khilman@...nel.org>,
Russell King <linux@...linux.org.uk>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH 5/7] ARM: dts: da850: add power-domains properties to
device nodes
On 02/07/2018 07:45 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@...libre.com>
>
> Make all devices managed by PSCs use the genpd driver.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>
> ---
> arch/arm/boot/dts/da850.dtsi | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index ac2eef4e6b7c..a2a1a3b7de3c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -83,6 +83,7 @@
> ti,intc-size = <101>;
> reg = <0xfffee000 0x2000>;
> clocks = <&psc0 6>;
> + power-domains = <&pwc0>;
If we are making this a power domain consumer, we can probably drop the
clock property.
> };
> };
> clocks: clocks {
> @@ -125,6 +126,7 @@
> interrupt-parent = <&intc>;
> interrupts = <28>;
> clocks = <&psc0 15>;
> + power-domains = <&pwc0>;
> status = "disabled";
> };
> soc@...0000 {
> @@ -400,12 +402,14 @@
> clocks = <&psc1 1>, <&usb_refclkin>,
> <&pll0_auxclk>;
> clock-names = "fck", "usb_refclkin", "auxclk";
> + power-domains = <&pwc1>;
If we are going to use a power domain here, this driver will need to be re-written.
> };
> ehrpwm_tbclk: ehrpwm_tbclk {
> compatible = "ti,da830-tbclksync";
> #clock-cells = <0>;
> clocks = <&psc1 17>;
> clock-names = "fck";
> + power-domains = <&pwc1>;
This is a clock node, not a (platform) device node, so I am not sure having
a power domain here makes sense.
> };
> div4p5_clk: div4.5 {
> compatible = "ti,da830-div4p5ena";
> @@ -439,6 +443,7 @@
>
> ti,tptcs = <&edma0_tptc0 7>, <&edma0_tptc1 0>;
> clocks = <&psc0 0>;
> + power-domains = <&pwc0>;
We really just need power-domains here, not clocks. Same applies to
the rest of the edma* nodes below.
> };
> edma0_tptc0: tptc@...0 {
> compatible = "ti,edma3-tptc";
> @@ -446,6 +451,7 @@
> interrupts = <13>;
> interrupt-names = "edm3_tcerrint";
> clocks = <&psc0 1>;
> + power-domains = <&pwc0>;
> };
> edma0_tptc1: tptc@...0 {
> compatible = "ti,edma3-tptc";
> @@ -453,6 +459,7 @@
> interrupts = <32>;
> interrupt-names = "edm3_tcerrint";
> clocks = <&psc0 2>;
> + power-domains = <&pwc0>;
> };
> edma1: edma@...000 {
> compatible = "ti,edma3-tpcc";
> @@ -465,6 +472,7 @@
>
> ti,tptcs = <&edma1_tptc0 7>;
> clocks = <&psc1 0>;
> + power-domains = <&pwc1>;
> };
> edma1_tptc0: tptc@...000 {
> compatible = "ti,edma3-tptc";
> @@ -472,6 +480,7 @@
> interrupts = <95>;
> interrupt-names = "edm3_tcerrint";
> clocks = <&psc1 21>;
> + power-domains = <&pwc1>;
> };
> serial0: serial@...00 {
> compatible = "ti,da830-uart", "ns16550a";
> @@ -480,6 +489,7 @@
> reg-shift = <2>;
> interrupts = <25>;
> clocks = <&psc0 9>;
> + power-domains = <&pwc0>;
> status = "disabled";
> };
> serial1: serial@...000 {
> @@ -489,6 +499,7 @@
> reg-shift = <2>;
> interrupts = <53>;
> clocks = <&psc1 12>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> serial2: serial@...000 {
> @@ -498,6 +509,7 @@
> reg-shift = <2>;
> interrupts = <61>;
> clocks = <&psc1 13>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> rtc0: rtc@...00 {
> @@ -523,6 +535,7 @@
> #address-cells = <1>;
> #size-cells = <0>;
> clocks = <&psc1 11>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> clocksource: timer@...00 {
> @@ -545,6 +558,7 @@
> dmas = <&edma0 16 0>, <&edma0 17 0>;
> dma-names = "rx", "tx";
> clocks = <&psc0 5>;
> + power-domains = <&pwc0>;
> status = "disabled";
> };
> vpif: video@...000 {
> @@ -552,6 +566,7 @@
> reg = <0x217000 0x1000>;
> interrupts = <92>;
> clocks = <&psc1 9>;
> + power-domains = <&pwc1>;
> status = "disabled";
>
> /* VPIF capture port */
> @@ -575,6 +590,7 @@
> dmas = <&edma1 28 0>, <&edma1 29 0>;
> dma-names = "rx", "tx";
> clocks = <&psc1 18>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> ehrpwm0: pwm@...000 {
> @@ -585,6 +601,7 @@
> clocks = <&psc1 17>, <&ehrpwm_tbclk>;
> clock-names = "fck", "tbclk";
> status = "disabled";
> + power-domains = <&pwc1>;
nit picking: it would be nice to have power-domains before status just
to be consistent in the order.
> };
> ehrpwm1: pwm@...000 {
> compatible = "ti,da850-ehrpwm", "ti,am3352-ehrpwm",
> @@ -592,6 +609,7 @@
> #pwm-cells = <3>;
> reg = <0x302000 0x2000>;
> clocks = <&psc1 17>, <&ehrpwm_tbclk>;
> + power-domains = <&pwc1>;
> clock-names = "fck", "tbclk";
nit picking: it would be nice to keep clocks and clock-names next to each
other. Also several places below.
> status = "disabled";
> };
> @@ -601,6 +619,7 @@
> #pwm-cells = <3>;
> reg = <0x306000 0x80>;
> clocks = <&psc1 20>;
> + power-domains = <&pwc1>;
> clock-names = "fck";
> status = "disabled";
> };
> @@ -610,6 +629,7 @@
> #pwm-cells = <3>;
> reg = <0x307000 0x80>;
> clocks = <&psc1 20>;
> + power-domains = <&pwc1>;
> clock-names = "fck";
> status = "disabled";
> };
> @@ -619,6 +639,7 @@
> #pwm-cells = <3>;
> reg = <0x308000 0x80>;
> clocks = <&psc1 20>;
> + power-domains = <&pwc1>;
> clock-names = "fck";
> status = "disabled";
> };
> @@ -633,6 +654,7 @@
> dmas = <&edma0 14 0>, <&edma0 15 0>;
> dma-names = "rx", "tx";
> clocks = <&psc0 4>;
> + power-domains = <&pwc0>;
> status = "disabled";
> };
> spi1: spi@...000 {
> @@ -646,6 +668,7 @@
> dmas = <&edma0 18 0>, <&edma0 19 0>;
> dma-names = "rx", "tx";
> clocks = <&psc1 10>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> usb0: usb@...000 {
> @@ -658,6 +681,7 @@
> phys = <&usb_phy 0>;
> phy-names = "usb-phy";
> clocks = <&psc1 1>;
> + power-domains = <&pwc1>;
> status = "disabled";
>
> #address-cells = <1>;
> @@ -682,6 +706,7 @@
> #dma-cells = <2>;
> #dma-channels = <4>;
> clocks = <&psc1 1>;
> + power-domains = <&pwc1>;
This node should not have clocks or power-domains since it is
a child node. Instead, the parent node should have clock-ranges.
I have already fixed this in my v7 branch.
> status = "okay";
> };
> };
> @@ -690,6 +715,7 @@
> reg = <0x218000 0x2000>, <0x22c018 0x4>;
> interrupts = <67>;
> clocks = <&psc1 8>, <&sata_refclk>;
> + power-domains = <&pwc1>;
> clock-names = "fck", "refclk";
> status = "disabled";
> };
> @@ -713,6 +739,7 @@
> reg = <0x224000 0x1000>;
> clocks = <&psc1 5>;
> clock-names = "fck";
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> eth0: ethernet@...000 {
> @@ -729,6 +756,7 @@
> 36
> >;
> clocks = <&psc1 5>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> usb1: usb@...000 {
> @@ -738,6 +766,7 @@
> phys = <&usb_phy 1>;
> phy-names = "usb-phy";
> clocks = <&psc1 2>;
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> gpio: gpio@...000 {
> @@ -756,6 +785,7 @@
> interrupt-controller;
> #interrupt-cells = <2>;
> clocks = <&psc1 3>;
> + power-domains = <&pwc1>;
> clock-names = "gpio";
> };
> psc1: clock-controller@...000 {
> @@ -788,6 +818,7 @@
> <&edma0 0 1>;
> dma-names = "tx", "rx";
> clocks = <&psc1 7>;
> + power-domains = <&pwc1>;
> };
>
> lcdc: display@...000 {
> @@ -797,6 +828,7 @@
> max-pixelclock = <37500>;
> clocks = <&psc1 16>;
> clock-names = "fck";
> + power-domains = <&pwc1>;
> status = "disabled";
> };
> };
> @@ -809,6 +841,7 @@
> ranges = <0 0 0x60000000 0x08000000
> 1 0 0x68000000 0x00008000>;
> clocks = <&psc0 3>;
> + power-domains = <&pwc0>;
> status = "disabled";
> };
> memctrl: memory-controller@...00000 {
>
Bigger picture question: what is the benefit of adding
power-domain nodes to all of these devices if they are
already working?
Powered by blists - more mailing lists