| Message ID | 2729097.X0eAsuYF97@amdc1032 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
(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>; > }; > };
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>; > > }; > > }; > >
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>; > > > }; > > > };
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.
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) {