[PATCH 0/6 V2] arm: omap: usb: Runtime PM support for EHCI and OHCI drivers

[PATCH 0/6 V2] arm: omap: usb: Runtime PM support for EHCI and OHCI drivers

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

The Hwmod structures and Runtime PM features are implemented
For EHCI and OHCI drivers of OMAP3 and OMAP4.
The global suspend/resume of EHCI and OHCI
is validated on OMAP3430 sdp board with these patches.

Benoit Cousson (1):
  arm: omap: usb: ehci and ohci hwmod structures for omap4

Keshava Munegowda (5):
  arm: omap: usb: ehci and ohci hwmod structures for omap3
  arm: omap: usb: register hwmods of usbhs
  arm: omap: usb: device name change for the clk names of usbhs
  arm: omap: usb: Runtime PM support
  arm: omap: usb: global Suspend and resume support of ehci and ohci

 arch/arm/mach-omap2/clock3xxx_data.c       |   28 +-
 arch/arm/mach-omap2/clock44xx_data.c       |   10 +-
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  279 +++++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  235 +++++++++++
 arch/arm/mach-omap2/usb-host.c             |  113 +++---
 arch/arm/plat-omap/include/plat/usb.h      |    3 -
 drivers/mfd/omap-usb-host.c                |  620 +++++++++++++---------------
 drivers/usb/host/ehci-omap.c               |   36 ++-
 drivers/usb/host/ohci-omap3.c              |   33 ++-
 9 files changed, 914 insertions(+), 443 deletions(-)

[PATCH 1/6 v2] arm: omap: usb: ehci and ohci hwmod structures for omap4

[Keshava Munegowda]

From: Benoit Cousson <b-cousson@ti.com>

Following 4 hwmod strcuture are added:
UHH hwmod of usbhs with uhh base address and functional clock,
EHCI hwmod with irq and base address,
OHCI hwmod with irq and base address,
TLL hwmod of usbhs with the TLL base address and irq.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  235 ++++++++++++++++++++++++++++
 1 files changed, 235 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index e1c69ff..3968b32 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -66,6 +66,10 @@ static struct omap_hwmod omap44xx_mmc2_hwmod;
 static struct omap_hwmod omap44xx_mpu_hwmod;
 static struct omap_hwmod omap44xx_mpu_private_hwmod;
 static struct omap_hwmod omap44xx_usb_otg_hs_hwmod;
+static struct omap_hwmod omap44xx_usb_host_hs_hwmod;
+static struct omap_hwmod omap44xx_usbhs_ohci_hwmod;
+static struct omap_hwmod omap44xx_usbhs_ehci_hwmod;
+static struct omap_hwmod omap44xx_usb_tll_hs_hwmod;
 
 /*
  * Interconnects omap_hwmod structures
@@ -5027,6 +5031,233 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
+/*
+ * 'usb_host_hs' class
+ * high-speed multi-port usb host controller
+ */
+static struct omap_hwmod_ocp_if omap44xx_usb_host_hs__l3_main_2 = {
+	.master		= &omap44xx_usb_host_hs_hwmod,
+	.slave		= &omap44xx_l3_main_2_hwmod,
+	.clk		= "l3_div_ck",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_class_sysconfig omap44xx_usb_host_hs_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.syss_offs	= 0x0014,
+	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
+			   MSTANDBY_FORCE | MSTANDBY_NO | MSTANDBY_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class omap44xx_usb_host_hs_hwmod_class = {
+	.name = "usbhs_uhh",
+	.sysc = &omap44xx_usb_host_hs_sysc,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_masters[] = {
+	&omap44xx_usb_host_hs__l3_main_2,
+};
+
+static struct omap_hwmod_addr_space omap44xx_usb_host_hs_addrs[] = {
+	{
+		.name		= "uhh",
+		.pa_start	= 0x4a064000,
+		.pa_end		= 0x4a0647ff,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usb_host_hs = {
+	.master		= &omap44xx_l4_cfg_hwmod,
+	.slave		= &omap44xx_usb_host_hs_hwmod,
+	.clk		= "l4_div_ck",
+	.addr		= omap44xx_usb_host_hs_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_usb_host_hs_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usb_host_hs_slaves[] = {
+	&omap44xx_l4_cfg__usb_host_hs,
+};
+
+static struct omap_hwmod omap44xx_usb_host_hs_hwmod = {
+	.name		= "usbhs_uhh",
+	.class		= &omap44xx_usb_host_hs_hwmod_class,
+	.main_clk	= "usb_host_hs_fck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_reg = OMAP4430_CM_L3INIT_USB_HOST_CLKCTRL,
+		},
+	},
+	.slaves		= omap44xx_usb_host_hs_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usb_host_hs_slaves),
+	.masters	= omap44xx_usb_host_hs_masters,
+	.masters_cnt	= ARRAY_SIZE(omap44xx_usb_host_hs_masters),
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
+/* 'usbhs_ohci' class */
+static struct omap_hwmod_class omap44xx_usbhs_ohci_hwmod_class = {
+	.name = "usbhs_ohci",
+};
+
+static struct omap_hwmod_irq_info omap44xx_usbhs_ohci_irqs[] = {
+	{ .name = "ohci-irq", .irq = 76 + OMAP44XX_IRQ_GIC_START },
+};
+
+static struct omap_hwmod_addr_space omap44xx_usbhs_ohci_addrs[] = {
+	{
+		.name		= "ohci",
+		.pa_start	= 0x4A064800,
+		.pa_end		= 0x4A064BFF,
+		.flags		= ADDR_MAP_ON_INIT
+	}
+};
+
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usbhs_ohci = {
+	.master		= &omap44xx_l4_cfg_hwmod,
+	.slave		= &omap44xx_usbhs_ohci_hwmod,
+	.clk		= "l4_div_ck",
+	.addr		= omap44xx_usbhs_ohci_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_usbhs_ohci_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usbhs_ohci_slaves[] = {
+	&omap44xx_l4_cfg__usbhs_ohci,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usbhs_ohci_masters[] = {
+	&omap44xx_usb_host_hs__l3_main_2,
+};
+
+static struct omap_hwmod omap44xx_usbhs_ohci_hwmod = {
+	.name		= "usbhs_ohci",
+	.class		= &omap44xx_usbhs_ohci_hwmod_class,
+	.mpu_irqs	= omap44xx_usbhs_ohci_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_usbhs_ohci_irqs),
+	.slaves		= omap44xx_usbhs_ohci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usbhs_ohci_slaves),
+	.masters	= omap44xx_usbhs_ohci_masters,
+	.masters_cnt	= ARRAY_SIZE(omap44xx_usbhs_ohci_masters),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+};
+
+/* 'usbhs_ehci' class */
+static struct omap_hwmod_class omap44xx_usbhs_ehci_hwmod_class = {
+	.name = "usbhs_ehci",
+};
+
+static struct omap_hwmod_irq_info omap44xx_usbhs_ehci_irqs[] = {
+	{ .name = "ehci-irq", .irq = 77 + OMAP44XX_IRQ_GIC_START },
+};
+
+static struct omap_hwmod_addr_space omap44xx_usbhs_ehci_addrs[] = {
+	{
+		.name		= "ehci",
+		.pa_start	= 0x4A064C00,
+		.pa_end		= 0x4A064FFF,
+		.flags		= ADDR_MAP_ON_INIT
+	}
+};
+
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usbhs_ehci = {
+	.master		= &omap44xx_l4_cfg_hwmod,
+	.slave		= &omap44xx_usbhs_ehci_hwmod,
+	.clk		= "l4_div_ck",
+	.addr		= omap44xx_usbhs_ehci_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_usbhs_ehci_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usbhs_ehci_slaves[] = {
+	&omap44xx_l4_cfg__usbhs_ehci,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usbhs_ehci_masters[] = {
+	&omap44xx_usb_host_hs__l3_main_2,
+};
+
+
+static struct omap_hwmod omap44xx_usbhs_ehci_hwmod = {
+	.name		= "usbhs_ehci",
+	.class		= &omap44xx_usbhs_ehci_hwmod_class,
+	.mpu_irqs	= omap44xx_usbhs_ehci_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_usbhs_ehci_irqs),
+	.slaves		= omap44xx_usbhs_ehci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usbhs_ehci_slaves),
+	.masters	= omap44xx_usbhs_ehci_masters,
+	.masters_cnt	= ARRAY_SIZE(omap44xx_usbhs_ehci_masters),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+};
+
+/*
+ * 'usb_tll_hs' class
+ * usb_tll_hs module is the adapter on the usb_host_hs ports
+ */
+static struct omap_hwmod_class_sysconfig omap44xx_usb_tll_hs_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.syss_offs	= 0x0014,
+	.sysc_flags	= (SYSC_HAS_AUTOIDLE | SYSC_HAS_SIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_usb_tll_hs_hwmod_class = {
+	.name = "usbhs_tll",
+	.sysc = &omap44xx_usb_tll_hs_sysc,
+};
+
+static struct omap_hwmod_irq_info omap44xx_usb_tll_hs_irqs[] = {
+	{ .name = "tll-irq", .irq = 78 + OMAP44XX_IRQ_GIC_START },
+};
+
+static struct omap_hwmod_addr_space omap44xx_usb_tll_hs_addrs[] = {
+	{
+		.name		= "tll",
+		.pa_start	= 0x4a062000,
+		.pa_end		= 0x4a063fff,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__usb_tll_hs = {
+	.master		= &omap44xx_l4_cfg_hwmod,
+	.slave		= &omap44xx_usb_tll_hs_hwmod,
+	.clk		= "l4_div_ck",
+	.addr		= omap44xx_usb_tll_hs_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_usb_tll_hs_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap44xx_usb_tll_hs_slaves[] = {
+	&omap44xx_l4_cfg__usb_tll_hs,
+};
+
+static struct omap_hwmod omap44xx_usb_tll_hs_hwmod = {
+	.name		= "usbhs_tll",
+	.class		= &omap44xx_usb_tll_hs_hwmod_class,
+	.mpu_irqs	= omap44xx_usb_tll_hs_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_usb_tll_hs_irqs),
+	.main_clk	= "usb_tll_hs_ick",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_reg = OMAP4430_CM_L3INIT_USB_TLL_CLKCTRL,
+		},
+	},
+	.slaves		= omap44xx_usb_tll_hs_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usb_tll_hs_slaves),
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
 static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* dmm class */
@@ -5173,6 +5404,10 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	&omap44xx_wd_timer2_hwmod,
 	&omap44xx_wd_timer3_hwmod,
 
+	&omap44xx_usb_host_hs_hwmod,
+	&omap44xx_usbhs_ohci_hwmod,
+	&omap44xx_usbhs_ehci_hwmod,
+	&omap44xx_usb_tll_hs_hwmod,
 	NULL,
 };
 
-- 
1.6.0.4

[PATCH 2/6 v2] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

Following 4 hwmod strcuture are added:
UHH hwmod of usbhs with uhh base address and functional clock,
EHCI hwmod with irq and base address,
OHCI hwmod with irq and base address,
TLL hwmod of usbhs with the TLL base address and irq.

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  279 ++++++++++++++++++++++++++++
 1 files changed, 279 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 909a84d..8d1d4f5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -84,6 +84,10 @@ static struct omap_hwmod omap3xxx_mcbsp4_hwmod;
 static struct omap_hwmod omap3xxx_mcbsp5_hwmod;
 static struct omap_hwmod omap3xxx_mcbsp2_sidetone_hwmod;
 static struct omap_hwmod omap3xxx_mcbsp3_sidetone_hwmod;
+static struct omap_hwmod omap34xx_usb_host_hs_hwmod;
+static struct omap_hwmod omap34xx_usbhs_ohci_hwmod;
+static struct omap_hwmod omap34xx_usbhs_ehci_hwmod;
+static struct omap_hwmod omap34xx_usb_tll_hs_hwmod;
 
 /* L3 -> L4_CORE interface */
 static struct omap_hwmod_ocp_if omap3xxx_l3_main__l4_core = {
@@ -3574,6 +3578,276 @@ static struct omap_hwmod omap3xxx_mmc3_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 };
 
+/*
+ * 'usb_host_hs' class
+ * high-speed multi-port usb host controller
+ */
+static struct omap_hwmod_ocp_if omap34xx_usb_host_hs__l3_main_2 = {
+	.master		= &omap34xx_usb_host_hs_hwmod,
+	.slave		= &omap3xxx_l3_main_hwmod,
+	.clk		= "core_l3_ick",
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_class_sysconfig omap34xx_usb_host_hs_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.syss_offs	= 0x0014,
+	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
+			   MSTANDBY_FORCE | MSTANDBY_NO | MSTANDBY_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap34xx_usb_host_hs_hwmod_class = {
+	.name = "usbhs_uhh",
+	.sysc = &omap34xx_usb_host_hs_sysc,
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usb_host_hs_masters[] = {
+	&omap34xx_usb_host_hs__l3_main_2,
+};
+
+static struct omap_hwmod_addr_space omap34xx_usb_host_hs_addrs[] = {
+	{
+		.name		= "uhh",
+		.pa_start	= 0x48064000,
+		.pa_end		= 0x480643ff,
+		.flags		= ADDR_TYPE_RT
+	}
+};
+
+static struct omap_hwmod_ocp_if omap34xx_l4_cfg__usb_host_hs = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap34xx_usb_host_hs_hwmod,
+	.clk		= "l4_ick",
+	.addr		= omap34xx_usb_host_hs_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap34xx_usb_host_hs_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
+	.clk		= "usbhost_120m_fck",
+	.user		= OCP_USER_MPU,
+	.flags		= OCPIF_SWSUP_IDLE,
+};
+
+static struct omap_hwmod_ocp_if omap34xx_f48m_cfg__usb_host_hs = {
+	.clk		= "usbhost_48m_fck",
+	.user		= OCP_USER_MPU,
+	.flags		= OCPIF_SWSUP_IDLE,
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usb_host_hs_slaves[] = {
+	&omap34xx_l4_cfg__usb_host_hs,
+	&omap34xx_f128m_cfg__usb_host_hs,
+	&omap34xx_f48m_cfg__usb_host_hs,
+};
+
+static struct omap_hwmod omap34xx_usb_host_hs_hwmod = {
+	.name		= "usbhs_uhh",
+	.class		= &omap34xx_usb_host_hs_hwmod_class,
+	.main_clk	= "usbhost_ick",
+	.prcm = {
+		.omap2 = {
+			.module_offs = OMAP3430ES2_USBHOST_MOD,
+			.prcm_reg_id = 1,
+			.module_bit = 0,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = 1,
+			.idlest_stdby_bit = 0,
+		},
+	},
+	.slaves		= omap34xx_usb_host_hs_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usb_host_hs_slaves),
+	.masters	= omap34xx_usb_host_hs_masters,
+	.masters_cnt	= ARRAY_SIZE(omap34xx_usb_host_hs_masters),
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/* 'usbhs_ohci' class  */
+static struct omap_hwmod_ocp_if omap34xx_usbhs_ohci__l3_main_2 = {
+	.master		= &omap34xx_usbhs_ohci_hwmod,
+	.slave		= &omap3xxx_l3_main_hwmod,
+	.clk		= "core_l3_ick",
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_class omap34xx_usbhs_ohci_hwmod_class = {
+	.name = "usbhs_ohci",
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usbhs_ohci_masters[] = {
+	&omap34xx_usbhs_ohci__l3_main_2,
+};
+
+static struct omap_hwmod_irq_info omap34xx_usbhs_ohci_irqs[] = {
+	{ .name = "ohci-irq", .irq = 76 },
+};
+
+static struct omap_hwmod_addr_space omap34xx_usbhs_ohci_addrs[] = {
+	{
+		.name		= "ohci",
+		.pa_start	= 0x48064400,
+		.pa_end		= 0x480647FF,
+		.flags		= ADDR_MAP_ON_INIT
+	}
+};
+
+static struct omap_hwmod_ocp_if omap34xx_l4_cfg__usbhs_ohci = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap34xx_usbhs_ohci_hwmod,
+	.clk		= "l4_ick",
+	.addr		= omap34xx_usbhs_ohci_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usbhs_ohci_slaves[] = {
+	&omap34xx_l4_cfg__usbhs_ohci,
+};
+
+static struct omap_hwmod omap34xx_usbhs_ohci_hwmod = {
+	.name		= "usbhs_ohci",
+	.class		= &omap34xx_usbhs_ohci_hwmod_class,
+	.mpu_irqs	= omap34xx_usbhs_ohci_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_irqs),
+	.slaves		= omap34xx_usbhs_ohci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_slaves),
+	.masters	= omap34xx_usbhs_ohci_masters,
+	.masters_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_masters),
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/* 'usbhs_ehci' class  */
+static struct omap_hwmod_ocp_if omap34xx_usbhs_ehci__l3_main_2 = {
+	.master		= &omap34xx_usbhs_ehci_hwmod,
+	.slave		= &omap3xxx_l3_main_hwmod,
+	.clk		= "core_l3_ick",
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_class omap34xx_usbhs_ehci_hwmod_class = {
+	.name = "usbhs_ehci",
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usbhs_ehci_masters[] = {
+	&omap34xx_usbhs_ehci__l3_main_2,
+};
+
+static struct omap_hwmod_irq_info omap34xx_usbhs_ehci_irqs[] = {
+	{ .name = "ehci-irq", .irq = 77 },
+};
+
+static struct omap_hwmod_addr_space omap34xx_usbhs_ehci_addrs[] = {
+	{
+		.name		= "ehci",
+		.pa_start	= 0x48064800,
+		.pa_end		= 0x48064CFF,
+		.flags		= ADDR_MAP_ON_INIT
+	}
+};
+
+static struct omap_hwmod_ocp_if omap34xx_l4_cfg__usbhs_ehci = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap34xx_usbhs_ehci_hwmod,
+	.clk		= "l4_ick",
+	.addr		= omap34xx_usbhs_ehci_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usbhs_ehci_slaves[] = {
+	&omap34xx_l4_cfg__usbhs_ehci,
+};
+
+static struct omap_hwmod omap34xx_usbhs_ehci_hwmod = {
+	.name		= "usbhs_ehci",
+	.class		= &omap34xx_usbhs_ehci_hwmod_class,
+	.mpu_irqs	= omap34xx_usbhs_ehci_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_irqs),
+	.slaves		= omap34xx_usbhs_ehci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_slaves),
+	.masters	= omap34xx_usbhs_ehci_masters,
+	.masters_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_masters),
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/*
+ * 'usb_tll_hs' class
+ * usb_tll_hs module is the adapter on the usb_host_hs ports
+ */
+static struct omap_hwmod_class_sysconfig omap34xx_usb_tll_hs_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.syss_offs	= 0x0014,
+	.sysc_flags	= (SYSC_HAS_AUTOIDLE | SYSC_HAS_SIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap34xx_usb_tll_hs_hwmod_class = {
+	.name = "usbhs_tll",
+	.sysc = &omap34xx_usb_tll_hs_sysc,
+};
+
+static struct omap_hwmod_irq_info omap34xx_usb_tll_hs_irqs[] = {
+	{ .name = "tll-irq", .irq = 78 },
+};
+
+static struct omap_hwmod_addr_space omap34xx_usb_tll_hs_addrs[] = {
+	{
+		.name		= "tll",
+		.pa_start	= 0x48062000,
+		.pa_end		= 0x48062fff,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+static struct omap_hwmod_ocp_if omap34xx_f_cfg__usb_tll_hs = {
+	.clk		= "usbtll_fck",
+	.user		= OCP_USER_MPU,
+	.flags		= OCPIF_SWSUP_IDLE,
+};
+
+static struct omap_hwmod_ocp_if omap34xx_l4_cfg__usb_tll_hs = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap34xx_usb_tll_hs_hwmod,
+	.clk		= "l4_ick",
+	.addr		= omap34xx_usb_tll_hs_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap34xx_usb_tll_hs_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_usb_tll_hs_slaves[] = {
+	&omap34xx_l4_cfg__usb_tll_hs,
+	&omap34xx_f_cfg__usb_tll_hs,
+};
+
+static struct omap_hwmod omap34xx_usb_tll_hs_hwmod = {
+	.name		= "usbhs_tll",
+	.class		= &omap34xx_usb_tll_hs_hwmod_class,
+	.mpu_irqs	= omap34xx_usb_tll_hs_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap34xx_usb_tll_hs_irqs),
+	.main_clk	= "usbtll_ick",
+	.prcm = {
+		.omap2 = {
+			.module_offs = CORE_MOD,
+			.prcm_reg_id = 3,
+			.module_bit = 2,
+			.idlest_reg_id = 3,
+			.idlest_idle_bit = 2,
+		},
+	},
+	.slaves		= omap34xx_usb_tll_hs_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usb_tll_hs_slaves),
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
 static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_l3_main_hwmod,
 	&omap3xxx_l4_core_hwmod,
@@ -3656,6 +3930,11 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	/* usbotg for am35x */
 	&am35xx_usbhsotg_hwmod,
 
+	&omap34xx_usb_host_hs_hwmod,
+	&omap34xx_usbhs_ohci_hwmod,
+	&omap34xx_usbhs_ehci_hwmod,
+	&omap34xx_usb_tll_hs_hwmod,
+
 	NULL,
 };
 
-- 
1.6.0.4

[PATCH 3/6 v2] arm: omap: usb: register hwmods of usbhs

[Keshava Munegowda]

The hwmod structure of uhh, ohci, ehci and tll are
retrived and registered with omap device

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
---
 arch/arm/mach-omap2/usb-host.c |  113 +++++++++++++++++----------------------
 1 files changed, 49 insertions(+), 64 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 89ae298..dea0eed 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,51 +28,30 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 #include <plat/usb.h>
+#include <plat/omap_device.h>
 
 #include "mux.h"
 
 #ifdef CONFIG_MFD_OMAP_USB_HOST
 
-#define OMAP_USBHS_DEVICE	"usbhs-omap"
-
-static struct resource usbhs_resources[] = {
-	{
-		.name	= "uhh",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "tll",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci-irq",
-		.flags	= IORESOURCE_IRQ,
-	},
-	{
-		.name	= "ohci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ohci-irq",
-		.flags	= IORESOURCE_IRQ,
-	}
-};
-
-static struct platform_device usbhs_device = {
-	.name		= OMAP_USBHS_DEVICE,
-	.id		= 0,
-	.num_resources	= ARRAY_SIZE(usbhs_resources),
-	.resource	= usbhs_resources,
-};
+#define OMAP_USBHS_DEVICE	"usbhs_omap"
+#define	USBHS_UHH_HWMODNAME	"usbhs_uhh"
+#define	USBHS_OHCI_HWMODNAME	"usbhs_ohci"
+#define USBHS_EHCI_HWMODNAME	"usbhs_ehci"
+#define USBHS_TLL_HWMODNAME	"usbhs_tll"
 
 static struct usbhs_omap_platform_data		usbhs_data;
 static struct ehci_hcd_omap_platform_data	ehci_data;
 static struct ohci_hcd_omap_platform_data	ohci_data;
 
+static struct omap_device_pm_latency omap_uhhtll_latency[] = {
+	  {
+		.deactivate_func = omap_device_idle_hwmods,
+		.activate_func	 = omap_device_enable_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	  },
+};
+
 /* MUX settings for EHCI pins */
 /*
  * setup_ehci_io_mux - initialize IO pad mux for USBHOST
@@ -508,7 +487,10 @@ static void setup_4430ohci_io_mux(const enum usbhs_omap_port_mode *port_mode)
 
 void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 {
-	int	i;
+	struct omap_hwmod	*oh[4];
+	struct omap_device	*od;
+	int			bus_id = -1;
+	int			i;
 
 	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
 		usbhs_data.port_mode[i] = pdata->port_mode[i];
@@ -523,44 +505,47 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 	usbhs_data.ohci_data = &ohci_data;
 
 	if (cpu_is_omap34xx()) {
-		usbhs_resources[0].start = OMAP34XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP34XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP34XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP34XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start	= OMAP34XX_EHCI_BASE;
-		usbhs_resources[2].end	= OMAP34XX_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = INT_34XX_EHCI_IRQ;
-		usbhs_resources[4].start	= OMAP34XX_OHCI_BASE;
-		usbhs_resources[4].end	= OMAP34XX_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = INT_34XX_OHCI_IRQ;
 		setup_ehci_io_mux(pdata->port_mode);
 		setup_ohci_io_mux(pdata->port_mode);
 	} else if (cpu_is_omap44xx()) {
-		usbhs_resources[0].start = OMAP44XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP44XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP44XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP44XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start = OMAP44XX_HSUSB_EHCI_BASE;
-		usbhs_resources[2].end = OMAP44XX_HSUSB_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = OMAP44XX_IRQ_EHCI;
-		usbhs_resources[4].start = OMAP44XX_HSUSB_OHCI_BASE;
-		usbhs_resources[4].end = OMAP44XX_HSUSB_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = OMAP44XX_IRQ_OHCI;
 		setup_4430ehci_io_mux(pdata->port_mode);
 		setup_4430ohci_io_mux(pdata->port_mode);
 	}
 
-	if (platform_device_add_data(&usbhs_device,
-				&usbhs_data, sizeof(usbhs_data)) < 0) {
-		printk(KERN_ERR "USBHS platform_device_add_data failed\n");
-		goto init_end;
+	oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
+	if (!oh[0]) {
+		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
+		return;
+	}
+
+	oh[1] = omap_hwmod_lookup(USBHS_OHCI_HWMODNAME);
+	if (!oh[1]) {
+		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
+		return;
 	}
 
-	if (platform_device_register(&usbhs_device) < 0)
-		printk(KERN_ERR "USBHS platform_device_register failed\n");
+	oh[2] = omap_hwmod_lookup(USBHS_EHCI_HWMODNAME);
+	if (!oh[2]) {
+		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
+		return;
+	}
 
-init_end:
-	return;
+	oh[3] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
+	if (!oh[3]) {
+		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
+		return;
+	}
+
+	od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 4,
+				(void *)&usbhs_data, sizeof(usbhs_data),
+				omap_uhhtll_latency,
+				ARRAY_SIZE(omap_uhhtll_latency), false);
+
+	if (IS_ERR(od)) {
+		pr_err("Could not build hwmod devices %s, %s\n",
+			USBHS_UHH_HWMODNAME, USBHS_TLL_HWMODNAME);
+		return;
+	}
 }
 
 #else
-- 
1.6.0.4

[PATCH 4/6 v2] arm: omap: usb: device name change for the clk names of usbhs

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

device name usbhs clocks are changed from
usbhs-omap.0 to usbhs_omap; this is because
in the hwmod registration the device name is set
as usbhs_omap

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
---
 arch/arm/mach-omap2/clock3xxx_data.c |   28 ++++++++++++++--------------
 arch/arm/mach-omap2/clock44xx_data.c |   10 +++++-----
 drivers/mfd/omap-usb-host.c          |    2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 75b119b..fabe482 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3285,7 +3285,7 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"cpefuse_fck",	&cpefuse_fck,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK(NULL,	"ts_fck",	&ts_fck,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK(NULL,	"usbtll_fck",	&usbtll_fck,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"usbtll_fck",	&usbtll_fck,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
+	CLK("usbhs_omap",	"usbtll_fck",	&usbtll_fck,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK("omap-mcbsp.1",	"prcm_fck",	&core_96m_fck,	CK_3XXX),
 	CLK("omap-mcbsp.5",	"prcm_fck",	&core_96m_fck,	CK_3XXX),
 	CLK(NULL,	"core_96m_fck",	&core_96m_fck,	CK_3XXX),
@@ -3321,7 +3321,7 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"pka_ick",	&pka_ick,	CK_34XX | CK_36XX),
 	CLK(NULL,	"core_l4_ick",	&core_l4_ick,	CK_3XXX),
 	CLK(NULL,	"usbtll_ick",	&usbtll_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"usbtll_ick",	&usbtll_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
+	CLK("usbhs_omap",	"usbtll_ick",	&usbtll_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK("omap_hsmmc.2",	"ick",	&mmchs3_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK(NULL,	"icr_ick",	&icr_ick,	CK_34XX | CK_36XX),
 	CLK("omap-aes",	"ick",	&aes2_ick,	CK_34XX | CK_36XX),
@@ -3367,20 +3367,20 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"cam_ick",	&cam_ick,	CK_34XX | CK_36XX),
 	CLK(NULL,	"csi2_96m_fck",	&csi2_96m_fck,	CK_34XX | CK_36XX),
 	CLK(NULL,	"usbhost_120m_fck", &usbhost_120m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"hs_fck", &usbhost_120m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
+	CLK("usbhs_omap",	"hs_fck", &usbhost_120m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK(NULL,	"usbhost_48m_fck", &usbhost_48m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"fs_fck", &usbhost_48m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
+	CLK("usbhs_omap",	"fs_fck", &usbhost_48m_fck, CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
 	CLK(NULL,	"usbhost_ick",	&usbhost_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"usbhost_ick",	&usbhost_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
-	CLK("usbhs-omap.0",	"utmi_p1_gfclk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"utmi_p2_gfclk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"xclk60mhsp1_ck",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"xclk60mhsp2_ck",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"usb_host_hs_utmi_p1_clk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"usb_host_hs_utmi_p2_clk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"usb_tll_hs_usb_ch0_clk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"usb_tll_hs_usb_ch1_clk",	&dummy_ck,	CK_3XXX),
-	CLK("usbhs-omap.0",	"init_60m_fclk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"usbhost_ick",	&usbhost_ick,	CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
+	CLK("usbhs_omap",	"utmi_p1_gfclk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"utmi_p2_gfclk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"xclk60mhsp1_ck",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"xclk60mhsp2_ck",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"usb_host_hs_utmi_p1_clk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"usb_host_hs_utmi_p2_clk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"usb_tll_hs_usb_ch0_clk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"usb_tll_hs_usb_ch1_clk",	&dummy_ck,	CK_3XXX),
+	CLK("usbhs_omap",	"init_60m_fclk",	&dummy_ck,	CK_3XXX),
 	CLK(NULL,	"usim_fck",	&usim_fck,	CK_3430ES2PLUS | CK_36XX),
 	CLK(NULL,	"gpt1_fck",	&gpt1_fck,	CK_3XXX),
 	CLK(NULL,	"wkup_32k_fck",	&wkup_32k_fck,	CK_3XXX),
diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 8c96567..34e91eb 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3205,7 +3205,7 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"uart3_fck",			&uart3_fck,	CK_443X),
 	CLK(NULL,	"uart4_fck",			&uart4_fck,	CK_443X),
 	CLK(NULL,	"usb_host_fs_fck",		&usb_host_fs_fck,	CK_443X),
-	CLK("usbhs-omap.0",	"fs_fck",		&usb_host_fs_fck,	CK_443X),
+	CLK("usbhs_omap",	"fs_fck",		&usb_host_fs_fck,	CK_443X),
 	CLK(NULL,	"utmi_p1_gfclk",		&utmi_p1_gfclk,	CK_443X),
 	CLK(NULL,	"usb_host_hs_utmi_p1_clk",	&usb_host_hs_utmi_p1_clk,	CK_443X),
 	CLK(NULL,	"utmi_p2_gfclk",		&utmi_p2_gfclk,	CK_443X),
@@ -3217,8 +3217,8 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"usb_host_hs_hsic480m_p2_clk",	&usb_host_hs_hsic480m_p2_clk,	CK_443X),
 	CLK(NULL,	"usb_host_hs_func48mclk",	&usb_host_hs_func48mclk,	CK_443X),
 	CLK(NULL,	"usb_host_hs_fck",		&usb_host_hs_fck,	CK_443X),
-	CLK("usbhs-omap.0",	"hs_fck",		&usb_host_hs_fck,	CK_443X),
-	CLK("usbhs-omap.0",	"usbhost_ick",		&dummy_ck,		CK_443X),
+	CLK("usbhs_omap",	"hs_fck",		&usb_host_hs_fck,	CK_443X),
+	CLK("usbhs_omap",	"usbhost_ick",		&dummy_ck,		CK_443X),
 	CLK(NULL,	"otg_60m_gfclk",		&otg_60m_gfclk,	CK_443X),
 	CLK(NULL,	"usb_otg_hs_xclk",		&usb_otg_hs_xclk,	CK_443X),
 	CLK("musb-omap2430",	"ick",				&usb_otg_hs_ick,	CK_443X),
@@ -3227,8 +3227,8 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"usb_tll_hs_usb_ch0_clk",	&usb_tll_hs_usb_ch0_clk,	CK_443X),
 	CLK(NULL,	"usb_tll_hs_usb_ch1_clk",	&usb_tll_hs_usb_ch1_clk,	CK_443X),
 	CLK(NULL,	"usb_tll_hs_ick",		&usb_tll_hs_ick,	CK_443X),
-	CLK("usbhs-omap.0",	"usbtll_ick",		&usb_tll_hs_ick,	CK_443X),
-	CLK("usbhs-omap.0",	"usbtll_fck",		&dummy_ck,	CK_443X),
+	CLK("usbhs_omap",	"usbtll_ick",		&usb_tll_hs_ick,	CK_443X),
+	CLK("usbhs_omap",	"usbtll_fck",		&dummy_ck,	CK_443X),
 	CLK(NULL,	"usim_ck",			&usim_ck,	CK_443X),
 	CLK(NULL,	"usim_fclk",			&usim_fclk,	CK_443X),
 	CLK(NULL,	"usim_fck",			&usim_fck,	CK_443X),
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 8552195..43de12a 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -28,7 +28,7 @@
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
 
-#define USBHS_DRIVER_NAME	"usbhs-omap"
+#define USBHS_DRIVER_NAME	"usbhs_omap"
 #define OMAP_EHCI_DEVICE	"ehci-omap"
 #define OMAP_OHCI_DEVICE	"ohci-omap3"
 
-- 
1.6.0.4

[PATCH 5/6 v2] arm: omap: usb: Runtime PM support

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

The usbhs core driver does not enable/disable the intefrace and
fucntional clocks; These clocks are handled by hwmod and runtime pm,
hence insted of the clock enable/disable, the runtime pm APIS are
used. however,the port clocks and tll clocks are handled
by the usbhs core.
In this architecture, the usbhs is core driver and it is parent of
ehci and ohci drivers.The earlier APIs omap_usbhs_enable and
omap_usbhs_disable of usbhs core driver are removed;
The ehci and ohci drivers call the pm_runtime_get_sync and
pm_runtime_put_sync of parent device usbhs core.
In fact, the runtime framework takes care the get sync and
put sync of the child in turn call the get sync and put sync
of parent too; but calling get sync and put sync of parent is
by ASYNC mode; This mode queues the work item in runtime pm
work queue, which not getting scheduled in case of global suspend path.
so, here ehci and ohci runtim pm is not required, these
drivers will pass the parent usbhs dev pointer in runtime pm APIs.

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
---
 arch/arm/plat-omap/include/plat/usb.h |    3 -
 drivers/mfd/omap-usb-host.c           |  618 +++++++++++++++------------------
 drivers/usb/host/ehci-omap.c          |   14 +-
 drivers/usb/host/ohci-omap3.c         |   12 +-
 4 files changed, 290 insertions(+), 357 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 17d3c93..2b66dc2 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -100,9 +100,6 @@ extern void usb_musb_init(struct omap_musb_board_data *board_data);
 
 extern void usbhs_init(const struct usbhs_omap_board_data *pdata);
 
-extern int omap_usbhs_enable(struct device *dev);
-extern void omap_usbhs_disable(struct device *dev);
-
 extern int omap4430_phy_power(struct device *dev, int ID, int on);
 extern int omap4430_phy_set_clk(struct device *dev, int on);
 extern int omap4430_phy_init(struct device *dev);
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 43de12a..521ee86 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -164,7 +164,6 @@ struct usbhs_hcd_omap {
 
 	u32				usbhs_rev;
 	spinlock_t			lock;
-	int				count;
 };
 /*-------------------------------------------------------------------------*/
 
@@ -314,218 +313,6 @@ err_end:
 	return ret;
 }
 
-/**
- * usbhs_omap_probe - initialize TI-based HCDs
- *
- * Allocates basic resources for this USB host controller.
- */
-static int __devinit usbhs_omap_probe(struct platform_device *pdev)
-{
-	struct device			*dev =  &pdev->dev;
-	struct usbhs_omap_platform_data	*pdata = dev->platform_data;
-	struct usbhs_hcd_omap		*omap;
-	struct resource			*res;
-	int				ret = 0;
-	int				i;
-
-	if (!pdata) {
-		dev_err(dev, "Missing platform data\n");
-		ret = -ENOMEM;
-		goto end_probe;
-	}
-
-	omap = kzalloc(sizeof(*omap), GFP_KERNEL);
-	if (!omap) {
-		dev_err(dev, "Memory allocation failed\n");
-		ret = -ENOMEM;
-		goto end_probe;
-	}
-
-	spin_lock_init(&omap->lock);
-
-	for (i = 0; i < OMAP3_HS_USB_PORTS; i++)
-		omap->platdata.port_mode[i] = pdata->port_mode[i];
-
-	omap->platdata.ehci_data = pdata->ehci_data;
-	omap->platdata.ohci_data = pdata->ohci_data;
-
-	pm_runtime_enable(&pdev->dev);
-
-	omap->utmi_p1_fck = clk_get(dev, "utmi_p1_gfclk");
-	if (IS_ERR(omap->utmi_p1_fck)) {
-		ret = PTR_ERR(omap->utmi_p1_fck);
-		dev_err(dev, "utmi_p1_gfclk failed error:%d\n",	ret);
-		goto err_end;
-	}
-
-	omap->xclk60mhsp1_ck = clk_get(dev, "xclk60mhsp1_ck");
-	if (IS_ERR(omap->xclk60mhsp1_ck)) {
-		ret = PTR_ERR(omap->xclk60mhsp1_ck);
-		dev_err(dev, "xclk60mhsp1_ck failed error:%d\n", ret);
-		goto err_utmi_p1_fck;
-	}
-
-	omap->utmi_p2_fck = clk_get(dev, "utmi_p2_gfclk");
-	if (IS_ERR(omap->utmi_p2_fck)) {
-		ret = PTR_ERR(omap->utmi_p2_fck);
-		dev_err(dev, "utmi_p2_gfclk failed error:%d\n", ret);
-		goto err_xclk60mhsp1_ck;
-	}
-
-	omap->xclk60mhsp2_ck = clk_get(dev, "xclk60mhsp2_ck");
-	if (IS_ERR(omap->xclk60mhsp2_ck)) {
-		ret = PTR_ERR(omap->xclk60mhsp2_ck);
-		dev_err(dev, "xclk60mhsp2_ck failed error:%d\n", ret);
-		goto err_utmi_p2_fck;
-	}
-
-	omap->usbhost_p1_fck = clk_get(dev, "usb_host_hs_utmi_p1_clk");
-	if (IS_ERR(omap->usbhost_p1_fck)) {
-		ret = PTR_ERR(omap->usbhost_p1_fck);
-		dev_err(dev, "usbhost_p1_fck failed error:%d\n", ret);
-		goto err_xclk60mhsp2_ck;
-	}
-
-	omap->usbtll_p1_fck = clk_get(dev, "usb_tll_hs_usb_ch0_clk");
-	if (IS_ERR(omap->usbtll_p1_fck)) {
-		ret = PTR_ERR(omap->usbtll_p1_fck);
-		dev_err(dev, "usbtll_p1_fck failed error:%d\n", ret);
-		goto err_usbhost_p1_fck;
-	}
-
-	omap->usbhost_p2_fck = clk_get(dev, "usb_host_hs_utmi_p2_clk");
-	if (IS_ERR(omap->usbhost_p2_fck)) {
-		ret = PTR_ERR(omap->usbhost_p2_fck);
-		dev_err(dev, "usbhost_p2_fck failed error:%d\n", ret);
-		goto err_usbtll_p1_fck;
-	}
-
-	omap->usbtll_p2_fck = clk_get(dev, "usb_tll_hs_usb_ch1_clk");
-	if (IS_ERR(omap->usbtll_p2_fck)) {
-		ret = PTR_ERR(omap->usbtll_p2_fck);
-		dev_err(dev, "usbtll_p2_fck failed error:%d\n", ret);
-		goto err_usbhost_p2_fck;
-	}
-
-	omap->init_60m_fclk = clk_get(dev, "init_60m_fclk");
-	if (IS_ERR(omap->init_60m_fclk)) {
-		ret = PTR_ERR(omap->init_60m_fclk);
-		dev_err(dev, "init_60m_fclk failed error:%d\n", ret);
-		goto err_usbtll_p2_fck;
-	}
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
-	if (!res) {
-		dev_err(dev, "UHH EHCI get resource failed\n");
-		ret = -ENODEV;
-		goto err_init_60m_fclk;
-	}
-
-	omap->uhh_base = ioremap(res->start, resource_size(res));
-	if (!omap->uhh_base) {
-		dev_err(dev, "UHH ioremap failed\n");
-		ret = -ENOMEM;
-		goto err_init_60m_fclk;
-	}
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tll");
-	if (!res) {
-		dev_err(dev, "UHH EHCI get resource failed\n");
-		ret = -ENODEV;
-		goto err_tll;
-	}
-
-	omap->tll_base = ioremap(res->start, resource_size(res));
-	if (!omap->tll_base) {
-		dev_err(dev, "TLL ioremap failed\n");
-		ret = -ENOMEM;
-		goto err_tll;
-	}
-
-	platform_set_drvdata(pdev, omap);
-
-	ret = omap_usbhs_alloc_children(pdev);
-	if (ret) {
-		dev_err(dev, "omap_usbhs_alloc_children failed\n");
-		goto err_alloc;
-	}
-
-	goto end_probe;
-
-err_alloc:
-	iounmap(omap->tll_base);
-
-err_tll:
-	iounmap(omap->uhh_base);
-
-err_init_60m_fclk:
-	clk_put(omap->init_60m_fclk);
-
-err_usbtll_p2_fck:
-	clk_put(omap->usbtll_p2_fck);
-
-err_usbhost_p2_fck:
-	clk_put(omap->usbhost_p2_fck);
-
-err_usbtll_p1_fck:
-	clk_put(omap->usbtll_p1_fck);
-
-err_usbhost_p1_fck:
-	clk_put(omap->usbhost_p1_fck);
-
-err_xclk60mhsp2_ck:
-	clk_put(omap->xclk60mhsp2_ck);
-
-err_utmi_p2_fck:
-	clk_put(omap->utmi_p2_fck);
-
-err_xclk60mhsp1_ck:
-	clk_put(omap->xclk60mhsp1_ck);
-
-err_utmi_p1_fck:
-	clk_put(omap->utmi_p1_fck);
-
-err_end:
-	pm_runtime_disable(&pdev->dev);
-	kfree(omap);
-
-end_probe:
-	return ret;
-}
-
-/**
- * usbhs_omap_remove - shutdown processing for UHH & TLL HCDs
- * @pdev: USB Host Controller being removed
- *
- * Reverses the effect of usbhs_omap_probe().
- */
-static int __devexit usbhs_omap_remove(struct platform_device *pdev)
-{
-	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
-
-	if (omap->count != 0) {
-		dev_err(&pdev->dev,
-			"Either EHCI or OHCI is still using usbhs core\n");
-		return -EBUSY;
-	}
-
-	iounmap(omap->tll_base);
-	iounmap(omap->uhh_base);
-	clk_put(omap->init_60m_fclk);
-	clk_put(omap->usbtll_p2_fck);
-	clk_put(omap->usbhost_p2_fck);
-	clk_put(omap->usbtll_p1_fck);
-	clk_put(omap->usbhost_p1_fck);
-	clk_put(omap->xclk60mhsp2_ck);
-	clk_put(omap->utmi_p2_fck);
-	clk_put(omap->xclk60mhsp1_ck);
-	clk_put(omap->utmi_p1_fck);
-	pm_runtime_disable(&pdev->dev);
-	kfree(omap);
-
-	return 0;
-}
-
 static bool is_ohci_port(enum usbhs_omap_port_mode pmode)
 {
 	switch (pmode) {
@@ -634,26 +421,73 @@ static void usbhs_omap_tll_init(struct device *dev, u8 tll_channel_count)
 	}
 }
 
-static int usbhs_enable(struct device *dev)
+static int usbhs_runtime_resume(struct device *dev)
 {
 	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
 	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
-	unsigned long			flags = 0;
-	int				ret = 0;
-	unsigned			reg;
 
-	dev_dbg(dev, "starting TI HSUSB Controller\n");
+	dev_dbg(dev, "usbhs_runtime_resume\n");
+
 	if (!pdata) {
 		dev_dbg(dev, "missing platform_data\n");
 		return  -ENODEV;
 	}
 
-	spin_lock_irqsave(&omap->lock, flags);
-	if (omap->count > 0)
-		goto end_count;
+	if (is_omap_usbhs_rev2(omap)) {
+		if (is_ehci_tll_mode(pdata->port_mode[0])) {
+			clk_enable(omap->usbhost_p1_fck);
+			clk_enable(omap->usbtll_p1_fck);
+		}
+		if (is_ehci_tll_mode(pdata->port_mode[1])) {
+			clk_enable(omap->usbhost_p2_fck);
+			clk_enable(omap->usbtll_p2_fck);
+		}
+		clk_enable(omap->utmi_p1_fck);
+		clk_enable(omap->utmi_p2_fck);
+	}
+	return 0;
+}
+
+static int usbhs_runtime_suspend(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	dev_dbg(dev, "usbhs_runtime_suspend\n");
+
+	if (!pdata) {
+		dev_dbg(dev, "missing platform_data\n");
+		return  -ENODEV;
+	}
+
+	if (is_omap_usbhs_rev2(omap)) {
+		if (is_ehci_tll_mode(pdata->port_mode[0])) {
+			clk_disable(omap->usbhost_p1_fck);
+			clk_disable(omap->usbtll_p1_fck);
+		}
+		if (is_ehci_tll_mode(pdata->port_mode[1])) {
+			clk_disable(omap->usbhost_p2_fck);
+			clk_disable(omap->usbtll_p2_fck);
+		}
+		clk_disable(omap->utmi_p2_fck);
+		clk_disable(omap->utmi_p1_fck);
+	}
+	return 0;
+}
+
+static void omap_usbhs_init(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+	unsigned long			flags = 0;
+	unsigned			reg;
+
+	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
 
+	spin_lock_irqsave(&omap->lock, flags);
+
 	if (pdata->ehci_data->phy_reset) {
 		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0])) {
 			gpio_request(pdata->ehci_data->reset_gpio_port[0],
@@ -676,6 +510,14 @@ static int usbhs_enable(struct device *dev)
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
+	/*
+	 * Really enable the port clocks
+	 * first call of pm_runtime_get_sync does not enable these
+	 * port clocks; because omap->usbhs_rev was aviable
+	 * This omap->usbhs_rev is aviable now!
+	 */
+	usbhs_runtime_resume(dev);
+
 	reg = usbhs_read(omap->uhh_base, OMAP_UHH_HOSTCONFIG);
 	/* setup ULPI bypass and burst configurations */
 	reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN
@@ -721,49 +563,6 @@ static int usbhs_enable(struct device *dev)
 		reg &= ~OMAP4_P1_MODE_CLEAR;
 		reg &= ~OMAP4_P2_MODE_CLEAR;
 
-		if (is_ehci_phy_mode(pdata->port_mode[0])) {
-			ret = clk_set_parent(omap->utmi_p1_fck,
-						omap->xclk60mhsp1_ck);
-			if (ret != 0) {
-				dev_err(dev, "xclk60mhsp1_ck set parent"
-				"failed error:%d\n", ret);
-				goto err_tll;
-			}
-		} else if (is_ehci_tll_mode(pdata->port_mode[0])) {
-			ret = clk_set_parent(omap->utmi_p1_fck,
-						omap->init_60m_fclk);
-			if (ret != 0) {
-				dev_err(dev, "init_60m_fclk set parent"
-				"failed error:%d\n", ret);
-				goto err_tll;
-			}
-			clk_enable(omap->usbhost_p1_fck);
-			clk_enable(omap->usbtll_p1_fck);
-		}
-
-		if (is_ehci_phy_mode(pdata->port_mode[1])) {
-			ret = clk_set_parent(omap->utmi_p2_fck,
-						omap->xclk60mhsp2_ck);
-			if (ret != 0) {
-				dev_err(dev, "xclk60mhsp1_ck set parent"
-					"failed error:%d\n", ret);
-				goto err_tll;
-			}
-		} else if (is_ehci_tll_mode(pdata->port_mode[1])) {
-			ret = clk_set_parent(omap->utmi_p2_fck,
-						omap->init_60m_fclk);
-			if (ret != 0) {
-				dev_err(dev, "init_60m_fclk set parent"
-				"failed error:%d\n", ret);
-				goto err_tll;
-			}
-			clk_enable(omap->usbhost_p2_fck);
-			clk_enable(omap->usbtll_p2_fck);
-		}
-
-		clk_enable(omap->utmi_p1_fck);
-		clk_enable(omap->utmi_p2_fck);
-
 		if (is_ehci_tll_mode(pdata->port_mode[0]) ||
 			(is_ohci_port(pdata->port_mode[0])))
 			reg |= OMAP4_P1_MODE_TLL;
@@ -809,14 +608,17 @@ static int usbhs_enable(struct device *dev)
 				(pdata->ehci_data->reset_gpio_port[1], 1);
 	}
 
-end_count:
-	omap->count++;
 	spin_unlock_irqrestore(&omap->lock, flags);
-	return 0;
-
-err_tll:
 	pm_runtime_put_sync(dev);
-	spin_unlock_irqrestore(&omap->lock, flags);
+}
+
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	dev_dbg(dev, "stopping TI HSUSB Controller\n");
+
 	if (pdata->ehci_data->phy_reset) {
 		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
 			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
@@ -824,112 +626,256 @@ err_tll:
 		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
 			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
 	}
-	return ret;
 }
 
-static void usbhs_disable(struct device *dev)
+/**
+ * usbhs_omap_probe - initialize TI-based HCDs
+ *
+ * Allocates basic resources for this USB host controller.
+ */
+static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 {
-	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
-	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
-	unsigned long			flags = 0;
-	unsigned long			timeout;
+	struct device			*dev =  &pdev->dev;
+	struct usbhs_omap_platform_data	*pdata = dev->platform_data;
+	struct usbhs_hcd_omap		*omap;
+	struct resource			*res;
+	int				ret = 0;
+	int				i;
 
-	dev_dbg(dev, "stopping TI HSUSB Controller\n");
+	if (!pdata) {
+		dev_err(dev, "Missing platform data\n");
+		ret = -ENOMEM;
+		goto end_probe;
+	}
 
-	spin_lock_irqsave(&omap->lock, flags);
+	omap = kzalloc(sizeof(*omap), GFP_KERNEL);
+	if (!omap) {
+		dev_err(dev, "Memory allocation failed\n");
+		ret = -ENOMEM;
+		goto end_probe;
+	}
 
-	if (omap->count == 0)
-		goto end_disble;
+	spin_lock_init(&omap->lock);
 
-	omap->count--;
+	for (i = 0; i < OMAP3_HS_USB_PORTS; i++)
+		omap->platdata.port_mode[i] = pdata->port_mode[i];
 
-	if (omap->count != 0)
-		goto end_disble;
+	omap->platdata.ehci_data = pdata->ehci_data;
+	omap->platdata.ohci_data = pdata->ohci_data;
 
-	/* Reset OMAP modules for insmod/rmmod to work */
-	usbhs_write(omap->uhh_base, OMAP_UHH_SYSCONFIG,
-			is_omap_usbhs_rev2(omap) ?
-			OMAP4_UHH_SYSCONFIG_SOFTRESET :
-			OMAP_UHH_SYSCONFIG_SOFTRESET);
+	pm_runtime_enable(dev);
 
-	timeout = jiffies + msecs_to_jiffies(100);
-	while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
-				& (1 << 0))) {
-		cpu_relax();
+	omap->utmi_p1_fck = clk_get(dev, "utmi_p1_gfclk");
+	if (IS_ERR(omap->utmi_p1_fck)) {
+		ret = PTR_ERR(omap->utmi_p1_fck);
+		dev_err(dev, "utmi_p1_gfclk failed error:%d\n",	ret);
+		goto err_end;
+	}
 
-		if (time_after(jiffies, timeout))
-			dev_dbg(dev, "operation timed out\n");
+	omap->xclk60mhsp1_ck = clk_get(dev, "xclk60mhsp1_ck");
+	if (IS_ERR(omap->xclk60mhsp1_ck)) {
+		ret = PTR_ERR(omap->xclk60mhsp1_ck);
+		dev_err(dev, "xclk60mhsp1_ck failed error:%d\n", ret);
+		goto err_utmi_p1_fck;
 	}
 
-	while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
-				& (1 << 1))) {
-		cpu_relax();
+	omap->utmi_p2_fck = clk_get(dev, "utmi_p2_gfclk");
+	if (IS_ERR(omap->utmi_p2_fck)) {
+		ret = PTR_ERR(omap->utmi_p2_fck);
+		dev_err(dev, "utmi_p2_gfclk failed error:%d\n", ret);
+		goto err_xclk60mhsp1_ck;
+	}
 
-		if (time_after(jiffies, timeout))
-			dev_dbg(dev, "operation timed out\n");
+	omap->xclk60mhsp2_ck = clk_get(dev, "xclk60mhsp2_ck");
+	if (IS_ERR(omap->xclk60mhsp2_ck)) {
+		ret = PTR_ERR(omap->xclk60mhsp2_ck);
+		dev_err(dev, "xclk60mhsp2_ck failed error:%d\n", ret);
+		goto err_utmi_p2_fck;
 	}
 
-	while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
-				& (1 << 2))) {
-		cpu_relax();
+	omap->usbhost_p1_fck = clk_get(dev, "usb_host_hs_utmi_p1_clk");
+	if (IS_ERR(omap->usbhost_p1_fck)) {
+		ret = PTR_ERR(omap->usbhost_p1_fck);
+		dev_err(dev, "usbhost_p1_fck failed error:%d\n", ret);
+		goto err_xclk60mhsp2_ck;
+	}
 
-		if (time_after(jiffies, timeout))
-			dev_dbg(dev, "operation timed out\n");
+	omap->usbtll_p1_fck = clk_get(dev, "usb_tll_hs_usb_ch0_clk");
+	if (IS_ERR(omap->usbtll_p1_fck)) {
+		ret = PTR_ERR(omap->usbtll_p1_fck);
+		dev_err(dev, "usbtll_p1_fck failed error:%d\n", ret);
+		goto err_usbhost_p1_fck;
 	}
 
-	usbhs_write(omap->tll_base, OMAP_USBTLL_SYSCONFIG, (1 << 1));
+	omap->usbhost_p2_fck = clk_get(dev, "usb_host_hs_utmi_p2_clk");
+	if (IS_ERR(omap->usbhost_p2_fck)) {
+		ret = PTR_ERR(omap->usbhost_p2_fck);
+		dev_err(dev, "usbhost_p2_fck failed error:%d\n", ret);
+		goto err_usbtll_p1_fck;
+	}
 
-	while (!(usbhs_read(omap->tll_base, OMAP_USBTLL_SYSSTATUS)
-				& (1 << 0))) {
-		cpu_relax();
+	omap->usbtll_p2_fck = clk_get(dev, "usb_tll_hs_usb_ch1_clk");
+	if (IS_ERR(omap->usbtll_p2_fck)) {
+		ret = PTR_ERR(omap->usbtll_p2_fck);
+		dev_err(dev, "usbtll_p2_fck failed error:%d\n", ret);
+		goto err_usbhost_p2_fck;
+	}
 
-		if (time_after(jiffies, timeout))
-			dev_dbg(dev, "operation timed out\n");
+	omap->init_60m_fclk = clk_get(dev, "init_60m_fclk");
+	if (IS_ERR(omap->init_60m_fclk)) {
+		ret = PTR_ERR(omap->init_60m_fclk);
+		dev_err(dev, "init_60m_fclk failed error:%d\n", ret);
+		goto err_usbtll_p2_fck;
 	}
 
-	if (is_omap_usbhs_rev2(omap)) {
-		if (is_ehci_tll_mode(pdata->port_mode[0]))
-			clk_enable(omap->usbtll_p1_fck);
-		if (is_ehci_tll_mode(pdata->port_mode[1]))
-			clk_enable(omap->usbtll_p2_fck);
-		clk_disable(omap->utmi_p2_fck);
-		clk_disable(omap->utmi_p1_fck);
+	if (is_ehci_phy_mode(pdata->port_mode[0])) {
+		/* for OMAP3 , the clk set paretn fails */
+		ret = clk_set_parent(omap->utmi_p1_fck,
+					omap->xclk60mhsp1_ck);
+		if (ret != 0)
+			dev_err(dev, "xclk60mhsp1_ck set parent"
+				"failed error:%d\n", ret);
+	} else if (is_ehci_tll_mode(pdata->port_mode[0])) {
+		ret = clk_set_parent(omap->utmi_p1_fck,
+					omap->init_60m_fclk);
+		if (ret != 0)
+			dev_err(dev, "init_60m_fclk set parent"
+				"failed error:%d\n", ret);
 	}
 
-	pm_runtime_put_sync(dev);
+	if (is_ehci_phy_mode(pdata->port_mode[1])) {
+		ret = clk_set_parent(omap->utmi_p2_fck,
+					omap->xclk60mhsp2_ck);
+		if (ret != 0)
+			dev_err(dev, "xclk60mhsp2_ck set parent"
+					"failed error:%d\n", ret);
+	} else if (is_ehci_tll_mode(pdata->port_mode[1])) {
+		ret = clk_set_parent(omap->utmi_p2_fck,
+						omap->init_60m_fclk);
+		if (ret != 0)
+			dev_err(dev, "init_60m_fclk set parent"
+				"failed error:%d\n", ret);
+	}
 
-	/* The gpio_free migh sleep; so unlock the spinlock */
-	spin_unlock_irqrestore(&omap->lock, flags);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "uhh");
+	if (!res) {
+		dev_err(dev, "UHH EHCI get resource failed\n");
+		ret = -ENODEV;
+		goto err_init_60m_fclk;
+	}
 
-	if (pdata->ehci_data->phy_reset) {
-		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
-			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+	omap->uhh_base = ioremap(res->start, resource_size(res));
+	if (!omap->uhh_base) {
+		dev_err(dev, "UHH ioremap failed\n");
+		ret = -ENOMEM;
+		goto err_init_60m_fclk;
+	}
 
-		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
-			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tll");
+	if (!res) {
+		dev_err(dev, "UHH EHCI get resource failed\n");
+		ret = -ENODEV;
+		goto err_tll;
 	}
-	return;
 
-end_disble:
-	spin_unlock_irqrestore(&omap->lock, flags);
-}
+	omap->tll_base = ioremap(res->start, resource_size(res));
+	if (!omap->tll_base) {
+		dev_err(dev, "TLL ioremap failed\n");
+		ret = -ENOMEM;
+		goto err_tll;
+	}
 
-int omap_usbhs_enable(struct device *dev)
-{
-	return  usbhs_enable(dev->parent);
+	platform_set_drvdata(pdev, omap);
+
+	ret = omap_usbhs_alloc_children(pdev);
+	if (ret) {
+		dev_err(dev, "omap_usbhs_alloc_children failed\n");
+		goto err_alloc;
+	}
+
+	omap_usbhs_init(dev);
+
+	goto end_probe;
+
+err_alloc:
+	iounmap(omap->tll_base);
+
+err_tll:
+	iounmap(omap->uhh_base);
+
+err_init_60m_fclk:
+	clk_put(omap->init_60m_fclk);
+
+err_usbtll_p2_fck:
+	clk_put(omap->usbtll_p2_fck);
+
+err_usbhost_p2_fck:
+	clk_put(omap->usbhost_p2_fck);
+
+err_usbtll_p1_fck:
+	clk_put(omap->usbtll_p1_fck);
+
+err_usbhost_p1_fck:
+	clk_put(omap->usbhost_p1_fck);
+
+err_xclk60mhsp2_ck:
+	clk_put(omap->xclk60mhsp2_ck);
+
+err_utmi_p2_fck:
+	clk_put(omap->utmi_p2_fck);
+
+err_xclk60mhsp1_ck:
+	clk_put(omap->xclk60mhsp1_ck);
+
+err_utmi_p1_fck:
+	clk_put(omap->utmi_p1_fck);
+
+err_end:
+	pm_runtime_disable(dev);
+	kfree(omap);
+
+end_probe:
+	return ret;
 }
-EXPORT_SYMBOL_GPL(omap_usbhs_enable);
 
-void omap_usbhs_disable(struct device *dev)
+/**
+ * usbhs_omap_remove - shutdown processing for UHH & TLL HCDs
+ * @pdev: USB Host Controller being removed
+ *
+ * Reverses the effect of usbhs_omap_probe().
+ */
+static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
-	usbhs_disable(dev->parent);
+	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
+
+	omap_usbhs_deinit(&pdev->dev);
+	iounmap(omap->tll_base);
+	iounmap(omap->uhh_base);
+	clk_put(omap->init_60m_fclk);
+	clk_put(omap->usbtll_p2_fck);
+	clk_put(omap->usbhost_p2_fck);
+	clk_put(omap->usbtll_p1_fck);
+	clk_put(omap->usbhost_p1_fck);
+	clk_put(omap->xclk60mhsp2_ck);
+	clk_put(omap->utmi_p2_fck);
+	clk_put(omap->xclk60mhsp1_ck);
+	clk_put(omap->utmi_p1_fck);
+	pm_runtime_disable(&pdev->dev);
+	kfree(omap);
+
+	return 0;
 }
-EXPORT_SYMBOL_GPL(omap_usbhs_disable);
+
+static const struct dev_pm_ops usbhsomap_dev_pm_ops = {
+	.runtime_suspend	= usbhs_runtime_suspend,
+	.runtime_resume		= usbhs_runtime_resume,
+};
 
 static struct platform_driver usbhs_omap_driver = {
 	.driver = {
 		.name		= (char *)usbhs_driver_name,
 		.owner		= THIS_MODULE,
+		.pm		= &usbhsomap_dev_pm_ops,
 	},
 	.remove		= __exit_p(usbhs_omap_remove),
 };
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 55a57c2..178f63e 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -41,6 +41,7 @@
 #include <linux/usb/ulpi.h>
 #include <plat/usb.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
 
 /* EHCI Register Set */
 #define EHCI_INSNREG04					(0xA0)
@@ -178,11 +179,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = omap_usbhs_enable(dev);
-	if (ret) {
-		dev_err(dev, "failed to start usbhs with err %d\n", ret);
-		goto err_enable;
-	}
+	pm_runtime_get_sync(dev->parent);
 
 	/*
 	 * An undocumented "feature" in the OMAP3 EHCI controller,
@@ -228,10 +225,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
-	omap_usbhs_disable(dev);
-
-err_enable:
-	usb_put_hcd(hcd);
+	pm_runtime_put_sync(dev->parent);
 
 err_io:
 	return ret;
@@ -252,7 +246,7 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 	struct usb_hcd *hcd	= dev_get_drvdata(dev);
 
 	usb_remove_hcd(hcd);
-	omap_usbhs_disable(dev);
+	pm_runtime_put_sync(dev->parent);
 	usb_put_hcd(hcd);
 	return 0;
 }
diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c
index 6048f2f..6ce50de 100644
--- a/drivers/usb/host/ohci-omap3.c
+++ b/drivers/usb/host/ohci-omap3.c
@@ -31,6 +31,7 @@
 
 #include <linux/platform_device.h>
 #include <plat/usb.h>
+#include <linux/pm_runtime.h>
 
 /*-------------------------------------------------------------------------*/
 
@@ -172,11 +173,7 @@ static int __devinit ohci_hcd_omap3_probe(struct platform_device *pdev)
 	hcd->rsrc_len = resource_size(res);
 	hcd->regs =  regs;
 
-	ret = omap_usbhs_enable(dev);
-	if (ret) {
-		dev_dbg(dev, "failed to start ohci\n");
-		goto err_end;
-	}
+	pm_runtime_get_sync(dev->parent);
 
 	ohci_hcd_init(hcd_to_ohci(hcd));
 
@@ -189,7 +186,7 @@ static int __devinit ohci_hcd_omap3_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
-	omap_usbhs_disable(dev);
+	pm_runtime_get_sync(dev->parent);
 
 err_end:
 	usb_put_hcd(hcd);
@@ -220,9 +217,8 @@ static int __devexit ohci_hcd_omap3_remove(struct platform_device *pdev)
 
 	iounmap(hcd->regs);
 	usb_remove_hcd(hcd);
-	omap_usbhs_disable(dev);
+	pm_runtime_put_sync(dev->parent);
 	usb_put_hcd(hcd);
-
 	return 0;
 }
 
-- 
1.6.0.4

[PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

The global suspend and resume functions for ehci and ohci
drivers are implemented; these functions does the
pm_runtime_get_sync and pm_runtime_put_sync of the
parent device usbhs core driver respectively.

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
---
 drivers/usb/host/ehci-omap.c  |   22 ++++++++++++++++++++--
 drivers/usb/host/ohci-omap3.c |   21 +++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 178f63e..a02a684 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
+static int omap_ehci_resume(struct device *dev)
+{
+	if (dev->parent)
+		pm_runtime_get_sync(dev->parent);
+	return 0;
+}
+
+static int omap_ehci_suspend(struct device *dev)
+{
+	if (dev->parent)
+		pm_runtime_put_sync(dev->parent);
+	return 0;
+}
+
+static const struct dev_pm_ops omap_ehci_dev_pm_ops = {
+	.suspend	= omap_ehci_suspend,
+	.resume		= omap_ehci_resume,
+};
+
 static struct platform_driver ehci_hcd_omap_driver = {
 	.probe			= ehci_hcd_omap_probe,
 	.remove			= ehci_hcd_omap_remove,
 	.shutdown		= ehci_hcd_omap_shutdown,
-	/*.suspend		= ehci_hcd_omap_suspend, */
-	/*.resume		= ehci_hcd_omap_resume, */
 	.driver = {
 		.name		= "ehci-omap",
+		.pm		= &omap_ehci_dev_pm_ops,
 	}
 };
 
diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c
index 6ce50de..a1a7981 100644
--- a/drivers/usb/host/ohci-omap3.c
+++ b/drivers/usb/host/ohci-omap3.c
@@ -230,12 +230,33 @@ static void ohci_hcd_omap3_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
+
+static int omap_ohci_resume(struct device *dev)
+{
+	if (dev->parent)
+		pm_runtime_get_sync(dev->parent);
+	return 0;
+}
+
+static int omap_ohci_suspend(struct device *dev)
+{
+	if (dev->parent)
+		pm_runtime_put_sync(dev->parent);
+	return 0;
+}
+
+static const struct dev_pm_ops omap_ohci_dev_pm_ops = {
+	.suspend	= omap_ohci_suspend,
+	.resume		= omap_ohci_resume,
+};
+
 static struct platform_driver ohci_hcd_omap3_driver = {
 	.probe		= ohci_hcd_omap3_probe,
 	.remove		= __devexit_p(ohci_hcd_omap3_remove),
 	.shutdown	= ohci_hcd_omap3_shutdown,
 	.driver		= {
 		.name	= "ohci-omap3",
+		.pm		= &omap_ohci_dev_pm_ops,
 	},
 };
 
-- 
1.6.0.4

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Sat, 2 Jul 2011, Keshava Munegowda wrote:

> From: Keshava Munegowda <Keshava_mgowda@ti.com>
> 
> The global suspend and resume functions for ehci and ohci
> drivers are implemented; these functions does the
> pm_runtime_get_sync and pm_runtime_put_sync of the
> parent device usbhs core driver respectively.
> 
> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> ---
>  drivers/usb/host/ehci-omap.c  |   22 ++++++++++++++++++++--
>  drivers/usb/host/ohci-omap3.c |   21 +++++++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 178f63e..a02a684 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct platform_device *pdev)
>  		hcd->driver->shutdown(hcd);
>  }
>  
> +static int omap_ehci_resume(struct device *dev)
> +{
> +	if (dev->parent)
> +		pm_runtime_get_sync(dev->parent);
> +	return 0;
> +}
> +
> +static int omap_ehci_suspend(struct device *dev)
> +{
> +	if (dev->parent)
> +		pm_runtime_put_sync(dev->parent);
> +	return 0;
> +}

I don't see any point in these routines (and likewise for
omap_ohci_suspend/resume).  When the whole system is going to sleep
anyway, what reason is there for enabling runtime PM on the parent
device?

Alan Stern

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak]

>-----Original Message-----
>From: Alan Stern [mailto:stern@rowland.harvard.edu]
>Sent: Saturday, July 02, 2011 12:37 AM
>To: Keshava Munegowda
>Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>kernel@vger.kernel.org; balbi@ti.com; gadiyar@ti.com;
>sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
>khilman@ti.com; b-cousson@ti.com; paul@pwsan.com; johnstul@us.ibm.com;
>vishwanath.bs@ti.com
>Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>support of ehci and ohci
>
>On Sat, 2 Jul 2011, Keshava Munegowda wrote:
>
>> From: Keshava Munegowda <Keshava_mgowda@ti.com>
>>
>> The global suspend and resume functions for ehci and ohci
>> drivers are implemented; these functions does the
>> pm_runtime_get_sync and pm_runtime_put_sync of the
>> parent device usbhs core driver respectively.
>>
>> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> ---
>>  drivers/usb/host/ehci-omap.c  |   22 ++++++++++++++++++++--
>>  drivers/usb/host/ohci-omap3.c |   21 +++++++++++++++++++++
>>  2 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-
>omap.c
>> index 178f63e..a02a684 100644
>> --- a/drivers/usb/host/ehci-omap.c
>> +++ b/drivers/usb/host/ehci-omap.c
>> @@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct
>platform_device *pdev)
>>  		hcd->driver->shutdown(hcd);
>>  }
>>
>> +static int omap_ehci_resume(struct device *dev)
>> +{
>> +	if (dev->parent)
>> +		pm_runtime_get_sync(dev->parent);
>> +	return 0;
>> +}
>> +
>> +static int omap_ehci_suspend(struct device *dev)
>> +{
>> +	if (dev->parent)
>> +		pm_runtime_put_sync(dev->parent);
>> +	return 0;
>> +}
>
>I don't see any point in these routines (and likewise for
>omap_ohci_suspend/resume).  When the whole system is going to sleep
>anyway, what reason is there for enabling runtime PM on the parent
>device?

Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).

Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
will turn-off the parent clocks in the Suspend path.

Similarly, calling pm_runtime_get_sync(dev->parent) within
omap_ehci_resume
will turn-on the parent clocks in the resume path.

This way, all reference counting are implicit within the Runtime PM layer
and takes care of all combinations of only EHCI insmoded, OHCI insmoded,
both insmoded etc.

When both EHCI & OHCI are suspended, parent clocks will actually be
turned OFF and vice-versa.

Note that the parent per-se does not have any .suspend & .resume hooked
up.
At the end of the _probe of parent, the clocks are turned OFF.
Subsequently, enabling
the parent clocks are entirely done implicitly by the children get_sync()
in their _probe.

Therefore while .suspend/.resume of children are called they call back
into the parent to turn-off the clocks.

FYI, Keshava is out of office due to some emergency this week,
there will be some delays in replying from his side.

>
>Alan Stern

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

Hi,

On Mon, Jul 04, 2011 at 10:36:54AM +0530, Partha Basak wrote:
> >-----Original Message-----
> >From: Alan Stern [mailto:stern@rowland.harvard.edu]
> >Sent: Saturday, July 02, 2011 12:37 AM
> >To: Keshava Munegowda
> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
> >kernel@vger.kernel.org; balbi@ti.com; gadiyar@ti.com;
> >sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
> >khilman@ti.com; b-cousson@ti.com; paul@pwsan.com; johnstul@us.ibm.com;
> >vishwanath.bs@ti.com
> >Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
> >support of ehci and ohci
> >
> >On Sat, 2 Jul 2011, Keshava Munegowda wrote:
> >
> >> From: Keshava Munegowda <Keshava_mgowda@ti.com>
> >>
> >> The global suspend and resume functions for ehci and ohci
> >> drivers are implemented; these functions does the
> >> pm_runtime_get_sync and pm_runtime_put_sync of the
> >> parent device usbhs core driver respectively.
> >>
> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> >> ---
> >>  drivers/usb/host/ehci-omap.c  |   22 ++++++++++++++++++++--
> >>  drivers/usb/host/ohci-omap3.c |   21 +++++++++++++++++++++
> >>  2 files changed, 41 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-
> >omap.c
> >> index 178f63e..a02a684 100644
> >> --- a/drivers/usb/host/ehci-omap.c
> >> +++ b/drivers/usb/host/ehci-omap.c
> >> @@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct
> >platform_device *pdev)
> >>  		hcd->driver->shutdown(hcd);
> >>  }
> >>
> >> +static int omap_ehci_resume(struct device *dev)
> >> +{
> >> +	if (dev->parent)
> >> +		pm_runtime_get_sync(dev->parent);
> >> +	return 0;
> >> +}
> >> +
> >> +static int omap_ehci_suspend(struct device *dev)
> >> +{
> >> +	if (dev->parent)
> >> +		pm_runtime_put_sync(dev->parent);
> >> +	return 0;
> >> +}
> >
> >I don't see any point in these routines (and likewise for
> >omap_ohci_suspend/resume).  When the whole system is going to sleep
> >anyway, what reason is there for enabling runtime PM on the parent
> >device?
> 
> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
> 
> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
> will turn-off the parent clocks in the Suspend path.
> 
> Similarly, calling pm_runtime_get_sync(dev->parent) within
> omap_ehci_resume
> will turn-on the parent clocks in the resume path.
> 
> This way, all reference counting are implicit within the Runtime PM layer
> and takes care of all combinations of only EHCI insmoded, OHCI insmoded,
> both insmoded etc.
> 
> When both EHCI & OHCI are suspended, parent clocks will actually be
> turned OFF and vice-versa.

not sure this is necessary. I would expect:

pm_runtime_get_sync(dev) to propagate up the parent tree and enable all
necessary resources to get the child in a working state. IOW, you
shouldn't need to manuall access the parent device.

Kevin ? Paul ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak]

>-----Original Message-----
>From: Felipe Balbi [mailto:balbi@ti.com]
>Sent: Monday, July 04, 2011 1:56 PM
>To: Partha Basak
>Cc: Alan Stern; Keshava Munegowda; linux-usb@vger.kernel.org; linux-
>omap@vger.kernel.org; linux-kernel@vger.kernel.org; Felipe Balbi; Anand
>Gadiyar; sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
>Kevin Hilman; Benoit Cousson; paul@pwsan.com; johnstul@us.ibm.com;
>Vishwanath Sripathy
>Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>support of ehci and ohci
>
>Hi,
>
>On Mon, Jul 04, 2011 at 10:36:54AM +0530, Partha Basak wrote:
>> >-----Original Message-----
>> >From: Alan Stern [mailto:stern@rowland.harvard.edu]
>> >Sent: Saturday, July 02, 2011 12:37 AM
>> >To: Keshava Munegowda
>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>> >kernel@vger.kernel.org; balbi@ti.com; gadiyar@ti.com;
>> >sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
>> >khilman@ti.com; b-cousson@ti.com; paul@pwsan.com;
>> >johnstul@us.ibm.com; vishwanath.bs@ti.com
>> >Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>> >support of ehci and ohci
>> >
>> >On Sat, 2 Jul 2011, Keshava Munegowda wrote:
>> >
>> >> From: Keshava Munegowda <Keshava_mgowda@ti.com>
>> >>
>> >> The global suspend and resume functions for ehci and ohci drivers
>> >> are implemented; these functions does the pm_runtime_get_sync and
>> >> pm_runtime_put_sync of the parent device usbhs core driver
>> >> respectively.
>> >>
>> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> >> ---
>> >>  drivers/usb/host/ehci-omap.c  |   22 ++++++++++++++++++++--
>> >>  drivers/usb/host/ohci-omap3.c |   21 +++++++++++++++++++++
>> >>  2 files changed, 41 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-
>> >omap.c
>> >> index 178f63e..a02a684 100644
>> >> --- a/drivers/usb/host/ehci-omap.c
>> >> +++ b/drivers/usb/host/ehci-omap.c
>> >> @@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct
>> >platform_device *pdev)
>> >>  		hcd->driver->shutdown(hcd);
>> >>  }
>> >>
>> >> +static int omap_ehci_resume(struct device *dev) {
>> >> +	if (dev->parent)
>> >> +		pm_runtime_get_sync(dev->parent);
>> >> +	return 0;
>> >> +}
>> >> +
>> >> +static int omap_ehci_suspend(struct device *dev) {
>> >> +	if (dev->parent)
>> >> +		pm_runtime_put_sync(dev->parent);
>> >> +	return 0;
>> >> +}
>> >
>> >I don't see any point in these routines (and likewise for
>> >omap_ohci_suspend/resume).  When the whole system is going to sleep
>> >anyway, what reason is there for enabling runtime PM on the parent
>> >device?
>>
>> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
>>
>> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend will
>> turn-off the parent clocks in the Suspend path.
>>
>> Similarly, calling pm_runtime_get_sync(dev->parent) within
>> omap_ehci_resume will turn-on the parent clocks in the resume path.
>>
>> This way, all reference counting are implicit within the Runtime PM
>> layer and takes care of all combinations of only EHCI insmoded, OHCI
>> insmoded, both insmoded etc.
>>
>> When both EHCI & OHCI are suspended, parent clocks will actually be
>> turned OFF and vice-versa.
>
>not sure this is necessary. I would expect:
>
>pm_runtime_get_sync(dev) to propagate up the parent tree and enable all
>necessary resources to get the child in a working state. IOW, you
>shouldn't need to manuall access the parent device.
>
Refer to the description in Patch(5/6)
<snip>
In fact, the runtime framework takes care the get sync and put sync of the
child
in turn call the get sync and put sync of parent too; but calling get sync
and
put sync of parent is by ASYNC mode;
This mode queues the work item in runtime pm work queue,
which not getting scheduled in case of global suspend path.
<snip>
This approach was tried, but did not work in the Suspend path
static int rpm_suspend(struct device *dev, int rpmflags)
	__releases(&dev->power.lock) __acquires(&dev->power.lock)
{
.
.
.
no_callback:
.
.
.
	/* Maybe the parent is now able to suspend. */
	if (parent && !parent->power.ignore_children &&
!dev->power.irq_safe) {
		spin_unlock(&dev->power.lock);

		spin_lock(&parent->power.lock);
		rpm_idle(parent, RPM_ASYNC);
		spin_unlock(&parent->power.lock);

		spin_lock(&dev->power.lock);
	}
This is the reason of directly calling the parent Runtime PM calls from
the children.
If directly calling Runtime PM APIs with parent dev-pointer isn't
acceptable,
this can be achieved by exporting wrapper APIs from the
parent and calling them from the chidren .suspend/.resume routines.

>Kevin ? Paul ?
>
>--
>balbi

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]

Hi,

On Mon, Jul 04, 2011 at 02:56:30PM +0530, Partha Basak wrote:
> >> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
> >>
> >> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend will
> >> turn-off the parent clocks in the Suspend path.
> >>
> >> Similarly, calling pm_runtime_get_sync(dev->parent) within
> >> omap_ehci_resume will turn-on the parent clocks in the resume path.
> >>
> >> This way, all reference counting are implicit within the Runtime PM
> >> layer and takes care of all combinations of only EHCI insmoded, OHCI
> >> insmoded, both insmoded etc.
> >>
> >> When both EHCI & OHCI are suspended, parent clocks will actually be
> >> turned OFF and vice-versa.
> >
> >not sure this is necessary. I would expect:
> >
> >pm_runtime_get_sync(dev) to propagate up the parent tree and enable all
> >necessary resources to get the child in a working state. IOW, you
> >shouldn't need to manuall access the parent device.
> >
> Refer to the description in Patch(5/6)
> <snip>
> In fact, the runtime framework takes care the get sync and put sync of the
> child
> in turn call the get sync and put sync of parent too; but calling get sync
> and
> put sync of parent is by ASYNC mode;
> This mode queues the work item in runtime pm work queue,
> which not getting scheduled in case of global suspend path.
> <snip>
> This approach was tried, but did not work in the Suspend path

sounds to me like a bug on pm runtime ? If you're calling
pm_runtime_*_sync() family, shouldn't all calls be _sync() too ?

> static int rpm_suspend(struct device *dev, int rpmflags)
> 	__releases(&dev->power.lock) __acquires(&dev->power.lock)
> {
> .
> .
> .
> no_callback:
> .
> .
> .
> 	/* Maybe the parent is now able to suspend. */
> 	if (parent && !parent->power.ignore_children &&
> !dev->power.irq_safe) {
> 		spin_unlock(&dev->power.lock);
> 
> 		spin_lock(&parent->power.lock);
> 		rpm_idle(parent, RPM_ASYNC);

to me this is bogus, if you called pm_runtime_put_sync() should should
be sync too. Shouldn't it ?

> 		spin_unlock(&parent->power.lock);
> 
> 		spin_lock(&dev->power.lock);
> 	}
> This is the reason of directly calling the parent Runtime PM calls from
> the children.
> If directly calling Runtime PM APIs with parent dev-pointer isn't
> acceptable,
> this can be achieved by exporting wrapper APIs from the
> parent and calling them from the chidren .suspend/.resume routines.

Still no good, IMHO.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak]

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Felipe Balbi
>Sent: Monday, July 04, 2011 3:01 PM
>To: Partha Basak
>Cc: balbi@ti.com; Alan Stern; Keshava Munegowda; linux-
>usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>kernel@vger.kernel.org; Anand Gadiyar; sameo@linux.intel.com;
>parthab@india.ti.com; tony@atomide.com; Kevin Hilman; Benoit Cousson;
>paul@pwsan.com; johnstul@us.ibm.com; Vishwanath Sripathy
>Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>support of ehci and ohci
>
>Hi,
>
>On Mon, Jul 04, 2011 at 02:56:30PM +0530, Partha Basak wrote:
>> >> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
>> >>
>> >> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
>> >> will turn-off the parent clocks in the Suspend path.
>> >>
>> >> Similarly, calling pm_runtime_get_sync(dev->parent) within
>> >> omap_ehci_resume will turn-on the parent clocks in the resume path.
>> >>
>> >> This way, all reference counting are implicit within the Runtime PM
>> >> layer and takes care of all combinations of only EHCI insmoded,
>> >> OHCI insmoded, both insmoded etc.
>> >>
>> >> When both EHCI & OHCI are suspended, parent clocks will actually be
>> >> turned OFF and vice-versa.
>> >
>> >not sure this is necessary. I would expect:
>> >
>> >pm_runtime_get_sync(dev) to propagate up the parent tree and enable
>> >all necessary resources to get the child in a working state. IOW, you
>> >shouldn't need to manuall access the parent device.
>> >
>> Refer to the description in Patch(5/6) <snip> In fact, the runtime
>> framework takes care the get sync and put sync of the child in turn
>> call the get sync and put sync of parent too; but calling get sync and
>> put sync of parent is by ASYNC mode; This mode queues the work item in
>> runtime pm work queue, which not getting scheduled in case of global
>> suspend path.
>> <snip>
>> This approach was tried, but did not work in the Suspend path
>
>sounds to me like a bug on pm runtime ? If you're calling
>pm_runtime_*_sync() family, shouldn't all calls be _sync() too ?
>
>> static int rpm_suspend(struct device *dev, int rpmflags)
>> 	__releases(&dev->power.lock) __acquires(&dev->power.lock) { .
>> .
>> .
>> no_callback:
>> .
>> .
>> .
>> 	/* Maybe the parent is now able to suspend. */
>> 	if (parent && !parent->power.ignore_children &&
>> !dev->power.irq_safe) {
>> 		spin_unlock(&dev->power.lock);
>>
>> 		spin_lock(&parent->power.lock);
>> 		rpm_idle(parent, RPM_ASYNC);
>
>to me this is bogus, if you called pm_runtime_put_sync() should should
>be sync too. Shouldn't it ?
>
>> 		spin_unlock(&parent->power.lock);
>>
>> 		spin_lock(&dev->power.lock);
>> 	}
>> This is the reason of directly calling the parent Runtime PM calls
>> from the children.
>> If directly calling Runtime PM APIs with parent dev-pointer isn't
>> acceptable, this can be achieved by exporting wrapper APIs from the
>> parent and calling them from the chidren .suspend/.resume routines.
>
>Still no good, IMHO.

Kevin, any comments?

Shouldn't the framework ensure that if put_sync(dev) is called
it should ensure that the parent is rmp_idled synchronously?
>
>--
>balbi

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Mon, 4 Jul 2011, Felipe Balbi wrote:

> sounds to me like a bug on pm runtime ? If you're calling
> pm_runtime_*_sync() family, shouldn't all calls be _sync() too ?

No.  This was a deliberate design decision.  It minimizes stack usage 
and it gives a chance for some other child to resume before the parent 
is powered down.

> > static int rpm_suspend(struct device *dev, int rpmflags)
> > 	__releases(&dev->power.lock) __acquires(&dev->power.lock)
> > {
> > .
> > .
> > .
> > no_callback:
> > .
> > .
> > .
> > 	/* Maybe the parent is now able to suspend. */
> > 	if (parent && !parent->power.ignore_children &&
> > !dev->power.irq_safe) {
> > 		spin_unlock(&dev->power.lock);
> > 
> > 		spin_lock(&parent->power.lock);
> > 		rpm_idle(parent, RPM_ASYNC);
> 
> to me this is bogus, if you called pm_runtime_put_sync() should should
> be sync too. Shouldn't it ?

No, it shouldn't.

> > 		spin_unlock(&parent->power.lock);
> > 
> > 		spin_lock(&dev->power.lock);
> > 	}
> > This is the reason of directly calling the parent Runtime PM calls from
> > the children.
> > If directly calling Runtime PM APIs with parent dev-pointer isn't
> > acceptable,
> > this can be achieved by exporting wrapper APIs from the
> > parent and calling them from the chidren .suspend/.resume routines.
> 
> Still no good, IMHO.

The real problem here is that you guys are trying to use the runtime PM
framework to carry out activities during system suspend.  That won't
work; it's just a bad idea all round.  Use the proper callbacks to do
what you want.

Alan Stern

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

Hi,

On Mon, Jul 04, 2011 at 12:01:24PM -0400, Alan Stern wrote:
> On Mon, 4 Jul 2011, Felipe Balbi wrote:
> 
> > sounds to me like a bug on pm runtime ? If you're calling
> > pm_runtime_*_sync() family, shouldn't all calls be _sync() too ?
> 
> No.  This was a deliberate design decision.  It minimizes stack usage 
> and it gives a chance for some other child to resume before the parent 
> is powered down.

fair enough.

> > > 		spin_unlock(&parent->power.lock);
> > > 
> > > 		spin_lock(&dev->power.lock);
> > > 	}
> > > This is the reason of directly calling the parent Runtime PM calls from
> > > the children.
> > > If directly calling Runtime PM APIs with parent dev-pointer isn't
> > > acceptable,
> > > this can be achieved by exporting wrapper APIs from the
> > > parent and calling them from the chidren .suspend/.resume routines.
> > 
> > Still no good, IMHO.
> 
> The real problem here is that you guys are trying to use the runtime PM
> framework to carry out activities during system suspend.  That won't
> work; it's just a bad idea all round.  Use the proper callbacks to do
> what you want.

then what's the point in even having runtime PM if we will still have to
implement the same functionality on the other callbacks ? Well, of
course runtime PM will conserve power on runtime, but system suspend
should be no different other than an "always deepest sleep state"
decision.

The thing now is that pm_runtime was done so that drivers would stop
caring about clocks, which is a big plus, but if we still have to handle
->suspend()/->resume() differently, we will still need to clk_get();
clk_enable(); clk_disable(); Then what was the big deal with runtime PM?

IMHO, we should have only one PM layer. System suspend/resume should be
implemented so that core PM "forcefully" calls
->runtime_suspend()/->runtime_resume() of call drivers, all
synchronously. Maybe we need an extra
RPM_STATIC_SUSPEND_PLEASE_HANDLE_IT_ALL_SYNCHRONOUSLY flag, but that's
another detail.

If drivers are really supposed to stop handling clocks directly, then
runtime PM is THE framework to do that, but if we still have system
suspend/resume the old way, I don't see the benefit for the driver
(other than the uAmps saved during runtime, which is great, don't get me
wrong ;-) that this will bring. Having two PM layers which, in fact, are
doing the same thing - reducing power consumption - is just too much
IMO.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Tue, 5 Jul 2011, Felipe Balbi wrote:

> > The real problem here is that you guys are trying to use the runtime PM
> > framework to carry out activities during system suspend.  That won't
> > work; it's just a bad idea all round.  Use the proper callbacks to do
> > what you want.
> 
> then what's the point in even having runtime PM if we will still have to
> implement the same functionality on the other callbacks ?

You don't have to duplicate the functionality.  You can use exactly the
same functions for both sets of callbacks if you want; just make sure
the callbacks point to them.

>  Well, of
> course runtime PM will conserve power on runtime, but system suspend
> should be no different other than an "always deepest sleep state"
> decision.

No, it is significantly different for several reasons.  Some of the
most important differences are concerned with freezing userspace and
deciding what events should be allowed to wake up the system.  Also, 
there are systems which can achieve greater power savings by system 
sleep than they can by runtime PM + cpuidle.

> The thing now is that pm_runtime was done so that drivers would stop
> caring about clocks, which is a big plus, but if we still have to handle
> ->suspend()/->resume() differently, we will still need to clk_get();
> clk_enable(); clk_disable(); Then what was the big deal with runtime PM?

I don't know about that.  Clock usage has always been internal to the
implementation you guys have been working on, and I haven't followed
it.  If your implementation was designed incorrectly, well, that's a
shame but it's understandable.  Things like that happen.  It shouldn't
be too hard to fix.

But first you do need to understand that system suspend really _is_ 
different from runtime suspend.  Sure, you may be able to share some 
code between them, but you should not expect to be able to use one in 
place of the other.

> IMHO, we should have only one PM layer. System suspend/resume should be
> implemented so that core PM "forcefully" calls
> ->runtime_suspend()/->runtime_resume() of call drivers, all
> synchronously. Maybe we need an extra
> RPM_STATIC_SUSPEND_PLEASE_HANDLE_IT_ALL_SYNCHRONOUSLY flag, but that's
> another detail.

Statements like this should be posted to linux-pm where they can be 
discussed properly.  It certainly isn't fair to make such claims 
without even CC-ing the PM maintainer.

Besides, handling runtime PM synchronously won't do you any good if the 
user has disabled runtime PM via sysfs or not enabled 
CONFIG_PM_RUNTIME in the first place.  Have you forgotten about those 
possibilities?

Furthermore, from what I've gathered so far from this thread, the
_real_ problem is that nobody has written suspend and resume callbacks
for the parent device.  You're relying on runtime PM to do things with
the parent, but instead you should make use of the usual system sleep
mechanism: Parents are always suspended after their children and
awakened before.  Have the parent's suspend routine disable the clocks 
and have the resume routine enable them.  Problem solved, no changes 
needed in the child's driver code.

> If drivers are really supposed to stop handling clocks directly, then
> runtime PM is THE framework to do that, but if we still have system
> suspend/resume the old way, I don't see the benefit for the driver
> (other than the uAmps saved during runtime, which is great, don't get me
> wrong ;-) that this will bring. Having two PM layers which, in fact, are
> doing the same thing - reducing power consumption - is just too much
> IMO.

They aren't doing the same thing.  If you don't believe me, ask the PM 
maintainer.

And if you actually do need your callbacks to do the same thing, simply
use shared subroutines.

Alan Stern

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4642 bytes --]

Hi,

On Tue, Jul 05, 2011 at 10:17:14AM -0400, Alan Stern wrote:
> On Tue, 5 Jul 2011, Felipe Balbi wrote:
> 
> > > The real problem here is that you guys are trying to use the runtime PM
> > > framework to carry out activities during system suspend.  That won't
> > > work; it's just a bad idea all round.  Use the proper callbacks to do
> > > what you want.
> > 
> > then what's the point in even having runtime PM if we will still have to
> > implement the same functionality on the other callbacks ?
> 
> You don't have to duplicate the functionality.  You can use exactly the
> same functions for both sets of callbacks if you want; just make sure
> the callbacks point to them.

true, good point.

> >  Well, of
> > course runtime PM will conserve power on runtime, but system suspend
> > should be no different other than an "always deepest sleep state"
> > decision.
> 
> No, it is significantly different for several reasons.  Some of the
> most important differences are concerned with freezing userspace and
> deciding what events should be allowed to wake up the system.  Also, 
> there are systems which can achieve greater power savings by system 
> sleep than they can by runtime PM + cpuidle.

I remember we've been through this discussion before and it's just
nonsensical to make such statement. What does freezing userspace have to
do with power consumption ? If you can't reach lower power consumption
with runtime PM it only means userspace is waking the system too much.

> > The thing now is that pm_runtime was done so that drivers would stop
> > caring about clocks, which is a big plus, but if we still have to handle
> > ->suspend()/->resume() differently, we will still need to clk_get();
> > clk_enable(); clk_disable(); Then what was the big deal with runtime PM?
> 
> I don't know about that.  Clock usage has always been internal to the
> implementation you guys have been working on, and I haven't followed
> it.  If your implementation was designed incorrectly, well, that's a
> shame but it's understandable.  Things like that happen.  It shouldn't
> be too hard to fix.
> 
> But first you do need to understand that system suspend really _is_ 
> different from runtime suspend.  Sure, you may be able to share some 
> code between them, but you should not expect to be able to use one in 
> place of the other.

I really fail to see why not, and maybe it's only my fault and I need to
read the Documentation/ more carefully :-s

> > IMHO, we should have only one PM layer. System suspend/resume should be
> > implemented so that core PM "forcefully" calls
> > ->runtime_suspend()/->runtime_resume() of call drivers, all
> > synchronously. Maybe we need an extra
> > RPM_STATIC_SUSPEND_PLEASE_HANDLE_IT_ALL_SYNCHRONOUSLY flag, but that's
> > another detail.
> 
> Statements like this should be posted to linux-pm where they can be 
> discussed properly.  It certainly isn't fair to make such claims 
> without even CC-ing the PM maintainer.
> 
> Besides, handling runtime PM synchronously won't do you any good if the 
> user has disabled runtime PM via sysfs or not enabled 
> CONFIG_PM_RUNTIME in the first place.  Have you forgotten about those 
> possibilities?

I thought that the "we should have only one PM layer" already carried
the idea that CONFIG_PM and CONFIG_PM_RUNTIME would be combined into
one, and sysfs would need a little re-factoring...

> Furthermore, from what I've gathered so far from this thread, the
> _real_ problem is that nobody has written suspend and resume callbacks
> for the parent device.  You're relying on runtime PM to do things with
> the parent, but instead you should make use of the usual system sleep
> mechanism: Parents are always suspended after their children and
> awakened before.  Have the parent's suspend routine disable the clocks 
> and have the resume routine enable them.  Problem solved, no changes 
> needed in the child's driver code.

that's currently hidden on the omap rutime pm support. No driver is to
talk to clk API directly anymore. Granted, now that I read what I just
wrote it does sound like it's a limitation, although it's really nice
not to have to remember all the numerous clocks needed for a particular
device to work properly. So, if there would be a way, other than
pm_runtime_resume(), to enable all clocks a particular device has
without really having to clk_get(); clk_enable() each one of them, fine,
this would be solved. But as of today, we only have pm_runtime_resume()
to achieve that, unless I'm missing something.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Tue, 5 Jul 2011, Felipe Balbi wrote:

> > >  Well, of
> > > course runtime PM will conserve power on runtime, but system suspend
> > > should be no different other than an "always deepest sleep state"
> > > decision.
> > 
> > No, it is significantly different for several reasons.  Some of the
> > most important differences are concerned with freezing userspace and
> > deciding what events should be allowed to wake up the system.  Also, 
> > there are systems which can achieve greater power savings by system 
> > sleep than they can by runtime PM + cpuidle.
> 
> I remember we've been through this discussion before and it's just
> nonsensical to make such statement. What does freezing userspace have to
> do with power consumption ? If you can't reach lower power consumption
> with runtime PM it only means userspace is waking the system too much.

_That's_ what freezing userspace has to do with power consumption.  You 
have answered your own question.

And don't forget the other points about wakeup events and greater power
savings.  If you want to discuss this further, you should start a new
thread on linux-pm.


> > Furthermore, from what I've gathered so far from this thread, the
> > _real_ problem is that nobody has written suspend and resume callbacks
> > for the parent device.  You're relying on runtime PM to do things with
> > the parent, but instead you should make use of the usual system sleep
> > mechanism: Parents are always suspended after their children and
> > awakened before.  Have the parent's suspend routine disable the clocks 
> > and have the resume routine enable them.  Problem solved, no changes 
> > needed in the child's driver code.
> 
> that's currently hidden on the omap rutime pm support. No driver is to
> talk to clk API directly anymore. Granted, now that I read what I just
> wrote it does sound like it's a limitation, although it's really nice
> not to have to remember all the numerous clocks needed for a particular
> device to work properly. So, if there would be a way, other than
> pm_runtime_resume(), to enable all clocks a particular device has
> without really having to clk_get(); clk_enable() each one of them, fine,
> this would be solved. But as of today, we only have pm_runtime_resume()
> to achieve that, unless I'm missing something.

(Actually, your problem isn't enabling clocks during resume; it is 
disabling them during suspend.)

The OMAP maintainer should be involved in this discussion.  Basically,
the new PM domain framework is now supposed to handle these issues.

Alan Stern

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Mon, 4 Jul 2011, Partha Basak wrote:

> >I don't see any point in these routines (and likewise for
> >omap_ohci_suspend/resume).  When the whole system is going to sleep
> >anyway, what reason is there for enabling runtime PM on the parent
> >device?
> 
> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
> 
> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
> will turn-off the parent clocks in the Suspend path.
> 
> Similarly, calling pm_runtime_get_sync(dev->parent) within
> omap_ehci_resume
> will turn-on the parent clocks in the resume path.
> 
> This way, all reference counting are implicit within the Runtime PM layer
> and takes care of all combinations of only EHCI insmoded, OHCI insmoded,
> both insmoded etc.
> 
> When both EHCI & OHCI are suspended, parent clocks will actually be
> turned OFF and vice-versa.
> 
> Note that the parent per-se does not have any .suspend & .resume hooked
> up.

Why not?  That sounds like a big bug.

> At the end of the _probe of parent, the clocks are turned OFF.
> Subsequently, enabling
> the parent clocks are entirely done implicitly by the children get_sync()
> in their _probe.
> 
> Therefore while .suspend/.resume of children are called they call back
> into the parent to turn-off the clocks.

You have ignored a few very important points:

Firstly, system suspend is supposed to work even when runtime PM is not 
configured.

Secondly, the user can disable runtime PM via sysfs at any time.  This 
shouldn't mess up system suspend.

Basically, it's a bad idea to mix up system suspend with runtime PM.

Alan Stern

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak]

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Alan Stern
>Sent: Monday, July 04, 2011 9:21 PM
>To: Partha Basak
>Cc: Keshava Munegowda; linux-usb@vger.kernel.org; linux-
>omap@vger.kernel.org; linux-kernel@vger.kernel.org; Felipe Balbi; Anand
>Gadiyar; sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
>Kevin Hilman; Benoit Cousson; paul@pwsan.com; johnstul@us.ibm.com;
>Vishwanath Sripathy
>Subject: RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>support of ehci and ohci
>
>On Mon, 4 Jul 2011, Partha Basak wrote:
>
>> >I don't see any point in these routines (and likewise for
>> >omap_ohci_suspend/resume).  When the whole system is going to sleep
>> >anyway, what reason is there for enabling runtime PM on the parent
>> >device?
>>
>> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
>>
>> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
>> will turn-off the parent clocks in the Suspend path.
>>
>> Similarly, calling pm_runtime_get_sync(dev->parent) within
>> omap_ehci_resume
>> will turn-on the parent clocks in the resume path.
>>
>> This way, all reference counting are implicit within the Runtime PM
>layer
>> and takes care of all combinations of only EHCI insmoded, OHCI
>insmoded,
>> both insmoded etc.
>>
>> When both EHCI & OHCI are suspended, parent clocks will actually be
>> turned OFF and vice-versa.
>>
>> Note that the parent per-se does not have any .suspend & .resume
>hooked
>> up.
>
>Why not?  That sounds like a big bug.

This was a design decision since the parent needs to be activated only
when at-least one child is insmoded.

If the chidren are suspended, automatically the parent is suspended
via the pm_runtime_putsync calls to the parent.

So, effectively, we do not need an explicit suspend for the parent.
>
>> At the end of the _probe of parent, the clocks are turned OFF.
>> Subsequently, enabling
>> the parent clocks are entirely done implicitly by the children
>get_sync()
>> in their _probe.
>>
>> Therefore while .suspend/.resume of children are called they call back
>> into the parent to turn-off the clocks.
>
>You have ignored a few very important points:
>
>Firstly, system suspend is supposed to work even when runtime PM is not
>configured.
>
>Secondly, the user can disable runtime PM via sysfs at any time.  This
>shouldn't mess up system suspend.
>
>Basically, it's a bad idea to mix up system suspend with runtime PM.

Your observations are correct but this is a generic limitation and Kevin
is working
on this problem in parallel.

As of now, all OMAP drivers are mandated to use ONLY runtime pm framework
for enabling/disabling clocks. I will let Kevin comment further.

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

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Alan Stern]

On Tue, 5 Jul 2011, Partha Basak wrote:

> >> Note that the parent per-se does not have any .suspend & .resume
> >hooked
> >> up.
> >
> >Why not?  That sounds like a big bug.
> 
> This was a design decision since the parent needs to be activated only
> when at-least one child is insmoded.
> 
> If the chidren are suspended, automatically the parent is suspended
> via the pm_runtime_putsync calls to the parent.

Now you know that this isn't true, because those calls don't get made 
while a system sleep transition is in progress.

> So, effectively, we do not need an explicit suspend for the parent.

Now you know that effectively you _do_ need an explicit suspend for the 
parent.

> >You have ignored a few very important points:
> >
> >Firstly, system suspend is supposed to work even when runtime PM is not
> >configured.
> >
> >Secondly, the user can disable runtime PM via sysfs at any time.  This
> >shouldn't mess up system suspend.
> >
> >Basically, it's a bad idea to mix up system suspend with runtime PM.
> 
> Your observations are correct but this is a generic limitation and Kevin
> is working
> on this problem in parallel.
> 
> As of now, all OMAP drivers are mandated to use ONLY runtime pm framework
> for enabling/disabling clocks. I will let Kevin comment further.

Okay, let's see what Kevin says.

Alan Stern

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Kevin Hilman]

Alan Stern <stern@rowland.harvard.edu> writes:

[...]

>>>You have ignored a few very important points:
>>>
>>>Firstly, system suspend is supposed to work even when runtime PM is
>>>not configured.
>>>
>>>Secondly, the user can disable runtime PM via sysfs at any time.
>>>This shouldn't mess up system suspend.
>>>
>>>Basically, it's a bad idea to mix up system suspend with runtime PM.
>> 
>> Your observations are correct but this is a generic limitation and
>> Kevin is working on this problem in parallel.
>> 
>> As of now, all OMAP drivers are mandated to use ONLY runtime pm framework
>> for enabling/disabling clocks. I will let Kevin comment further.
>
> Okay, let's see what Kevin says.

While I did design the OMAP PM core to be runtime PM centric, and we
implemented several drivers based on runtime PM alone, after some long
discussions on linux-pm[1] with Alan and Rafael (PM maintainer) over the
last couple weeks, I'm now convinced I had the wrong design/approach.

Rafael and Alan have been patient with my stubborness, but now I've been
pursuaded.  Rafael has detailed on linux-pm the various
problems/limitations/races between runtime PM and system PM[2], so I
don't plan debating them again here.

That being said, today we have several drivers that use runtime PM calls
in their suspend/resume path and our PM domain implementation (inside
omap_device) deals with most of the limitations fine.  However, there
are 2 main problems/limitation with this that we've chosen to live with
(for now):
        
1) synchronous calls must be used in the suspend/resume path (because
   PM workqueue is frozen during suspend/resumea)
2) disabling runtime PM from userspace will prevent that device
   from hitting its low-power state during suspend.

For v3.1 (and before), we've lived with these limitations, and I'm OK
with merging other drivers for v3.1 with these limitations.  After 3.1,
this will be changing (more on this below.)  

So, while I've been OK with merging drivers with the above limitations
for one more merge window, $SUBJECT patch adds a new twist by forcibly
managing the parent device from the child device.  Personally, I really
don't like that approach and it serves as a good illustration of yet
another reason why system PM and runtime PM need understood as
conceptually very different.

For v3.2, the PM core will change[2] to futher limit/protect
interactions between runtime PM and system PM, and I will be reworking
our PM domain (omap_device) implementation accordingly.  

Basically, what that will mean is that our PM domain layer (omap_device)
will also call omap_device_idle() in the suspend path, but only if the
device is *not* already idle (from previous runtime suspend.)  The PM
domain layer will then omap_device_enable() the device in the system
resume path if it was suspended in the system suspend path.  A minimally
tested patch to do this is below.

So, the driver still does not have to care about it's specific clocks
etc. (which should address Felipe's concern), clocks and other
IP-specific PM details will all continue to be handled by omap_device,
just like it is with runtime PM.

The primary change to the driver, is that whatever needs to be done to
prepare for both runtime PM and system PM (including context
save/restore etc.) will have to be done in a common function(s) that
will be called by *both* of its ->runtime_suspend() and ->suspend()
callbacks, and similar for ->runtime_resume() and ->resume().  

Some drivers will have additional work to do for system PM though.  This
is mainly because system PM can happen at *any* time, including in the
middle of ongoing activity, whereas runtime PM transitions happen when
the device is known to be idle.  What that means is that for example, a
drivers ->suspend() method might need to wait for (or forcibly stop) any
ongoing activity in order to be sure the device is ready to be suspended.

Frankly, this is not a very big change for the drivers, as the
device-specific idle work will still be handled by the PM domain layer.

Hope that helps clarify the background.

As for this particular patch, since it is rather late in the development
cycle for v3.1, I would recommend that it wait until the omap_device
changes, and then let the PM core (for system PM and runtime PM) handle
the parent/child relationships as they are designed to.   But that is up
to Felipe and USB maintainers to decide.

Kevin

[1] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031559.html
[2] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031977.html


>From 6696e9a2b106ca9c9936e5c2ad89650010120e10 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Tue, 7 Jun 2011 16:07:28 -0700
Subject: [PATCH] OMAP: PM: omap_device: add system PM methods for PM domain handling

Using PM domain callbacks, use omap_device idle/enable to
automatically suspend/resume devices.  Also use pm_generic_* routines
to ensure driver's callbacks are correctly called.

Driver ->suspend callback is needed to ensure the driver is in a state
that it can be suspended.

If device is already idle (typically because of previous runtime PM
activity), there's nothing extra to do.

KJH: The omap_device_* calls should probably actually be done in the
     _noirq() methods.

Not-yet-Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    4 +++
 arch/arm/plat-omap/omap_device.c              |   32 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index e4c349f..bc36d05 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -44,6 +44,9 @@ extern struct device omap_device_parent;
 #define OMAP_DEVICE_STATE_IDLE		2
 #define OMAP_DEVICE_STATE_SHUTDOWN	3
 
+/* omap_device.flags values */
+#define OMAP_DEVICE_SUSPENDED BIT(0)
+
 /**
  * struct omap_device - omap_device wrapper for platform_devices
  * @pdev: platform_device
@@ -73,6 +76,7 @@ struct omap_device {
 	s8				pm_lat_level;
 	u8				hwmods_cnt;
 	u8				_state;
+	u8                              flags;
 };
 
 /* Device driver interface (call via platform_data fn ptrs) */
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 49fc0df..f2711c3 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -564,12 +564,44 @@ static int _od_runtime_resume(struct device *dev)
 	return pm_generic_runtime_resume(dev);
 }
 
+static int _od_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap_device *od = to_omap_device(pdev);
+	int ret;
+
+	ret = pm_generic_suspend(dev);
+
+	od->flags &= ~OMAP_DEVICE_SUSPENDED;
+
+	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
+		omap_device_idle(pdev);
+		od->flags |= OMAP_DEVICE_SUSPENDED;
+	}
+
+	return ret;
+}
+
+static int _od_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap_device *od = to_omap_device(pdev);
+
+	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
+	    (od->_state == OMAP_DEVICE_STATE_IDLE))
+		omap_device_enable(pdev);
+	
+	return pm_generic_resume(dev);
+}
+
 static struct dev_power_domain omap_device_power_domain = {
 	.ops = {
 		.runtime_suspend = _od_runtime_suspend,
 		.runtime_idle = _od_runtime_idle,
 		.runtime_resume = _od_runtime_resume,
 		USE_PLATFORM_PM_SLEEP_OPS
+		.suspend = _od_suspend,
+		.resume = _od_resume,
 	}
 };
 
-- 
1.7.6

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 8066 bytes --]

Hi,

On Tue, Jul 05, 2011 at 10:37:40AM -0700, Kevin Hilman wrote:
> While I did design the OMAP PM core to be runtime PM centric, and we
> implemented several drivers based on runtime PM alone, after some long
> discussions on linux-pm[1] with Alan and Rafael (PM maintainer) over the
> last couple weeks, I'm now convinced I had the wrong design/approach.
> 
> Rafael and Alan have been patient with my stubborness, but now I've been
> pursuaded.  Rafael has detailed on linux-pm the various
> problems/limitations/races between runtime PM and system PM[2], so I
> don't plan debating them again here.
> 
> That being said, today we have several drivers that use runtime PM calls
> in their suspend/resume path and our PM domain implementation (inside
> omap_device) deals with most of the limitations fine.  However, there
> are 2 main problems/limitation with this that we've chosen to live with
> (for now):
>         
> 1) synchronous calls must be used in the suspend/resume path (because
>    PM workqueue is frozen during suspend/resumea)
> 2) disabling runtime PM from userspace will prevent that device
>    from hitting its low-power state during suspend.
> 
> For v3.1 (and before), we've lived with these limitations, and I'm OK
> with merging other drivers for v3.1 with these limitations.  After 3.1,
> this will be changing (more on this below.)  
> 
> So, while I've been OK with merging drivers with the above limitations
> for one more merge window, $SUBJECT patch adds a new twist by forcibly
> managing the parent device from the child device.  Personally, I really
> don't like that approach and it serves as a good illustration of yet
> another reason why system PM and runtime PM need understood as
> conceptually very different.
> 
> For v3.2, the PM core will change[2] to futher limit/protect
> interactions between runtime PM and system PM, and I will be reworking
> our PM domain (omap_device) implementation accordingly.  
> 
> Basically, what that will mean is that our PM domain layer (omap_device)
> will also call omap_device_idle() in the suspend path, but only if the
> device is *not* already idle (from previous runtime suspend.)  The PM
> domain layer will then omap_device_enable() the device in the system
> resume path if it was suspended in the system suspend path.  A minimally
> tested patch to do this is below.
> 
> So, the driver still does not have to care about it's specific clocks
> etc. (which should address Felipe's concern), clocks and other
> IP-specific PM details will all continue to be handled by omap_device,
> just like it is with runtime PM.
> 
> The primary change to the driver, is that whatever needs to be done to
> prepare for both runtime PM and system PM (including context
> save/restore etc.) will have to be done in a common function(s) that
> will be called by *both* of its ->runtime_suspend() and ->suspend()
> callbacks, and similar for ->runtime_resume() and ->resume().  
> 
> Some drivers will have additional work to do for system PM though.  This
> is mainly because system PM can happen at *any* time, including in the
> middle of ongoing activity, whereas runtime PM transitions happen when
> the device is known to be idle.  What that means is that for example, a
> drivers ->suspend() method might need to wait for (or forcibly stop) any
> ongoing activity in order to be sure the device is ready to be suspended.
> 
> Frankly, this is not a very big change for the drivers, as the
> device-specific idle work will still be handled by the PM domain layer.
> 
> Hope that helps clarify the background.
> 
> As for this particular patch, since it is rather late in the development
> cycle for v3.1, I would recommend that it wait until the omap_device
> changes, and then let the PM core (for system PM and runtime PM) handle
> the parent/child relationships as they are designed to.   But that is up
> to Felipe and USB maintainers to decide.
> 
> Kevin
> 
> [1] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031559.html
> [2] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031977.html
> 
> 
> From 6696e9a2b106ca9c9936e5c2ad89650010120e10 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Tue, 7 Jun 2011 16:07:28 -0700
> Subject: [PATCH] OMAP: PM: omap_device: add system PM methods for PM domain handling
> 
> Using PM domain callbacks, use omap_device idle/enable to
> automatically suspend/resume devices.  Also use pm_generic_* routines
> to ensure driver's callbacks are correctly called.
> 
> Driver ->suspend callback is needed to ensure the driver is in a state
> that it can be suspended.
> 
> If device is already idle (typically because of previous runtime PM
> activity), there's nothing extra to do.
> 
> KJH: The omap_device_* calls should probably actually be done in the
>      _noirq() methods.
> 
> Not-yet-Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/omap_device.h |    4 +++
>  arch/arm/plat-omap/omap_device.c              |   32 +++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index e4c349f..bc36d05 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -44,6 +44,9 @@ extern struct device omap_device_parent;
>  #define OMAP_DEVICE_STATE_IDLE		2
>  #define OMAP_DEVICE_STATE_SHUTDOWN	3
>  
> +/* omap_device.flags values */
> +#define OMAP_DEVICE_SUSPENDED BIT(0)
> +
>  /**
>   * struct omap_device - omap_device wrapper for platform_devices
>   * @pdev: platform_device
> @@ -73,6 +76,7 @@ struct omap_device {
>  	s8				pm_lat_level;
>  	u8				hwmods_cnt;
>  	u8				_state;
> +	u8                              flags;
>  };
>  
>  /* Device driver interface (call via platform_data fn ptrs) */
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 49fc0df..f2711c3 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -564,12 +564,44 @@ static int _od_runtime_resume(struct device *dev)
>  	return pm_generic_runtime_resume(dev);
>  }
>  
> +static int _od_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct omap_device *od = to_omap_device(pdev);
> +	int ret;
> +
> +	ret = pm_generic_suspend(dev);
> +
> +	od->flags &= ~OMAP_DEVICE_SUSPENDED;
> +
> +	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
> +		omap_device_idle(pdev);
> +		od->flags |= OMAP_DEVICE_SUSPENDED;
> +	}
> +
> +	return ret;
> +}
> +
> +static int _od_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct omap_device *od = to_omap_device(pdev);

seems like you guys have duplicated helpers for this. There's
_find_by_pdev() and to_omap_device and both do the exact same thing:

static inline struct omap_device *_find_by_pdev(struct platform_device *pdev)
{
	return container_of(pdev, struct omap_device, pdev);
}

#define to_omap_device(x) container_of((x), struct omap_device, pdev)

> +
> +	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
> +	    (od->_state == OMAP_DEVICE_STATE_IDLE))
> +		omap_device_enable(pdev);
> +	
> +	return pm_generic_resume(dev);
> +}
> +
>  static struct dev_power_domain omap_device_power_domain = {
>  	.ops = {
>  		.runtime_suspend = _od_runtime_suspend,
>  		.runtime_idle = _od_runtime_idle,
>  		.runtime_resume = _od_runtime_resume,
>  		USE_PLATFORM_PM_SLEEP_OPS
> +		.suspend = _od_suspend,
> +		.resume = _od_resume,
>  	}
>  };

it all depends on when are you planning to get this patch upstream. I'm
considering getting some PM working on USB host and remove the
pm_runtime calls from system suspend/resume either during -rc or next
merge window.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

> Hi,
>
> On Tue, Jul 05, 2011 at 10:37:40AM -0700, Kevin Hilman wrote:
>> While I did design the OMAP PM core to be runtime PM centric, and we
>> implemented several drivers based on runtime PM alone, after some long
>> discussions on linux-pm[1] with Alan and Rafael (PM maintainer) over the
>> last couple weeks, I'm now convinced I had the wrong design/approach.
>> 
>> Rafael and Alan have been patient with my stubborness, but now I've been
>> pursuaded.  Rafael has detailed on linux-pm the various
>> problems/limitations/races between runtime PM and system PM[2], so I
>> don't plan debating them again here.
>> 
>> That being said, today we have several drivers that use runtime PM calls
>> in their suspend/resume path and our PM domain implementation (inside
>> omap_device) deals with most of the limitations fine.  However, there
>> are 2 main problems/limitation with this that we've chosen to live with
>> (for now):
>>         
>> 1) synchronous calls must be used in the suspend/resume path (because
>>    PM workqueue is frozen during suspend/resumea)
>> 2) disabling runtime PM from userspace will prevent that device
>>    from hitting its low-power state during suspend.
>> 
>> For v3.1 (and before), we've lived with these limitations, and I'm OK
>> with merging other drivers for v3.1 with these limitations.  After 3.1,
>> this will be changing (more on this below.)  
>> 
>> So, while I've been OK with merging drivers with the above limitations
>> for one more merge window, $SUBJECT patch adds a new twist by forcibly
>> managing the parent device from the child device.  Personally, I really
>> don't like that approach and it serves as a good illustration of yet
>> another reason why system PM and runtime PM need understood as
>> conceptually very different.
>> 
>> For v3.2, the PM core will change[2] to futher limit/protect
>> interactions between runtime PM and system PM, and I will be reworking
>> our PM domain (omap_device) implementation accordingly.  
>> 
>> Basically, what that will mean is that our PM domain layer (omap_device)
>> will also call omap_device_idle() in the suspend path, but only if the
>> device is *not* already idle (from previous runtime suspend.)  The PM
>> domain layer will then omap_device_enable() the device in the system
>> resume path if it was suspended in the system suspend path.  A minimally
>> tested patch to do this is below.
>> 
>> So, the driver still does not have to care about it's specific clocks
>> etc. (which should address Felipe's concern), clocks and other
>> IP-specific PM details will all continue to be handled by omap_device,
>> just like it is with runtime PM.
>> 
>> The primary change to the driver, is that whatever needs to be done to
>> prepare for both runtime PM and system PM (including context
>> save/restore etc.) will have to be done in a common function(s) that
>> will be called by *both* of its ->runtime_suspend() and ->suspend()
>> callbacks, and similar for ->runtime_resume() and ->resume().  
>> 
>> Some drivers will have additional work to do for system PM though.  This
>> is mainly because system PM can happen at *any* time, including in the
>> middle of ongoing activity, whereas runtime PM transitions happen when
>> the device is known to be idle.  What that means is that for example, a
>> drivers ->suspend() method might need to wait for (or forcibly stop) any
>> ongoing activity in order to be sure the device is ready to be suspended.
>> 
>> Frankly, this is not a very big change for the drivers, as the
>> device-specific idle work will still be handled by the PM domain layer.
>> 
>> Hope that helps clarify the background.
>> 
>> As for this particular patch, since it is rather late in the development
>> cycle for v3.1, I would recommend that it wait until the omap_device
>> changes, and then let the PM core (for system PM and runtime PM) handle
>> the parent/child relationships as they are designed to.   But that is up
>> to Felipe and USB maintainers to decide.
>> 
>> Kevin
>> 
>> [1] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031559.html
>> [2] https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031977.html
>> 
>> 
>> From 6696e9a2b106ca9c9936e5c2ad89650010120e10 Mon Sep 17 00:00:00 2001
>> From: Kevin Hilman <khilman@ti.com>
>> Date: Tue, 7 Jun 2011 16:07:28 -0700
>> Subject: [PATCH] OMAP: PM: omap_device: add system PM methods for PM domain handling
>> 
>> Using PM domain callbacks, use omap_device idle/enable to
>> automatically suspend/resume devices.  Also use pm_generic_* routines
>> to ensure driver's callbacks are correctly called.
>> 
>> Driver ->suspend callback is needed to ensure the driver is in a state
>> that it can be suspended.
>> 
>> If device is already idle (typically because of previous runtime PM
>> activity), there's nothing extra to do.
>> 
>> KJH: The omap_device_* calls should probably actually be done in the
>>      _noirq() methods.
>> 
>> Not-yet-Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>>  arch/arm/plat-omap/include/plat/omap_device.h |    4 +++
>>  arch/arm/plat-omap/omap_device.c              |   32 +++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
>> index e4c349f..bc36d05 100644
>> --- a/arch/arm/plat-omap/include/plat/omap_device.h
>> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
>> @@ -44,6 +44,9 @@ extern struct device omap_device_parent;
>>  #define OMAP_DEVICE_STATE_IDLE		2
>>  #define OMAP_DEVICE_STATE_SHUTDOWN	3
>>  
>> +/* omap_device.flags values */
>> +#define OMAP_DEVICE_SUSPENDED BIT(0)
>> +
>>  /**
>>   * struct omap_device - omap_device wrapper for platform_devices
>>   * @pdev: platform_device
>> @@ -73,6 +76,7 @@ struct omap_device {
>>  	s8				pm_lat_level;
>>  	u8				hwmods_cnt;
>>  	u8				_state;
>> +	u8                              flags;
>>  };
>>  
>>  /* Device driver interface (call via platform_data fn ptrs) */
>> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
>> index 49fc0df..f2711c3 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -564,12 +564,44 @@ static int _od_runtime_resume(struct device *dev)
>>  	return pm_generic_runtime_resume(dev);
>>  }
>>  
>> +static int _od_suspend(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct omap_device *od = to_omap_device(pdev);
>> +	int ret;
>> +
>> +	ret = pm_generic_suspend(dev);
>> +
>> +	od->flags &= ~OMAP_DEVICE_SUSPENDED;
>> +
>> +	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
>> +		omap_device_idle(pdev);
>> +		od->flags |= OMAP_DEVICE_SUSPENDED;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int _od_resume(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct omap_device *od = to_omap_device(pdev);
>
> seems like you guys have duplicated helpers for this. There's
> _find_by_pdev() and to_omap_device and both do the exact same thing:
>
> static inline struct omap_device *_find_by_pdev(struct platform_device *pdev)
> {
> 	return container_of(pdev, struct omap_device, pdev);
> }
>
> #define to_omap_device(x) container_of((x), struct omap_device, pdev)

Yeah, I know.

>> +
>> +	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
>> +	    (od->_state == OMAP_DEVICE_STATE_IDLE))
>> +		omap_device_enable(pdev);
>> +	
>> +	return pm_generic_resume(dev);
>> +}
>> +
>>  static struct dev_power_domain omap_device_power_domain = {
>>  	.ops = {
>>  		.runtime_suspend = _od_runtime_suspend,
>>  		.runtime_idle = _od_runtime_idle,
>>  		.runtime_resume = _od_runtime_resume,
>>  		USE_PLATFORM_PM_SLEEP_OPS
>> +		.suspend = _od_suspend,
>> +		.resume = _od_resume,
>>  	}
>>  };
>
> it all depends on when are you planning to get this patch upstream. I'm
> considering getting some PM working on USB host and remove the
> pm_runtime calls from system suspend/resume either during -rc or next
> merge window.

Well, IMO it's way too late for this kind of change for -rc, so I'm
considering it for the upcoming merge window.

Kevin

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

Hi,

On Wed, Jul 06, 2011 at 12:20:40PM -0700, Kevin Hilman wrote:
> >> +	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
> >> +	    (od->_state == OMAP_DEVICE_STATE_IDLE))
> >> +		omap_device_enable(pdev);
> >> +	
> >> +	return pm_generic_resume(dev);
> >> +}
> >> +
> >>  static struct dev_power_domain omap_device_power_domain = {
> >>  	.ops = {
> >>  		.runtime_suspend = _od_runtime_suspend,
> >>  		.runtime_idle = _od_runtime_idle,
> >>  		.runtime_resume = _od_runtime_resume,
> >>  		USE_PLATFORM_PM_SLEEP_OPS
> >> +		.suspend = _od_suspend,
> >> +		.resume = _od_resume,
> >>  	}
> >>  };
> >
> > it all depends on when are you planning to get this patch upstream. I'm
> > considering getting some PM working on USB host and remove the
> > pm_runtime calls from system suspend/resume either during -rc or next
> > merge window.
> 
> Well, IMO it's way too late for this kind of change for -rc, so I'm
> considering it for the upcoming merge window.

yes, that's true. Who should take the hwmod patches btw ? I'm still
wondering if we should patch hwmod data first and push the _correct_ PM
part on 3.2.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak]

>-----Original Message-----
>From: Felipe Balbi [mailto:balbi@ti.com]
>Sent: Thursday, July 07, 2011 3:48 AM
>To: Kevin Hilman
>Cc: balbi@ti.com; Alan Stern; Partha Basak; Keshava Munegowda; linux-
>usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>kernel@vger.kernel.org; Anand Gadiyar; sameo@linux.intel.com;
>parthab@india.ti.com; tony@atomide.com; Benoit Cousson; paul@pwsan.com;
>johnstul@us.ibm.com; Vishwanath Sripathy
>Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
>support of ehci and ohci
>
>Hi,
>
>On Wed, Jul 06, 2011 at 12:20:40PM -0700, Kevin Hilman wrote:
>> >> +	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
>> >> +	    (od->_state == OMAP_DEVICE_STATE_IDLE))
>> >> +		omap_device_enable(pdev);
>> >> +
>> >> +	return pm_generic_resume(dev);
>> >> +}
>> >> +
>> >>  static struct dev_power_domain omap_device_power_domain = {
>> >>  	.ops = {
>> >>  		.runtime_suspend = _od_runtime_suspend,
>> >>  		.runtime_idle = _od_runtime_idle,
>> >>  		.runtime_resume = _od_runtime_resume,
>> >>  		USE_PLATFORM_PM_SLEEP_OPS
>> >> +		.suspend = _od_suspend,
>> >> +		.resume = _od_resume,
>> >>  	}
>> >>  };
>> >
>> > it all depends on when are you planning to get this patch upstream.
>> > I'm considering getting some PM working on USB host and remove the
>> > pm_runtime calls from system suspend/resume either during -rc or
>> > next merge window.
>>
>> Well, IMO it's way too late for this kind of change for -rc, so I'm
>> considering it for the upcoming merge window.
>
>yes, that's true. Who should take the hwmod patches btw ? I'm still
>wondering if we should patch hwmod data first and push the _correct_ PM
>part on 3.2.

Once Kevin pushes this infrastructure enhancement, all drivers have to
rework
the Suspend/Resume anyways. Another way would be to go by the current
approach
now and then do the rework in 3.2 along with other drivers.

In this way, we can soak the Runtime support of EHCI/OHCI for one merge
window.
>
>--
>balbi

Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

Hi,

On Thu, Jul 07, 2011 at 10:23:59AM +0530, Partha Basak wrote:
> >-----Original Message-----
> >From: Felipe Balbi [mailto:balbi@ti.com]
> >Sent: Thursday, July 07, 2011 3:48 AM
> >To: Kevin Hilman
> >Cc: balbi@ti.com; Alan Stern; Partha Basak; Keshava Munegowda; linux-
> >usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
> >kernel@vger.kernel.org; Anand Gadiyar; sameo@linux.intel.com;
> >parthab@india.ti.com; tony@atomide.com; Benoit Cousson; paul@pwsan.com;
> >johnstul@us.ibm.com; Vishwanath Sripathy
> >Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume
> >support of ehci and ohci
> >
> >Hi,
> >
> >On Wed, Jul 06, 2011 at 12:20:40PM -0700, Kevin Hilman wrote:
> >> >> +	if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
> >> >> +	    (od->_state == OMAP_DEVICE_STATE_IDLE))
> >> >> +		omap_device_enable(pdev);
> >> >> +
> >> >> +	return pm_generic_resume(dev);
> >> >> +}
> >> >> +
> >> >>  static struct dev_power_domain omap_device_power_domain = {
> >> >>  	.ops = {
> >> >>  		.runtime_suspend = _od_runtime_suspend,
> >> >>  		.runtime_idle = _od_runtime_idle,
> >> >>  		.runtime_resume = _od_runtime_resume,
> >> >>  		USE_PLATFORM_PM_SLEEP_OPS
> >> >> +		.suspend = _od_suspend,
> >> >> +		.resume = _od_resume,
> >> >>  	}
> >> >>  };
> >> >
> >> > it all depends on when are you planning to get this patch upstream.
> >> > I'm considering getting some PM working on USB host and remove the
> >> > pm_runtime calls from system suspend/resume either during -rc or
> >> > next merge window.
> >>
> >> Well, IMO it's way too late for this kind of change for -rc, so I'm
> >> considering it for the upcoming merge window.
> >
> >yes, that's true. Who should take the hwmod patches btw ? I'm still
> >wondering if we should patch hwmod data first and push the _correct_ PM
> >part on 3.2.
> 
> Once Kevin pushes this infrastructure enhancement, all drivers have to
> rework
> the Suspend/Resume anyways. Another way would be to go by the current
> approach
> now and then do the rework in 3.2 along with other drivers.
> 
> In this way, we can soak the Runtime support of EHCI/OHCI for one merge
> window.

but if it's known to be broken, why add something which is broken
anyway ?

Tony, can you queue the arch/arm/*omap*/ patches ? I'll take care of
the EHCI stuff for the next merge window, will keep them pending in my
queue.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/6 v2] arm: omap: usb: ehci and ohci hwmod structures for omap4

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

Hi Tony, Benoit, Kevin, Paul,

On Sat, Jul 02, 2011 at 12:24:29AM +0530, Keshava Munegowda wrote:
> From: Benoit Cousson <b-cousson@ti.com>
> 
> Following 4 hwmod strcuture are added:
> UHH hwmod of usbhs with uhh base address and functional clock,
> EHCI hwmod with irq and base address,
> OHCI hwmod with irq and base address,
> TLL hwmod of usbhs with the TLL base address and irq.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>

Can the hwmod part go into this merge window ? I can fix the PM part
later on but without hwmod, I can't do anything.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/6 v2] arm: omap: usb: ehci and ohci hwmod structures for omap4

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

> Hi Tony, Benoit, Kevin, Paul,
>
> On Sat, Jul 02, 2011 at 12:24:29AM +0530, Keshava Munegowda wrote:
>> From: Benoit Cousson <b-cousson@ti.com>
>> 
>> Following 4 hwmod strcuture are added:
>> UHH hwmod of usbhs with uhh base address and functional clock,
>> EHCI hwmod with irq and base address,
>> OHCI hwmod with irq and base address,
>> TLL hwmod of usbhs with the TLL base address and irq.
>> 
>> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
>> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>
> Can the hwmod part go into this merge window ? I can fix the PM part
> later on but without hwmod, I can't do anything.

The hwmod data parts of this series need to be udpated on top of
Benoit's changes since some new fields have been added for proper
clockdomain and modulemode support.

Benoit has updated the autogen tools so any hwmod data submitted for new
IP blocks needs to use the new autogenerated data.

Kevin

Re: [PATCH 0/6 V2] arm: omap: usb: Runtime PM support for EHCI and OHCI drivers

[Samuel Ortiz]

Hi Keshava,

On Sat, Jul 02, 2011 at 12:24:28AM +0530, Keshava Munegowda wrote:
> From: Keshava Munegowda <Keshava_mgowda@ti.com>
> 
> The Hwmod structures and Runtime PM features are implemented
> For EHCI and OHCI drivers of OMAP3 and OMAP4.
> The global suspend/resume of EHCI and OHCI
> is validated on OMAP3430 sdp board with these patches.
Hust fr your information, this will not make it to 3.0. I have a revert of
your patch queued for the 3.0 MFC update pull request that I'm going to send
soon.
So this is 3.1 merge window material.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/