diff mbox

ARM: EXYNOS: add power domains support for EXYNOS5440

Message ID 2729097.X0eAsuYF97@amdc1032 (mailing list archive)
State New, archived
Headers show

Commit Message

Bartlomiej Zolnierkiewicz Aug. 2, 2013, 1:23 p.m. UTC
On EXYNOS5440 power domains are handled in a different way than on
the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
driver. Then add device tree nodes for PCIe (controlling power for
PCIe host controller) and Conn2 (controlling power for Ethernet,
SATA and USB controllers) power domains to exynos5440.dtsi.

Currently if runtime Power Management is enabled and the driver
for devices under power domain is disabled then the power domain
will be disabled by EXYNOS pm_domains driver. To make more active
use of power domains (dynamically enable and disabled them as
needed) it is required to add runtime PM support to pci-exynos,
stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
done later).

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
---
 Documentation/devicetree/bindings/arm/exynos/power_domain.txt |   33 ++
 arch/arm/boot/dts/exynos5440.dtsi                             |   23 +
 arch/arm/mach-exynos/Kconfig                                  |    1 
 arch/arm/mach-exynos/common.c                                 |    4 
 arch/arm/mach-exynos/pm_domains.c                             |  138 +++++++++-
 5 files changed, 190 insertions(+), 9 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Stephen Warren Aug. 2, 2013, 9:01 p.m. UTC | #1
(CCing the other DT maintainers, hence quoting binding in full)

On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote:
> On EXYNOS5440 power domains are handled in a different way than on
> the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
> driver. Then add device tree nodes for PCIe (controlling power for
> PCIe host controller) and Conn2 (controlling power for Ethernet,
> SATA and USB controllers) power domains to exynos5440.dtsi.
> 
> Currently if runtime Power Management is enabled and the driver
> for devices under power domain is disabled then the power domain
> will be disabled by EXYNOS pm_domains driver. To make more active
> use of power domains (dynamically enable and disabled them as
> needed) it is required to add runtime PM support to pci-exynos,
> stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
> done later).
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  Documentation/devicetree/bindings/arm/exynos/power_domain.txt |   33 ++
>  arch/arm/boot/dts/exynos5440.dtsi                             |   23 +
>  arch/arm/mach-exynos/Kconfig                                  |    1 
>  arch/arm/mach-exynos/common.c                                 |    4 
>  arch/arm/mach-exynos/pm_domains.c                             |  138 +++++++++-
>  5 files changed, 190 insertions(+), 9 deletions(-)
> 
> Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> ===================================================================
> --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:45:53.551392396 +0200
> +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:46:29.799391845 +0200
> @@ -5,20 +5,47 @@ to gate power to one or more peripherals
>  
>  Required Properties:
>  - compatible: should be one of the following.
> -    * samsung,exynos4210-pd - for exynos4210 type power domain.
> +    * samsung,exynos4210-pd - for Exynos4210 type power domain.
> +    * samsung,exynos5440-pd - for Exynos5440 type power domain.
>  - reg: physical base address of the controller and length of memory mapped
> -    region.
> +    region (Exynos4210 type power domain) or bit offset in the control
> +    register (Exynos5440 type power domain).
> +
> +Additional parent node must be created for Exynos5440 power domains with
> +the following required properties:
> +- compatible: samsung,exynos5440-power-domains, simple-bus
> +- reg: physical base address of the XMU controller and length of memory
> +    mapped region

It's a little odd to describe the child nodes first. Given the 4210 and
5440 bindings work so differently, I'd suggest making separate binding
files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt.

The node being describe here clearly is not a simple-bus; the child
nodes appear to have a specific need that their parent be compatible =
"samsung,exynos5440-power-domains", hence they aren't the independent
devices that simple-bus would usually contain.

Note that I only briefly reviewed the low-level structural aspects of
the binding that I mentioned above; I haven't thought about the binding
at a higher level of e.g. "does this binding conceptually make sense".

>  Node of a device using power domains must have a samsung,power-domain property
>  defined with a phandle to respective power domain.
>  
> -Example:
> +Example for Exynos4210 compatible power domain:
>  
>  	lcd0: power-domain-lcd0 {
>  		compatible = "samsung,exynos4210-pd";
>  		reg = <0x10023C00 0x10>;
>  	};
>  
> +Example for Exynos5440 compatible power domains:
> +
> +	power-domains@00160000 {
> +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> +		reg = <0x00160000 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pd_pcie: pcie-power-domain@6 {
> +			compatible = "samsung,exynos5440-pd";
> +			reg = <6>;
> +		};
> +
> +		pd_conn2: conn2-power-domain@7 {
> +			compatible = "samsung,exynos5440-pd";
> +			reg = <7>;
> +		};
> +	};
> +
>  Example of the node using power domain:
>  
>  	node {
> Index: b/arch/arm/boot/dts/exynos5440.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:45:53.599392397 +0200
> +++ b/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:46:29.815391842 +0200
> @@ -29,6 +29,23 @@
>  		#clock-cells = <1>;
>  	};
>  
> +	power-domains@00160000 {
> +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> +		reg = <0x00160000 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pd_pcie: pcie-power-domain@6 {
> +			compatible = "samsung,exynos5440-pd";
> +			reg = <6>;
> +		};
> +
> +		pd_conn2: conn2-power-domain@7 {
> +			compatible = "samsung,exynos5440-pd";
> +			reg = <7>;
> +		};
> +	};
> +
>  	gic:interrupt-controller@2E0000 {
>  		compatible = "arm,cortex-a15-gic";
>  		#interrupt-cells = <3>;
> @@ -192,6 +209,7 @@
>  		phy-mode = "sgmii";
>  		clocks = <&clock 25>;
>  		clock-names = "stmmaceth";
> +		samsung,power-domain = <&pd_conn2>;
>  	};
>  
>  	amba {
> @@ -240,6 +258,7 @@
>  		interrupts = <0 30 0>;
>  		clocks = <&clock 23>;
>  		clock-names = "sata";
> +		samsung,power-domain = <&pd_conn2>;
>  	};
>  
>  	ohci@220000 {
> @@ -248,6 +267,7 @@
>  		interrupts = <0 29 0>;
>  		clocks = <&clock 24>;
>  		clock-names = "usbhost";
> +		samsung,power-domain = <&pd_conn2>;
>  	};
>  
>  	ehci@221000 {
> @@ -256,6 +276,7 @@
>  		interrupts = <0 29 0>;
>  		clocks = <&clock 24>;
>  		clock-names = "usbhost";
> +		samsung,power-domain = <&pd_conn2>;
>  	};
>  
>  	pcie@290000 {
> @@ -275,6 +296,7 @@
>  		#interrupt-cells = <1>;
>  		interrupt-map-mask = <0 0 0 0>;
>  		interrupt-map = <0x0 0 &gic 53>;
> +		samsung,power-domain = <&pd_pcie>;
>  	};
>  
>  	pcie@2a0000 {
> @@ -294,5 +316,6 @@
>  		#interrupt-cells = <1>;
>  		interrupt-map-mask = <0 0 0 0>;
>  		interrupt-map = <0x0 0 &gic 56>;
> +		samsung,power-domain = <&pd_pcie>;
>  	};
>  };

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Aug. 3, 2013, 8:14 p.m. UTC | #2
On Fri, Aug 02, 2013 at 10:01:52PM +0100, Stephen Warren wrote:
> (CCing the other DT maintainers, hence quoting binding in full)
> 
> On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote:
> > On EXYNOS5440 power domains are handled in a different way than on
> > the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
> > driver. Then add device tree nodes for PCIe (controlling power for
> > PCIe host controller) and Conn2 (controlling power for Ethernet,
> > SATA and USB controllers) power domains to exynos5440.dtsi.
> > 
> > Currently if runtime Power Management is enabled and the driver
> > for devices under power domain is disabled then the power domain
> > will be disabled by EXYNOS pm_domains driver. To make more active
> > use of power domains (dynamically enable and disabled them as
> > needed) it is required to add runtime PM support to pci-exynos,
> > stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
> > done later).
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> > Cc: Tomasz Figa <t.figa@samsung.com>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > ---
> >  Documentation/devicetree/bindings/arm/exynos/power_domain.txt |   33 ++
> >  arch/arm/boot/dts/exynos5440.dtsi                             |   23 +
> >  arch/arm/mach-exynos/Kconfig                                  |    1 
> >  arch/arm/mach-exynos/common.c                                 |    4 
> >  arch/arm/mach-exynos/pm_domains.c                             |  138 +++++++++-
> >  5 files changed, 190 insertions(+), 9 deletions(-)
> > 
> > Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> > ===================================================================
> > --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:45:53.551392396 +0200
> > +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:46:29.799391845 +0200
> > @@ -5,20 +5,47 @@ to gate power to one or more peripherals
> >  
> >  Required Properties:
> >  - compatible: should be one of the following.
> > -    * samsung,exynos4210-pd - for exynos4210 type power domain.
> > +    * samsung,exynos4210-pd - for Exynos4210 type power domain.
> > +    * samsung,exynos5440-pd - for Exynos5440 type power domain.
> >  - reg: physical base address of the controller and length of memory mapped
> > -    region.
> > +    region (Exynos4210 type power domain) or bit offset in the control
> > +    register (Exynos5440 type power domain).
> > +
> > +Additional parent node must be created for Exynos5440 power domains with
> > +the following required properties:
> > +- compatible: samsung,exynos5440-power-domains, simple-bus
> > +- reg: physical base address of the XMU controller and length of memory
> > +    mapped region
> 
> It's a little odd to describe the child nodes first. Given the 4210 and
> 5440 bindings work so differently, I'd suggest making separate binding
> files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt.
> 
> The node being describe here clearly is not a simple-bus; the child
> nodes appear to have a specific need that their parent be compatible =
> "samsung,exynos5440-power-domains", hence they aren't the independent
> devices that simple-bus would usually contain.

+1. This is most definitely not a simple-bus, the child nodes reg
properties cdon't represent the same address space as the
parent's reg property.

How much does the association of bit-offsets to power domains vary? 

> 
> Note that I only briefly reviewed the low-level structural aspects of
> the binding that I mentioned above; I haven't thought about the binding
> at a higher level of e.g. "does this binding conceptually make sense".

This is unfortunately difficult to do :(

> 
> >  Node of a device using power domains must have a samsung,power-domain property
> >  defined with a phandle to respective power domain.
> >  
> > -Example:
> > +Example for Exynos4210 compatible power domain:
> >  
> >  	lcd0: power-domain-lcd0 {
> >  		compatible = "samsung,exynos4210-pd";
> >  		reg = <0x10023C00 0x10>;
> >  	};
> >  
> > +Example for Exynos5440 compatible power domains:
> > +
> > +	power-domains@00160000 {
> > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > +		reg = <0x00160000 0x1000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		pd_pcie: pcie-power-domain@6 {
> > +			compatible = "samsung,exynos5440-pd";
> > +			reg = <6>;
> > +		};
> > +
> > +		pd_conn2: conn2-power-domain@7 {
> > +			compatible = "samsung,exynos5440-pd";
> > +			reg = <7>;
> > +		};
> > +	};
> > +
> >  Example of the node using power domain:
> >  
> >  	node {
> > Index: b/arch/arm/boot/dts/exynos5440.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:45:53.599392397 +0200
> > +++ b/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:46:29.815391842 +0200
> > @@ -29,6 +29,23 @@
> >  		#clock-cells = <1>;
> >  	};
> >  
> > +	power-domains@00160000 {
> > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > +		reg = <0x00160000 0x1000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		pd_pcie: pcie-power-domain@6 {
> > +			compatible = "samsung,exynos5440-pd";
> > +			reg = <6>;
> > +		};
> > +
> > +		pd_conn2: conn2-power-domain@7 {
> > +			compatible = "samsung,exynos5440-pd";
> > +			reg = <7>;
> > +		};
> > +	};
> > +
> >  	gic:interrupt-controller@2E0000 {
> >  		compatible = "arm,cortex-a15-gic";
> >  		#interrupt-cells = <3>;
> > @@ -192,6 +209,7 @@
> >  		phy-mode = "sgmii";
> >  		clocks = <&clock 25>;
> >  		clock-names = "stmmaceth";
> > +		samsung,power-domain = <&pd_conn2>;
> >  	};

This feels like a clock style binding would fit better:
samsung,power-domain = <&pwr 7>;

But there may be a better, more general way of handling this. I'm not
sure how this relates to regulators and so on, and how other vendors
handle this stuff...

Thanks,
Mark.

> >  
> >  	amba {
> > @@ -240,6 +258,7 @@
> >  		interrupts = <0 30 0>;
> >  		clocks = <&clock 23>;
> >  		clock-names = "sata";
> > +		samsung,power-domain = <&pd_conn2>;
> >  	};
> >  
> >  	ohci@220000 {
> > @@ -248,6 +267,7 @@
> >  		interrupts = <0 29 0>;
> >  		clocks = <&clock 24>;
> >  		clock-names = "usbhost";
> > +		samsung,power-domain = <&pd_conn2>;
> >  	};
> >  
> >  	ehci@221000 {
> > @@ -256,6 +276,7 @@
> >  		interrupts = <0 29 0>;
> >  		clocks = <&clock 24>;
> >  		clock-names = "usbhost";
> > +		samsung,power-domain = <&pd_conn2>;
> >  	};
> >  
> >  	pcie@290000 {
> > @@ -275,6 +296,7 @@
> >  		#interrupt-cells = <1>;
> >  		interrupt-map-mask = <0 0 0 0>;
> >  		interrupt-map = <0x0 0 &gic 53>;
> > +		samsung,power-domain = <&pd_pcie>;
> >  	};
> >  
> >  	pcie@2a0000 {
> > @@ -294,5 +316,6 @@
> >  		#interrupt-cells = <1>;
> >  		interrupt-map-mask = <0 0 0 0>;
> >  		interrupt-map = <0x0 0 &gic 56>;
> > +		samsung,power-domain = <&pd_pcie>;
> >  	};
> >  };
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz Aug. 6, 2013, 4:16 p.m. UTC | #3
Hi,

On Saturday, August 03, 2013 09:14:11 PM Mark Rutland wrote:
> On Fri, Aug 02, 2013 at 10:01:52PM +0100, Stephen Warren wrote:
> > (CCing the other DT maintainers, hence quoting binding in full)
> > 
> > On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote:
> > > On EXYNOS5440 power domains are handled in a different way than on
> > > the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
> > > driver. Then add device tree nodes for PCIe (controlling power for
> > > PCIe host controller) and Conn2 (controlling power for Ethernet,
> > > SATA and USB controllers) power domains to exynos5440.dtsi.
> > > 
> > > Currently if runtime Power Management is enabled and the driver
> > > for devices under power domain is disabled then the power domain
> > > will be disabled by EXYNOS pm_domains driver. To make more active
> > > use of power domains (dynamically enable and disabled them as
> > > needed) it is required to add runtime PM support to pci-exynos,
> > > stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
> > > done later).
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> > > Cc: Tomasz Figa <t.figa@samsung.com>
> > > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > > ---
> > >  Documentation/devicetree/bindings/arm/exynos/power_domain.txt |   33 ++
> > >  arch/arm/boot/dts/exynos5440.dtsi                             |   23 +
> > >  arch/arm/mach-exynos/Kconfig                                  |    1 
> > >  arch/arm/mach-exynos/common.c                                 |    4 
> > >  arch/arm/mach-exynos/pm_domains.c                             |  138 +++++++++-
> > >  5 files changed, 190 insertions(+), 9 deletions(-)
> > > 
> > > Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> > > ===================================================================
> > > --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:45:53.551392396 +0200
> > > +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:46:29.799391845 +0200
> > > @@ -5,20 +5,47 @@ to gate power to one or more peripherals
> > >  
> > >  Required Properties:
> > >  - compatible: should be one of the following.
> > > -    * samsung,exynos4210-pd - for exynos4210 type power domain.
> > > +    * samsung,exynos4210-pd - for Exynos4210 type power domain.
> > > +    * samsung,exynos5440-pd - for Exynos5440 type power domain.
> > >  - reg: physical base address of the controller and length of memory mapped
> > > -    region.
> > > +    region (Exynos4210 type power domain) or bit offset in the control
> > > +    register (Exynos5440 type power domain).
> > > +
> > > +Additional parent node must be created for Exynos5440 power domains with
> > > +the following required properties:
> > > +- compatible: samsung,exynos5440-power-domains, simple-bus
> > > +- reg: physical base address of the XMU controller and length of memory
> > > +    mapped region
> > 
> > It's a little odd to describe the child nodes first. Given the 4210 and
> > 5440 bindings work so differently, I'd suggest making separate binding
> > files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt.

OK, I'll fix it.

> > The node being describe here clearly is not a simple-bus; the child
> > nodes appear to have a specific need that their parent be compatible =
> > "samsung,exynos5440-power-domains", hence they aren't the independent
> > devices that simple-bus would usually contain.
> 
> +1. This is most definitely not a simple-bus, the child nodes reg
> properties cdon't represent the same address space as the
> parent's reg property.

What does in mean in the practice? What should I do instead?

> How much does the association of bit-offsets to power domains vary? 

On EXYNOS5440 power domains are controlled by control and status registers
which are shared among all power domains. On other EXYNOS SoCs there are
separate control/status registers for each power domain.

> > Note that I only briefly reviewed the low-level structural aspects of
> > the binding that I mentioned above; I haven't thought about the binding
> > at a higher level of e.g. "does this binding conceptually make sense".
> 
> This is unfortunately difficult to do :(
> 
> > 
> > >  Node of a device using power domains must have a samsung,power-domain property
> > >  defined with a phandle to respective power domain.
> > >  
> > > -Example:
> > > +Example for Exynos4210 compatible power domain:
> > >  
> > >  	lcd0: power-domain-lcd0 {
> > >  		compatible = "samsung,exynos4210-pd";
> > >  		reg = <0x10023C00 0x10>;
> > >  	};
> > >  
> > > +Example for Exynos5440 compatible power domains:
> > > +
> > > +	power-domains@00160000 {
> > > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > > +		reg = <0x00160000 0x1000>;
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		pd_pcie: pcie-power-domain@6 {
> > > +			compatible = "samsung,exynos5440-pd";
> > > +			reg = <6>;
> > > +		};
> > > +
> > > +		pd_conn2: conn2-power-domain@7 {
> > > +			compatible = "samsung,exynos5440-pd";
> > > +			reg = <7>;
> > > +		};
> > > +	};
> > > +
> > >  Example of the node using power domain:
> > >  
> > >  	node {
> > > Index: b/arch/arm/boot/dts/exynos5440.dtsi
> > > ===================================================================
> > > --- a/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:45:53.599392397 +0200
> > > +++ b/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:46:29.815391842 +0200
> > > @@ -29,6 +29,23 @@
> > >  		#clock-cells = <1>;
> > >  	};
> > >  
> > > +	power-domains@00160000 {
> > > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > > +		reg = <0x00160000 0x1000>;
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		pd_pcie: pcie-power-domain@6 {
> > > +			compatible = "samsung,exynos5440-pd";
> > > +			reg = <6>;
> > > +		};
> > > +
> > > +		pd_conn2: conn2-power-domain@7 {
> > > +			compatible = "samsung,exynos5440-pd";
> > > +			reg = <7>;
> > > +		};
> > > +	};
> > > +
> > >  	gic:interrupt-controller@2E0000 {
> > >  		compatible = "arm,cortex-a15-gic";
> > >  		#interrupt-cells = <3>;
> > > @@ -192,6 +209,7 @@
> > >  		phy-mode = "sgmii";
> > >  		clocks = <&clock 25>;
> > >  		clock-names = "stmmaceth";
> > > +		samsung,power-domain = <&pd_conn2>;
> > >  	};
> 
> This feels like a clock style binding would fit better:
> samsung,power-domain = <&pwr 7>;
> 
> But there may be a better, more general way of handling this. I'm not
> sure how this relates to regulators and so on, and how other vendors
> handle this stuff...

Other vendors don't implement device tree for their power domains yet.

Also there is not much power domain users in the tree currently:

$ git grep pm_genpd_init
arch/arm/mach-exynos/pm_domains.c:          pm_genpd_init(&pd->pd, NULL, !on);
arch/arm/mach-s3c64xx/pm.c:         pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
arch/arm/mach-s3c64xx/pm.c:         pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
arch/arm/mach-shmobile/pm-r8a7779.c:        pm_genpd_init(genpd, NULL, false);
arch/arm/mach-shmobile/pm-rmobile.c:        pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
drivers/base/power/domain.c: * pm_genpd_init - Initialize a generic I/O PM domain object.
drivers/base/power/domain.c:void pm_genpd_init(struct generic_pm_domain *genpd,
include/linux/pm_domain.h:extern void pm_genpd_init(struct generic_pm_domain *genpd,
include/linux/pm_domain.h:static inline void pm_genpd_init(struct generic_pm_domain *genpd,

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Thanks,
> Mark.
> 
> > >  
> > >  	amba {
> > > @@ -240,6 +258,7 @@
> > >  		interrupts = <0 30 0>;
> > >  		clocks = <&clock 23>;
> > >  		clock-names = "sata";
> > > +		samsung,power-domain = <&pd_conn2>;
> > >  	};
> > >  
> > >  	ohci@220000 {
> > > @@ -248,6 +267,7 @@
> > >  		interrupts = <0 29 0>;
> > >  		clocks = <&clock 24>;
> > >  		clock-names = "usbhost";
> > > +		samsung,power-domain = <&pd_conn2>;
> > >  	};
> > >  
> > >  	ehci@221000 {
> > > @@ -256,6 +276,7 @@
> > >  		interrupts = <0 29 0>;
> > >  		clocks = <&clock 24>;
> > >  		clock-names = "usbhost";
> > > +		samsung,power-domain = <&pd_conn2>;
> > >  	};
> > >  
> > >  	pcie@290000 {
> > > @@ -275,6 +296,7 @@
> > >  		#interrupt-cells = <1>;
> > >  		interrupt-map-mask = <0 0 0 0>;
> > >  		interrupt-map = <0x0 0 &gic 53>;
> > > +		samsung,power-domain = <&pd_pcie>;
> > >  	};
> > >  
> > >  	pcie@2a0000 {
> > > @@ -294,5 +316,6 @@
> > >  		#interrupt-cells = <1>;
> > >  		interrupt-map-mask = <0 0 0 0>;
> > >  		interrupt-map = <0x0 0 &gic 56>;
> > > +		samsung,power-domain = <&pd_pcie>;
> > >  	};
> > >  };

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Aug. 8, 2013, 2:29 p.m. UTC | #4
On Tue, Aug 06, 2013 at 05:16:56PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Saturday, August 03, 2013 09:14:11 PM Mark Rutland wrote:
> > On Fri, Aug 02, 2013 at 10:01:52PM +0100, Stephen Warren wrote:
> > > (CCing the other DT maintainers, hence quoting binding in full)
> > > 
> > > On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote:
> > > > On EXYNOS5440 power domains are handled in a different way than on
> > > > the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
> > > > driver. Then add device tree nodes for PCIe (controlling power for
> > > > PCIe host controller) and Conn2 (controlling power for Ethernet,
> > > > SATA and USB controllers) power domains to exynos5440.dtsi.
> > > > 
> > > > Currently if runtime Power Management is enabled and the driver
> > > > for devices under power domain is disabled then the power domain
> > > > will be disabled by EXYNOS pm_domains driver. To make more active
> > > > use of power domains (dynamically enable and disabled them as
> > > > needed) it is required to add runtime PM support to pci-exynos,
> > > > stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
> > > > done later).
> > > > 
> > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> > > > Cc: Tomasz Figa <t.figa@samsung.com>
> > > > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/exynos/power_domain.txt |   33 ++
> > > >  arch/arm/boot/dts/exynos5440.dtsi                             |   23 +
> > > >  arch/arm/mach-exynos/Kconfig                                  |    1 
> > > >  arch/arm/mach-exynos/common.c                                 |    4 
> > > >  arch/arm/mach-exynos/pm_domains.c                             |  138 +++++++++-
> > > >  5 files changed, 190 insertions(+), 9 deletions(-)
> > > > 
> > > > Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> > > > ===================================================================
> > > > --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:45:53.551392396 +0200
> > > > +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:46:29.799391845 +0200
> > > > @@ -5,20 +5,47 @@ to gate power to one or more peripherals
> > > >  
> > > >  Required Properties:
> > > >  - compatible: should be one of the following.
> > > > -    * samsung,exynos4210-pd - for exynos4210 type power domain.
> > > > +    * samsung,exynos4210-pd - for Exynos4210 type power domain.
> > > > +    * samsung,exynos5440-pd - for Exynos5440 type power domain.
> > > >  - reg: physical base address of the controller and length of memory mapped
> > > > -    region.
> > > > +    region (Exynos4210 type power domain) or bit offset in the control
> > > > +    register (Exynos5440 type power domain).
> > > > +
> > > > +Additional parent node must be created for Exynos5440 power domains with
> > > > +the following required properties:
> > > > +- compatible: samsung,exynos5440-power-domains, simple-bus
> > > > +- reg: physical base address of the XMU controller and length of memory
> > > > +    mapped region
> > > 
> > > It's a little odd to describe the child nodes first. Given the 4210 and
> > > 5440 bindings work so differently, I'd suggest making separate binding
> > > files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt.
> 
> OK, I'll fix it.
> 
> > > The node being describe here clearly is not a simple-bus; the child
> > > nodes appear to have a specific need that their parent be compatible =
> > > "samsung,exynos5440-power-domains", hence they aren't the independent
> > > devices that simple-bus would usually contain.
> > 
> > +1. This is most definitely not a simple-bus, the child nodes reg
> > properties cdon't represent the same address space as the
> > parent's reg property.
> 
> What does in mean in the practice? What should I do instead?

The driver for the parent nodes should probe for the child nodes via
some mechanism. The "simple-bus" compatible property should be removed
from the parent node as it's simply not true.

Simple-bus should only be used where the child nodes make sense and are
useable on their own, regardless of the parent node.

> 
> > How much does the association of bit-offsets to power domains vary? 
> 
> On EXYNOS5440 power domains are controlled by control and status registers
> which are shared among all power domains. On other EXYNOS SoCs there are
> separate control/status registers for each power domain.

Ok.

> 
> > > Note that I only briefly reviewed the low-level structural aspects of
> > > the binding that I mentioned above; I haven't thought about the binding
> > > at a higher level of e.g. "does this binding conceptually make sense".
> > 
> > This is unfortunately difficult to do :(
> > 
> > > 
> > > >  Node of a device using power domains must have a samsung,power-domain property
> > > >  defined with a phandle to respective power domain.
> > > >  
> > > > -Example:
> > > > +Example for Exynos4210 compatible power domain:
> > > >  
> > > >  	lcd0: power-domain-lcd0 {
> > > >  		compatible = "samsung,exynos4210-pd";
> > > >  		reg = <0x10023C00 0x10>;
> > > >  	};
> > > >  
> > > > +Example for Exynos5440 compatible power domains:
> > > > +
> > > > +	power-domains@00160000 {
> > > > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > > > +		reg = <0x00160000 0x1000>;
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		pd_pcie: pcie-power-domain@6 {
> > > > +			compatible = "samsung,exynos5440-pd";
> > > > +			reg = <6>;
> > > > +		};
> > > > +
> > > > +		pd_conn2: conn2-power-domain@7 {
> > > > +			compatible = "samsung,exynos5440-pd";
> > > > +			reg = <7>;
> > > > +		};
> > > > +	};
> > > > +
> > > >  Example of the node using power domain:
> > > >  
> > > >  	node {
> > > > Index: b/arch/arm/boot/dts/exynos5440.dtsi
> > > > ===================================================================
> > > > --- a/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:45:53.599392397 +0200
> > > > +++ b/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:46:29.815391842 +0200
> > > > @@ -29,6 +29,23 @@
> > > >  		#clock-cells = <1>;
> > > >  	};
> > > >  
> > > > +	power-domains@00160000 {
> > > > +		compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > > > +		reg = <0x00160000 0x1000>;
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		pd_pcie: pcie-power-domain@6 {
> > > > +			compatible = "samsung,exynos5440-pd";
> > > > +			reg = <6>;
> > > > +		};
> > > > +
> > > > +		pd_conn2: conn2-power-domain@7 {
> > > > +			compatible = "samsung,exynos5440-pd";
> > > > +			reg = <7>;
> > > > +		};
> > > > +	};
> > > > +
> > > >  	gic:interrupt-controller@2E0000 {
> > > >  		compatible = "arm,cortex-a15-gic";
> > > >  		#interrupt-cells = <3>;
> > > > @@ -192,6 +209,7 @@
> > > >  		phy-mode = "sgmii";
> > > >  		clocks = <&clock 25>;
> > > >  		clock-names = "stmmaceth";
> > > > +		samsung,power-domain = <&pd_conn2>;
> > > >  	};
> > 
> > This feels like a clock style binding would fit better:
> > samsung,power-domain = <&pwr 7>;
> > 
> > But there may be a better, more general way of handling this. I'm not
> > sure how this relates to regulators and so on, and how other vendors
> > handle this stuff...
> 
> Other vendors don't implement device tree for their power domains yet.
> 
> Also there is not much power domain users in the tree currently:
> 
> $ git grep pm_genpd_init
> arch/arm/mach-exynos/pm_domains.c:          pm_genpd_init(&pd->pd, NULL, !on);
> arch/arm/mach-s3c64xx/pm.c:         pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
> arch/arm/mach-s3c64xx/pm.c:         pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
> arch/arm/mach-shmobile/pm-r8a7779.c:        pm_genpd_init(genpd, NULL, false);
> arch/arm/mach-shmobile/pm-rmobile.c:        pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
> drivers/base/power/domain.c: * pm_genpd_init - Initialize a generic I/O PM domain object.
> drivers/base/power/domain.c:void pm_genpd_init(struct generic_pm_domain *genpd,
> include/linux/pm_domain.h:extern void pm_genpd_init(struct generic_pm_domain *genpd,
> include/linux/pm_domain.h:static inline void pm_genpd_init(struct generic_pm_domain *genpd,

Ok. How do you feel about for clock style binding I suggested above? Is
there any information you might need to membed in teh child *-pd nodes
in future?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
===================================================================
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:45:53.551392396 +0200
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt	2013-08-02 14:46:29.799391845 +0200
@@ -5,20 +5,47 @@  to gate power to one or more peripherals
 
 Required Properties:
 - compatible: should be one of the following.
-    * samsung,exynos4210-pd - for exynos4210 type power domain.
+    * samsung,exynos4210-pd - for Exynos4210 type power domain.
+    * samsung,exynos5440-pd - for Exynos5440 type power domain.
 - reg: physical base address of the controller and length of memory mapped
-    region.
+    region (Exynos4210 type power domain) or bit offset in the control
+    register (Exynos5440 type power domain).
+
+Additional parent node must be created for Exynos5440 power domains with
+the following required properties:
+- compatible: samsung,exynos5440-power-domains, simple-bus
+- reg: physical base address of the XMU controller and length of memory
+    mapped region
 
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
 
-Example:
+Example for Exynos4210 compatible power domain:
 
 	lcd0: power-domain-lcd0 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10023C00 0x10>;
 	};
 
+Example for Exynos5440 compatible power domains:
+
+	power-domains@00160000 {
+		compatible = "samsung,exynos5440-power-domains", "simple-bus";
+		reg = <0x00160000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pd_pcie: pcie-power-domain@6 {
+			compatible = "samsung,exynos5440-pd";
+			reg = <6>;
+		};
+
+		pd_conn2: conn2-power-domain@7 {
+			compatible = "samsung,exynos5440-pd";
+			reg = <7>;
+		};
+	};
+
 Example of the node using power domain:
 
 	node {
Index: b/arch/arm/boot/dts/exynos5440.dtsi
===================================================================
--- a/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:45:53.599392397 +0200
+++ b/arch/arm/boot/dts/exynos5440.dtsi	2013-08-02 14:46:29.815391842 +0200
@@ -29,6 +29,23 @@ 
 		#clock-cells = <1>;
 	};
 
+	power-domains@00160000 {
+		compatible = "samsung,exynos5440-power-domains", "simple-bus";
+		reg = <0x00160000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pd_pcie: pcie-power-domain@6 {
+			compatible = "samsung,exynos5440-pd";
+			reg = <6>;
+		};
+
+		pd_conn2: conn2-power-domain@7 {
+			compatible = "samsung,exynos5440-pd";
+			reg = <7>;
+		};
+	};
+
 	gic:interrupt-controller@2E0000 {
 		compatible = "arm,cortex-a15-gic";
 		#interrupt-cells = <3>;
@@ -192,6 +209,7 @@ 
 		phy-mode = "sgmii";
 		clocks = <&clock 25>;
 		clock-names = "stmmaceth";
+		samsung,power-domain = <&pd_conn2>;
 	};
 
 	amba {
@@ -240,6 +258,7 @@ 
 		interrupts = <0 30 0>;
 		clocks = <&clock 23>;
 		clock-names = "sata";
+		samsung,power-domain = <&pd_conn2>;
 	};
 
 	ohci@220000 {
@@ -248,6 +267,7 @@ 
 		interrupts = <0 29 0>;
 		clocks = <&clock 24>;
 		clock-names = "usbhost";
+		samsung,power-domain = <&pd_conn2>;
 	};
 
 	ehci@221000 {
@@ -256,6 +276,7 @@ 
 		interrupts = <0 29 0>;
 		clocks = <&clock 24>;
 		clock-names = "usbhost";
+		samsung,power-domain = <&pd_conn2>;
 	};
 
 	pcie@290000 {
@@ -275,6 +296,7 @@ 
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 53>;
+		samsung,power-domain = <&pd_pcie>;
 	};
 
 	pcie@2a0000 {
@@ -294,5 +316,6 @@ 
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 56>;
+		samsung,power-domain = <&pd_pcie>;
 	};
 };
Index: b/arch/arm/mach-exynos/Kconfig
===================================================================
--- a/arch/arm/mach-exynos/Kconfig	2013-08-02 14:45:53.563392396 +0200
+++ b/arch/arm/mach-exynos/Kconfig	2013-08-02 14:46:29.815391842 +0200
@@ -104,6 +104,7 @@  config SOC_EXYNOS5440
 	select MIGHT_HAVE_PCI
 	select PCI_DOMAINS if PCI
 	select PINCTRL_EXYNOS5440
+	select PM_GENERIC_DOMAINS if PM
 	select PM_OPP
 	help
 	  Enable EXYNOS5440 SoC support
Index: b/arch/arm/mach-exynos/common.c
===================================================================
--- a/arch/arm/mach-exynos/common.c	2013-08-02 14:45:53.583392397 +0200
+++ b/arch/arm/mach-exynos/common.c	2013-08-02 14:46:29.815391842 +0200
@@ -296,10 +296,6 @@  void exynos5_restart(enum reboot_mode mo
 
 void __init exynos_init_late(void)
 {
-	if (of_machine_is_compatible("samsung,exynos5440"))
-		/* to be supported later */
-		return;
-
 	exynos_pm_late_initcall();
 }
 
Index: b/arch/arm/mach-exynos/pm_domains.c
===================================================================
--- a/arch/arm/mach-exynos/pm_domains.c	2013-08-02 14:45:53.571392397 +0200
+++ b/arch/arm/mach-exynos/pm_domains.c	2013-08-02 14:47:36.471390825 +0200
@@ -24,12 +24,14 @@ 
 
 #include <mach/regs-pmu.h>
 #include <plat/devs.h>
+#include <plat/cpu.h>
 
 /*
  * Exynos specific wrapper around the generic power domain
  */
 struct exynos_pm_domain {
 	void __iomem *base;
+	unsigned int bit_offset;
 	char const *name;
 	bool is_off;
 	struct generic_pm_domain pd;
@@ -74,6 +76,75 @@  static int exynos_pd_power_off(struct ge
 	return exynos_pd_power(domain, false);
 }
 
+#define EXYNOS5440_PMU_STATUS	0x00BC
+#define EXYNOS5440_PMU_CTRL	0x00C0
+
+static int exynos5440_check_status(struct exynos_pm_domain *pd, u32 *pwr)
+{
+	u32 timeout = 10; /* Wait max 1ms */
+	int ret = 0;
+
+	do {
+		*pwr = readl(pd->base + EXYNOS5440_PMU_STATUS);
+		if ((*pwr & BIT(pd->bit_offset + 16)) == 0)
+			break;
+		if (!timeout) {
+			pr_err("%s: power domain %s status read failed\n",
+			       __func__, pd->name);
+			ret = -ETIMEDOUT;
+			break;
+		}
+		timeout--;
+		usleep_range(80, 100);
+	} while (1);
+
+	return ret;
+}
+
+static int exynos5440_pd_power(struct generic_pm_domain *domain,
+			       bool power_on)
+{
+	struct exynos_pm_domain *pd =
+			container_of(domain, struct exynos_pm_domain, pd);
+	void __iomem *base = pd->base;
+	unsigned int bit_offset = pd->bit_offset;
+	u32 pwr;
+	int ret;
+
+	pwr = readl(base + EXYNOS5440_PMU_CTRL);
+	if (power_on)
+		pwr |= BIT(bit_offset);
+	else
+		pwr |= BIT(bit_offset + 16);
+	writel(pwr, base + EXYNOS5440_PMU_CTRL);
+
+	ret = exynos5440_check_status(pd, &pwr);
+	if (ret)
+		goto err;
+
+	if ((power_on && (pwr & BIT(bit_offset)) == 0) ||
+	    (!power_on && (pwr & BIT(bit_offset)))) {
+		ret = -EIO;
+		goto err;
+	}
+
+	return 0;
+err:
+	pr_err("Power domain %s %s failed (error=%d)\n",
+	       domain->name, power_on ? "enable" : "disable", ret);
+	return ret;
+}
+
+static int exynos5440_pd_power_on(struct generic_pm_domain *domain)
+{
+	return exynos5440_pd_power(domain, true);
+}
+
+static int exynos5440_pd_power_off(struct generic_pm_domain *domain)
+{
+	return exynos5440_pd_power(domain, false);
+}
+
 static void exynos_add_device_to_domain(struct exynos_pm_domain *pd,
 					 struct device *dev)
 {
@@ -146,7 +217,7 @@  static struct notifier_block platform_nb
 	.notifier_call = exynos_pm_notifier_call,
 };
 
-static __init int exynos4_pm_init_power_domain(void)
+static __init int exynos4210_pm_init_power_domain(void)
 {
 	struct platform_device *pdev;
 	struct device_node *np;
@@ -182,7 +253,70 @@  static __init int exynos4_pm_init_power_
 
 	return 0;
 }
-arch_initcall(exynos4_pm_init_power_domain);
+
+static __init int exynos5440_pm_init_power_domain(void)
+{
+	struct device_node *np, *node = NULL;
+	void __iomem *reg_base;
+
+	np = of_find_compatible_node(NULL, NULL,
+				    "samsung,exynos5440-power-domains");
+	if (!np)
+		return 0;
+
+	reg_base = of_iomap(np, 0);
+
+	for_each_child_of_node(np, node) {
+		struct platform_device *pdev;
+		struct exynos_pm_domain *pd;
+		u32 pwr;
+		int ret, on;
+
+		pdev = of_find_device_by_node(node);
+		if (pdev == NULL) {
+			pr_err("%s: failed to get platform device for domain\n",
+			       __func__);
+			continue;
+		}
+
+		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			pr_err("%s: failed to allocate memory for domain\n",
+			       __func__);
+			return -ENOMEM;
+		}
+
+		pd->pd.name = kstrdup(node->name, GFP_KERNEL);
+		pd->name = pd->pd.name;
+		pd->base = reg_base;
+		if (of_property_read_u32(node, "reg", &pd->bit_offset))
+			pr_err("%s: bit offset not specified for domain %s\n",
+			       __func__, pd->name);
+		pd->pd.power_off = exynos5440_pd_power_off;
+		pd->pd.power_on = exynos5440_pd_power_on;
+		pd->pd.of_node = node;
+
+		platform_set_drvdata(pdev, pd);
+
+		ret = exynos5440_check_status(pd, &pwr);
+		on = ret ? 0 : !!(pwr & BIT(pd->bit_offset));
+
+		pm_genpd_init(&pd->pd, NULL, !on);
+	}
+
+	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	return 0;
+}
+
+static __init int exynos_pm_init_power_domain(void)
+{
+	if (soc_is_exynos5440())
+		return exynos5440_pm_init_power_domain();
+	else
+		return exynos4210_pm_init_power_domain();
+}
+arch_initcall(exynos_pm_init_power_domain);
 
 int __init exynos_pm_late_initcall(void)
 {