Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440

2013-08-08 Thread Mark Rutland
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 
> > > > Signed-off-by: Myungjoo Ham 
> > > > Cc: Tomasz Figa 
> > > > Cc: Stephen Warren 
> > > > ---
> > > >  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

Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440

2013-08-06 Thread Bartlomiej Zolnierkiewicz

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 
> > > Signed-off-by: Myungjoo Ham 
> > > Cc: Tomasz Figa 
> > > Cc: Stephen Warren 
> > > ---
> > >  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@0016 {
> > > + compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > > + reg = <0x0016 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + pd_

Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440

2013-08-03 Thread Mark Rutland
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 
> > Signed-off-by: Myungjoo Ham 
> > Cc: Tomasz Figa 
> > Cc: Stephen Warren 
> > ---
> >  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@0016 {
> > +   compatible = "samsung,exynos5440-power-domains", "simple-bus";
> > +   reg = <0x0016 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/b

Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440

2013-08-02 Thread Stephen Warren
(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 
> Signed-off-by: Myungjoo Ham 
> Cc: Tomasz Figa 
> Cc: Stephen Warren 
> ---
>  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@0016 {
> + compatible = "samsung,exynos5440-power-domains", "simple-bus";
> + reg = <0x0016 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@0016 {
> + compatible = "samsung,exynos5440-power-domains", "simple-bus";
> + reg = <0x0016 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pd_pcie: pcie-po