[PATCH 0/5 v11] mfd: omap: usb: Runtime PM support for EHCI and OHCI drivers

[PATCH 0/5 v11] mfd: 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.

These patches are re-based to Kevin's pm branch :
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

TODO:
   - Adding pad configurations to Hwmods
   - Aggressive clock cutting in usb bus suspends
   - Remote Wakeup implementation using irq-chaining

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

Keshava Munegowda (4):
  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
  mfd: omap: usb: Runtime PM support

 arch/arm/mach-omap2/clock3xxx_data.c       |   26 +-
 arch/arm/mach-omap2/clock44xx_data.c       |   10 +-
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  250 ++++++++++-
 arch/arm/mach-omap2/usb-host.c             |  116 ++---
 arch/arm/plat-omap/include/plat/usb.h      |    3 -
 drivers/mfd/omap-usb-host.c                |  733 +++++++++++-----------------
 drivers/usb/host/ehci-omap.c               |   17 +-
 drivers/usb/host/ohci-omap3.c              |   18 +-
 9 files changed, 883 insertions(+), 561 deletions(-)

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

[Keshava Munegowda]

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

Following 4 hwmod structures are added
1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
2. usb_ehci_hs hwmod with irq and base address,
3. usb_ohci_hs hwmod with irq and base address,
   - The ehci and ohci hwmods does not require functional clock
     because usb_host_hs has functional clock which is sufficient
     to access ehci and ohci address space.
   - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods
     which should not be combined. This is needed because ehci and
     ohci will have separate dedicated ports & in omap4 there is a
     clock per port.We should be able to configure the IO-Wakeup
     capability of pins specific to EHCI & OHCI separately and depending
     on the I/O wakeup event  the  only port clocks corresponding
     to the wakeup source will be enabled internally by the usb host driver.

4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>

- The initial version had only two hwmods,usb_host_hs and usb_tll_hs.
These two hwmods are reorganized as above four hwmods, because there
will be dedicated ports for ehci and ohci; and each port will have I/O
mux, which will be initialized per hwmod in future.

- migrated these hwmod structures to Kevin's pm branch:
 git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git .

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  250 +++++++++++++++++++++++++++-
 1 files changed, 249 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 6201422..5e8a640 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -68,6 +68,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
@@ -5336,6 +5340,245 @@ 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 |
+			   SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO |
+			   MSTANDBY_SMART | MSTANDBY_SMART_WKUP),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class omap44xx_usb_host_hs_hwmod_class = {
+	.name = "usb_host_hs",
+	.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,
+	.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		= "usb_host_hs",
+	.class		= &omap44xx_usb_host_hs_hwmod_class,
+	.clkdm_name	= "l3_init_clkdm",
+	.main_clk	= "usb_host_hs_fck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = OMAP4_CM_L3INIT_USB_HOST_CLKCTRL_OFFSET,
+			.context_offs = OMAP4_RM_L3INIT_USB_HOST_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_SWCTRL,
+		},
+	},
+	.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),
+/*
+ * The usbhs controller prevents the enter omap to low power mode
+ * if other than FORCE IDLE and FORCE STANDBY are used.
+ */
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
+/* 'usb_ohci_hs' class */
+static struct omap_hwmod_class omap44xx_usbhs_ohci_hwmod_class = {
+	.name = "usb_ohci_hs",
+};
+
+static struct omap_hwmod_irq_info omap44xx_usbhs_ohci_irqs[] = {
+	{ .name = "ohci-irq", .irq = 76 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap44xx_usbhs_ohci_addrs[] = {
+	{
+		.name		= "ohci",
+		.pa_start	= 0x4a064800,
+		.pa_end		= 0x4a064bff,
+	},
+	{}
+};
+
+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,
+	.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 omap44xx_usbhs_ohci_hwmod = {
+	.name		= "usb_ohci_hs",
+	.class		= &omap44xx_usbhs_ohci_hwmod_class,
+	.clkdm_name	= "l3_init_clkdm",
+	.mpu_irqs	= omap44xx_usbhs_ohci_irqs,
+	.slaves		= omap44xx_usbhs_ohci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usbhs_ohci_slaves),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+/*
+ * The ohci hwmod does not has any sysconfig , so
+ * there is no RESET and IDLE settings.
+ */
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+};
+
+/* 'usb_ehci_hs' class */
+static struct omap_hwmod_class omap44xx_usbhs_ehci_hwmod_class = {
+	.name = "usb_ehci_hs",
+};
+
+static struct omap_hwmod_irq_info omap44xx_usbhs_ehci_irqs[] = {
+	{ .name = "ehci-irq", .irq = 77 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap44xx_usbhs_ehci_addrs[] = {
+	{
+		.name		= "ehci",
+		.pa_start	= 0x4a064c00,
+		.pa_end		= 0x4a064fff,
+	},
+	{}
+};
+
+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,
+	.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 omap44xx_usbhs_ehci_hwmod = {
+	.name		= "usb_ehci_hs",
+	.class		= &omap44xx_usbhs_ehci_hwmod_class,
+	.clkdm_name	= "l3_init_clkdm",
+	.mpu_irqs	= omap44xx_usbhs_ehci_irqs,
+	.slaves		= omap44xx_usbhs_ehci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usbhs_ehci_slaves),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+/*
+ * The ehci hwmod does not has any sysconfig , so
+ * there is no RESET and IDLE settings.
+ */
+	.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 |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_ENAWAKEUP |
+			   SYSC_HAS_CLOCKACTIVITY),
+	.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 = "usb_tll_hs",
+	.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 },
+	{ .irq = -1 }
+};
+
+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,
+	.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		= "usb_tll_hs",
+	.class		= &omap44xx_usb_tll_hs_hwmod_class,
+	.clkdm_name	= "l3_init_clkdm",
+	.mpu_irqs	= omap44xx_usb_tll_hs_irqs,
+	.main_clk	= "usb_tll_hs_ick",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = OMAP4_CM_L3INIT_USB_TLL_CLKCTRL_OFFSET,
+			.context_offs = OMAP4_RM_L3INIT_USB_TLL_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+	.slaves		= omap44xx_usb_tll_hs_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_usb_tll_hs_slaves),
+/*
+ * The usbhs controller prevents the enter omap to low power mode
+ * if other than FORCE IDLE for TLL mode too.
+ */
+	.flags		= HWMOD_SWSUP_SIDLE,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
 static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* dmm class */
@@ -5475,13 +5718,18 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	&omap44xx_uart3_hwmod,
 	&omap44xx_uart4_hwmod,
 
+	/* usb host class */
+	&omap44xx_usb_host_hs_hwmod,
+	&omap44xx_usbhs_ohci_hwmod,
+	&omap44xx_usbhs_ehci_hwmod,
+	&omap44xx_usb_tll_hs_hwmod,
+
 	/* usb_otg_hs class */
 	&omap44xx_usb_otg_hs_hwmod,
 
 	/* wd_timer class */
 	&omap44xx_wd_timer2_hwmod,
 	&omap44xx_wd_timer3_hwmod,
-
 	NULL,
 };
 
-- 
1.6.0.4

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

[Keshava Munegowda]

following 4 hwmod structures are added
1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
2. usb_ehci_hs hwmod with irq and base address,
3. usb_ohci_hs hwmod with irq and base address,
	- the ehci and ohci hwmods does not require functional clock
	  because usb_host_hs has functional clock which is sufficient
          to access ehci and ohci address space.
4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
 1 files changed, 271 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 59fdb9f..d79f728 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 = {
@@ -3196,6 +3200,266 @@ 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 = "usb_host_hs",
+	.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,
+	.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		= "usb_host_hs",
+	.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),
+/*
+ * The usbhs controller prevents the enter omap to low power mode
+ * if other than FORCE IDLE and FORCE STANDBY are used.
+ */
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/* 'usb_ohci_hs' class  */
+static struct omap_hwmod_class omap34xx_usbhs_ohci_hwmod_class = {
+	.name = "usb_ohci_hs",
+};
+
+static struct omap_hwmod_irq_info omap34xx_usbhs_ohci_irqs[] = {
+	{ .name = "ohci-irq", .irq = 76 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap34xx_usbhs_ohci_addrs[] = {
+	{
+		.name		= "ohci",
+		.pa_start	= 0x48064400,
+		.pa_end		= 0x480647ff,
+	},
+	{}
+};
+
+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,
+	.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		= "usb_ohci_hs",
+	.class		= &omap34xx_usbhs_ohci_hwmod_class,
+	.mpu_irqs	= omap34xx_usbhs_ohci_irqs,
+	.slaves		= omap34xx_usbhs_ohci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_slaves),
+/*
+ * The ohci hwmod does not has any sysconfig , so
+ * there is no RESET and IDLE settings.
+ */
+	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/* 'usb_ehci_hs' class  */
+static struct omap_hwmod_class omap34xx_usbhs_ehci_hwmod_class = {
+	.name = "usb_ehci_hs",
+};
+
+static struct omap_hwmod_irq_info omap34xx_usbhs_ehci_irqs[] = {
+	{ .name = "ehci-irq", .irq = 77 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap34xx_usbhs_ehci_addrs[] = {
+	{
+		.name		= "ehci",
+		.pa_start	= 0x48064800,
+		.pa_end		= 0x48064cff,
+	},
+	{}
+};
+
+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,
+	.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		= "usb_ehci_hs",
+	.class		= &omap34xx_usbhs_ehci_hwmod_class,
+	.mpu_irqs	= omap34xx_usbhs_ehci_irqs,
+	.slaves		= omap34xx_usbhs_ehci_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_slaves),
+/*
+ * The ehci hwmod does not has any sysconfig , so
+ * there is no RESET and IDLE settings.
+ */
+	.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 |
+			SYSC_HAS_SOFTRESET | SYSC_HAS_ENAWAKEUP	|
+			SYSC_HAS_CLOCKACTIVITY),
+	.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 = "usb_tll_hs",
+	.sysc = &omap34xx_usb_tll_hs_sysc,
+};
+
+static struct omap_hwmod_irq_info omap34xx_usb_tll_hs_irqs[] = {
+	{ .name = "tll-irq", .irq = 78 },
+	{ .irq = -1 }
+};
+
+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,
+	.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		= "usb_tll_hs",
+	.class		= &omap34xx_usb_tll_hs_hwmod_class,
+	.mpu_irqs	= 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),
+/*
+ * The usbhs controller prevents the enter omap to low power mode
+ * if other than FORCE IDLE for TLL mode too.
+ */
+	.flags		= HWMOD_SWSUP_SIDLE,
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
 static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_l3_main_hwmod,
 	&omap3xxx_l4_core_hwmod,
@@ -3225,6 +3489,13 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_uart2_hwmod,
 	&omap3xxx_uart3_hwmod,
 	&omap3xxx_uart4_hwmod,
+
+	/* usb host class */
+	&omap34xx_usb_host_hs_hwmod,
+	&omap34xx_usbhs_ohci_hwmod,
+	&omap34xx_usbhs_ehci_hwmod,
+	&omap34xx_usb_tll_hs_hwmod,
+
 	/* dss class */
 	&omap3430es1_dss_core_hwmod,
 	&omap3xxx_dss_core_hwmod,
-- 
1.6.0.4

[PATCH 3/5 v11] arm: omap: usb: register hwmods of usbhs

[Keshava Munegowda]

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

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/mach-omap2/usb-host.c |  116 +++++++++++++++++-----------------------
 1 files changed, 50 insertions(+), 66 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 89ae298..9bc73eb 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	"usb_host_hs"
+#define	USBHS_OHCI_HWMODNAME	"usb_ohci_hs"
+#define USBHS_EHCI_HWMODNAME	"usb_ehci_hs"
+#define USBHS_TLL_HWMODNAME	"usb_tll_hs"
 
 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,48 @@ 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;
 	}
 
-	if (platform_device_register(&usbhs_device) < 0)
-		printk(KERN_ERR "USBHS platform_device_register failed\n");
+	oh[1] = omap_hwmod_lookup(USBHS_OHCI_HWMODNAME);
+	if (!oh[1]) {
+		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
+		return;
+	}
 
-init_end:
-	return;
+	oh[2] = omap_hwmod_lookup(USBHS_EHCI_HWMODNAME);
+	if (!oh[2]) {
+		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
+		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, %s, %s\n",
+			USBHS_UHH_HWMODNAME, USBHS_OHCI_HWMODNAME,
+			USBHS_EHCI_HWMODNAME, USBHS_TLL_HWMODNAME);
+		return;
+	}
 }
 
 #else
@@ -570,5 +556,3 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 }
 
 #endif
-
-
-- 
1.6.0.4

[PATCH 4/5 v11] 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; The redudant clock nodes are removed.

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

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index ffd55b1..63a822f 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,18 @@ 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(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(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 2af0e3f..088977a 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3281,7 +3281,7 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"uart2_fck",			&uart2_fck,	CK_443X),
 	CLK(NULL,	"uart3_fck",			&uart3_fck,	CK_443X),
 	CLK(NULL,	"uart4_fck",			&uart4_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),
@@ -3292,7 +3292,7 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"usb_host_hs_hsic60m_p2_clk",	&usb_host_hs_hsic60m_p2_clk,	CK_443X),
 	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("usbhs-omap.0",	"hs_fck",		&usb_host_hs_fck,	CK_443X),
+	CLK("usbhs_omap",	"hs_fck",		&usb_host_hs_fck,	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),
@@ -3300,7 +3300,7 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"usb_tll_hs_usb_ch2_clk",	&usb_tll_hs_usb_ch2_clk,	CK_443X),
 	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("usbhs-omap.0",	"usbtll_ick",		&usb_tll_hs_ick,	CK_443X),
+	CLK("usbhs_omap",	"usbtll_ick",		&usb_tll_hs_ick,	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),
@@ -3360,8 +3360,8 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"uart2_ick",			&dummy_ck,	CK_443X),
 	CLK(NULL,	"uart3_ick",			&dummy_ck,	CK_443X),
 	CLK(NULL,	"uart4_ick",			&dummy_ck,	CK_443X),
-	CLK("usbhs-omap.0",	"usbhost_ick",		&dummy_ck,		CK_443X),
-	CLK("usbhs-omap.0",	"usbtll_fck",		&dummy_ck,	CK_443X),
+	CLK("usbhs_omap",	"usbhost_ick",		&dummy_ck,		CK_443X),
+	CLK("usbhs_omap",	"usbtll_fck",		&dummy_ck,	CK_443X),
 	CLK("omap_wdt",	"ick",				&dummy_ck,	CK_443X),
 };
 
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 8db205e..9c2da29 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -27,7 +27,7 @@
 #include <linux/gpio.h>
 #include <plat/usb.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/5 v11] mfd: omap: usb: Runtime PM support

[Keshava Munegowda]

From: Keshava Munegowda <Keshava_mgowda@ti.com>

The usbhs core driver does not enable/disable the interface and
functional clocks; These clocks are handled by hwmod and runtime pm,
hence instead of the clock enable/disable, the runtime pm APIS are
used. however,the port clocks are handled by the usbhs core.

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/plat-omap/include/plat/usb.h |    3 -
 drivers/mfd/omap-usb-host.c           |  731 +++++++++++++--------------------
 drivers/usb/host/ehci-omap.c          |   17 +-
 drivers/usb/host/ohci-omap3.c         |   18 +-
 4 files changed, 295 insertions(+), 474 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 9c2da29..e6f3b01 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/gpio.h>
 #include <plat/usb.h>
+#include <linux/pm_runtime.h>
 
 #define USBHS_DRIVER_NAME	"usbhs_omap"
 #define OMAP_EHCI_DEVICE	"ehci-omap"
@@ -146,9 +147,6 @@
 
 
 struct usbhs_hcd_omap {
-	struct clk			*usbhost_ick;
-	struct clk			*usbhost_hs_fck;
-	struct clk			*usbhost_fs_fck;
 	struct clk			*xclk60mhsp1_ck;
 	struct clk			*xclk60mhsp2_ck;
 	struct clk			*utmi_p1_fck;
@@ -158,8 +156,6 @@ struct usbhs_hcd_omap {
 	struct clk			*usbhost_p2_fck;
 	struct clk			*usbtll_p2_fck;
 	struct clk			*init_60m_fclk;
-	struct clk			*usbtll_fck;
-	struct clk			*usbtll_ick;
 
 	void __iomem			*uhh_base;
 	void __iomem			*tll_base;
@@ -168,7 +164,6 @@ struct usbhs_hcd_omap {
 
 	u32				usbhs_rev;
 	spinlock_t			lock;
-	int				count;
 };
 /*-------------------------------------------------------------------------*/
 
@@ -318,269 +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;
-
-	omap->usbhost_ick = clk_get(dev, "usbhost_ick");
-	if (IS_ERR(omap->usbhost_ick)) {
-		ret =  PTR_ERR(omap->usbhost_ick);
-		dev_err(dev, "usbhost_ick failed error:%d\n", ret);
-		goto err_end;
-	}
-
-	omap->usbhost_hs_fck = clk_get(dev, "hs_fck");
-	if (IS_ERR(omap->usbhost_hs_fck)) {
-		ret = PTR_ERR(omap->usbhost_hs_fck);
-		dev_err(dev, "usbhost_hs_fck failed error:%d\n", ret);
-		goto err_usbhost_ick;
-	}
-
-	omap->usbhost_fs_fck = clk_get(dev, "fs_fck");
-	if (IS_ERR(omap->usbhost_fs_fck)) {
-		ret = PTR_ERR(omap->usbhost_fs_fck);
-		dev_err(dev, "usbhost_fs_fck failed error:%d\n", ret);
-		goto err_usbhost_hs_fck;
-	}
-
-	omap->usbtll_fck = clk_get(dev, "usbtll_fck");
-	if (IS_ERR(omap->usbtll_fck)) {
-		ret = PTR_ERR(omap->usbtll_fck);
-		dev_err(dev, "usbtll_fck failed error:%d\n", ret);
-		goto err_usbhost_fs_fck;
-	}
-
-	omap->usbtll_ick = clk_get(dev, "usbtll_ick");
-	if (IS_ERR(omap->usbtll_ick)) {
-		ret = PTR_ERR(omap->usbtll_ick);
-		dev_err(dev, "usbtll_ick failed error:%d\n", ret);
-		goto err_usbtll_fck;
-	}
-
-	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_usbtll_ick;
-	}
-
-	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_usbtll_ick:
-	clk_put(omap->usbtll_ick);
-
-err_usbtll_fck:
-	clk_put(omap->usbtll_fck);
-
-err_usbhost_fs_fck:
-	clk_put(omap->usbhost_fs_fck);
-
-err_usbhost_hs_fck:
-	clk_put(omap->usbhost_hs_fck);
-
-err_usbhost_ick:
-	clk_put(omap->usbhost_ick);
-
-err_end:
-	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);
-	clk_put(omap->usbtll_ick);
-	clk_put(omap->usbtll_fck);
-	clk_put(omap->usbhost_fs_fck);
-	clk_put(omap->usbhost_hs_fck);
-	clk_put(omap->usbhost_ick);
-	kfree(omap);
-
-	return 0;
-}
-
 static bool is_ohci_port(enum usbhs_omap_port_mode pmode)
 {
 	switch (pmode) {
@@ -687,30 +419,79 @@ 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 long			timeout;
-	unsigned			reg;
+	unsigned long			flags;
+
+	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 (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);
+
+	spin_unlock_irqrestore(&omap->lock, flags);
+
+	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;
+	unsigned long			flags;
+
+	dev_dbg(dev, "usbhs_runtime_suspend\n");
 
-	dev_dbg(dev, "starting TI HSUSB Controller\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;
 
-	clk_enable(omap->usbhost_ick);
-	clk_enable(omap->usbhost_hs_fck);
-	clk_enable(omap->usbhost_fs_fck);
-	clk_enable(omap->usbtll_fck);
-	clk_enable(omap->usbtll_ick);
+	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);
+
+	spin_unlock_irqrestore(&omap->lock, flags);
+
+	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;
+	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])) {
@@ -734,50 +515,6 @@ 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);
 
-	/* perform TLL soft reset, and wait until reset is complete */
-	usbhs_write(omap->tll_base, OMAP_USBTLL_SYSCONFIG,
-			OMAP_USBTLL_SYSCONFIG_SOFTRESET);
-
-	/* Wait for TLL reset to complete */
-	timeout = jiffies + msecs_to_jiffies(1000);
-	while (!(usbhs_read(omap->tll_base, OMAP_USBTLL_SYSSTATUS)
-			& OMAP_USBTLL_SYSSTATUS_RESETDONE)) {
-		cpu_relax();
-
-		if (time_after(jiffies, timeout)) {
-			dev_dbg(dev, "operation timed out\n");
-			ret = -EINVAL;
-			goto err_tll;
-		}
-	}
-
-	dev_dbg(dev, "TLL RESET DONE\n");
-
-	/* (1<<3) = no idle mode only for initial debugging */
-	usbhs_write(omap->tll_base, OMAP_USBTLL_SYSCONFIG,
-			OMAP_USBTLL_SYSCONFIG_ENAWAKEUP |
-			OMAP_USBTLL_SYSCONFIG_SIDLEMODE |
-			OMAP_USBTLL_SYSCONFIG_AUTOIDLE);
-
-	/* Put UHH in NoIdle/NoStandby mode */
-	reg = usbhs_read(omap->uhh_base, OMAP_UHH_SYSCONFIG);
-	if (is_omap_usbhs_rev1(omap)) {
-		reg |= (OMAP_UHH_SYSCONFIG_ENAWAKEUP
-				| OMAP_UHH_SYSCONFIG_SIDLEMODE
-				| OMAP_UHH_SYSCONFIG_CACTIVITY
-				| OMAP_UHH_SYSCONFIG_MIDLEMODE);
-		reg &= ~OMAP_UHH_SYSCONFIG_AUTOIDLE;
-
-
-	} else if (is_omap_usbhs_rev2(omap)) {
-		reg &= ~OMAP4_UHH_SYSCONFIG_IDLEMODE_CLEAR;
-		reg |= OMAP4_UHH_SYSCONFIG_NOIDLE;
-		reg &= ~OMAP4_UHH_SYSCONFIG_STDBYMODE_CLEAR;
-		reg |= OMAP4_UHH_SYSCONFIG_NOSTDBY;
-	}
-
-	usbhs_write(omap->uhh_base, OMAP_UHH_SYSCONFIG, reg);
-
 	reg = usbhs_read(omap->uhh_base, OMAP_UHH_HOSTCONFIG);
 	/* setup ULPI bypass and burst configurations */
 	reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN
@@ -823,49 +560,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;
@@ -911,12 +605,15 @@ 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;
+	pm_runtime_put_sync(dev);
+}
+
+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;
 
-err_tll:
 	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]);
@@ -924,123 +621,257 @@ err_tll:
 		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
 			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
 	}
-
-	clk_disable(omap->usbtll_ick);
-	clk_disable(omap->usbtll_fck);
-	clk_disable(omap->usbhost_fs_fck);
-	clk_disable(omap->usbhost_hs_fck);
-	clk_disable(omap->usbhost_ick);
-	spin_unlock_irqrestore(&omap->lock, flags);
-	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;
+	}
+
+	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;
 	}
 
-	while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
-				& (1 << 1))) {
-		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;
+	}
+
+	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->uhh_base, OMAP_UHH_SYSSTATUS)
-				& (1 << 2))) {
-		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;
 	}
 
-	usbhs_write(omap->tll_base, OMAP_USBTLL_SYSCONFIG, (1 << 1));
+	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);
+	}
 
-	while (!(usbhs_read(omap->tll_base, OMAP_USBTLL_SYSSTATUS)
-				& (1 << 0))) {
-		cpu_relax();
+	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);
+	}
 
-		if (time_after(jiffies, timeout))
-			dev_dbg(dev, "operation timed out\n");
+	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 (is_omap_usbhs_rev2(omap)) {
-		if (is_ehci_tll_mode(pdata->port_mode[0]))
-			clk_disable(omap->usbtll_p1_fck);
-		if (is_ehci_tll_mode(pdata->port_mode[1]))
-			clk_disable(omap->usbtll_p2_fck);
-		clk_disable(omap->utmi_p2_fck);
-		clk_disable(omap->utmi_p1_fck);
+	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;
 	}
 
-	clk_disable(omap->usbtll_ick);
-	clk_disable(omap->usbtll_fck);
-	clk_disable(omap->usbhost_fs_fck);
-	clk_disable(omap->usbhost_hs_fck);
-	clk_disable(omap->usbhost_ick);
+	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;
+	}
 
-	/* The gpio_free migh sleep; so unlock the spinlock */
-	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;
+	}
 
-	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]);
+	platform_set_drvdata(pdev, omap);
 
-		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
-			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	ret = omap_usbhs_alloc_children(pdev);
+	if (ret) {
+		dev_err(dev, "omap_usbhs_alloc_children failed\n");
+		goto err_alloc;
 	}
-	return;
 
-end_disble:
-	spin_unlock_irqrestore(&omap->lock, flags);
-}
+	omap_usbhs_init(dev);
 
-int omap_usbhs_enable(struct device *dev)
-{
-	return  usbhs_enable(dev->parent);
+	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 4524032..82cd3ea 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)
@@ -190,11 +191,8 @@ 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_enable(dev);
+	pm_runtime_get_sync(dev);
 
 	/*
 	 * An undocumented "feature" in the OMAP3 EHCI controller,
@@ -240,11 +238,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
-	omap_usbhs_disable(dev);
-
-err_enable:
 	disable_put_regulator(pdata);
-	usb_put_hcd(hcd);
+	pm_runtime_put_sync(dev);
 
 err_io:
 	iounmap(regs);
@@ -266,10 +261,12 @@ 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);
 	disable_put_regulator(dev->platform_data);
 	iounmap(hcd->regs);
 	usb_put_hcd(hcd);
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
 	return 0;
 }
 
diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c
index 6048f2f..6cfedaf 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>
 
 /*-------------------------------------------------------------------------*/
 
@@ -134,7 +135,7 @@ static int __devinit ohci_hcd_omap3_probe(struct platform_device *pdev)
 	int			irq;
 
 	if (usb_disabled())
-		goto err_end;
+		return -ENODEV;
 
 	if (!dev->parent) {
 		dev_err(dev, "Missing parent device\n");
@@ -172,11 +173,8 @@ 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_enable(dev);
+	pm_runtime_get_sync(dev);
 
 	ohci_hcd_init(hcd_to_ohci(hcd));
 
@@ -189,9 +187,7 @@ static int __devinit ohci_hcd_omap3_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
-	omap_usbhs_disable(dev);
-
-err_end:
+	pm_runtime_put_sync(dev);
 	usb_put_hcd(hcd);
 
 err_io:
@@ -220,9 +216,9 @@ 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);
+	pm_runtime_disable(dev);
 	usb_put_hcd(hcd);
-
 	return 0;
 }
 
-- 
1.6.0.4

Re: [PATCH 0/5 v11] mfd: omap: usb: Runtime PM support for EHCI and OHCI drivers

[Ming Lei]

Hi,

On Thu, Sep 22, 2011 at 7:37 PM, Keshava Munegowda
<keshava_mgowda@ti.com> 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.
>
> These patches are re-based to Kevin's pm branch :
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

Is kernel.org up now? :-)

thanks,
-- 
Ming Lei

Re: [PATCH 0/5 v11] mfd: omap: usb: Runtime PM support for EHCI and OHCI drivers

[Munegowda, Keshava]

On Thu, Sep 22, 2011 at 7:02 PM, Ming Lei <tom.leiming@gmail.com> wrote:
> Hi,
>
> On Thu, Sep 22, 2011 at 7:37 PM, Keshava Munegowda
> <keshava_mgowda@ti.com> 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.
>>
>> These patches are re-based to Kevin's pm branch :
>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
>
> Is kernel.org up now? :-)
>
> thanks,
> --
> Ming Lei


no!

its kevin tree, which  i took it 3 weeks back. :-(

regards
keshava

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

Hi

a few comments

On Thu, 22 Sep 2011, Keshava Munegowda wrote:

> following 4 hwmod structures are added
> 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
> 2. usb_ehci_hs hwmod with irq and base address,
> 3. usb_ohci_hs hwmod with irq and base address,
> 	- the ehci and ohci hwmods does not require functional clock
> 	  because usb_host_hs has functional clock which is sufficient
>           to access ehci and ohci address space.

Every hwmod should have a functional clock defined.  If they use the same 
functional clock as usb_host_hs, then the same main_clk should be listed 
for ehci/ohci also.

> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
> 
> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> Reviewed-by: Partha Basak <parthab@india.ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
>  1 files changed, 271 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 59fdb9f..d79f728 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 = {
> @@ -3196,6 +3200,266 @@ 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),

This is missing a bunch of SYSC bits, according to Table 24-152 
"UHH_SYSCONFIG" of the 34xx public TRM, version ZR.

> +	.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 = "usb_host_hs",
> +	.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,
> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> +};
> +
> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
> +	.clk		= "usbhost_120m_fck",

This doesn't look right.  This is an interface structure record, so it 
should be associated with an interface clock.  Is the hardware really 
using the functional clock as the interface clock?  Or, as seems more 
likely...

> +	.user		= OCP_USER_MPU,
> +	.flags		= OCPIF_SWSUP_IDLE,

... is this just a hack?  OCPIF_SWSUP_IDLE is intended to work around 
hardware autoidle bugs only.  Are you sure this shouldn't be defined as an 
optional clock instead?

> +};
> +
> +static struct omap_hwmod_ocp_if omap34xx_f48m_cfg__usb_host_hs = {
> +	.clk		= "usbhost_48m_fck",
> +	.user		= OCP_USER_MPU,
> +	.flags		= OCPIF_SWSUP_IDLE,
> +};

Same comments as the above.

> +
> +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		= "usb_host_hs",
> +	.class		= &omap34xx_usb_host_hs_hwmod_class,
> +	.main_clk	= "usbhost_ick",

Is this really the main clock?  The main clock is the clock that drives
the register logic in the IP block.  Looks to me, based on the integration
document in the 34xx TRM vZR, that this module has a functional clock.  In 
general, the only time that the main_clk should be an interface clock is 
when the clock is a combined interface and functional clock.  The mailbox 
IP block is a classic example.

> +	.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,

These bit shifts should use macros, rather than numbers, which is the 
existing practice for the other hwmods in the 
omap_hwmod_3xxx_data.c file.

> +		},
> +	},
> +	.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),
> +/*
> + * The usbhs controller prevents the enter omap to low power mode
> + * if other than FORCE IDLE and FORCE STANDBY are used.
> + */
> +	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
> +	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),

Please rebase this series on Tony's current master branch, which gets rid 
of the omap_chip structure field.

> +};
> +
> +/* 'usb_ohci_hs' class  */
> +static struct omap_hwmod_class omap34xx_usbhs_ohci_hwmod_class = {
> +	.name = "usb_ohci_hs",
> +};
> +
> +static struct omap_hwmod_irq_info omap34xx_usbhs_ohci_irqs[] = {
> +	{ .name = "ohci-irq", .irq = 76 },
> +	{ .irq = -1 }
> +};
> +
> +static struct omap_hwmod_addr_space omap34xx_usbhs_ohci_addrs[] = {
> +	{
> +		.name		= "ohci",
> +		.pa_start	= 0x48064400,
> +		.pa_end		= 0x480647ff,

This is missing a 

                .flags          = ADDR_TYPE_RT

since this address range is a register target.

> +	},
> +	{}
> +};
> +
> +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,
> +	.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		= "usb_ohci_hs",

This is missing a main_clk.

> +	.class		= &omap34xx_usbhs_ohci_hwmod_class,
> +	.mpu_irqs	= omap34xx_usbhs_ohci_irqs,
> +	.slaves		= omap34xx_usbhs_ohci_slaves,
> +	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ohci_slaves),
> +/*
> + * The ohci hwmod does not has any sysconfig , so
> + * there is no RESET and IDLE settings.
> + */
> +	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
> +	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +};
> +
> +/* 'usb_ehci_hs' class  */
> +static struct omap_hwmod_class omap34xx_usbhs_ehci_hwmod_class = {
> +	.name = "usb_ehci_hs",
> +};
> +
> +static struct omap_hwmod_irq_info omap34xx_usbhs_ehci_irqs[] = {
> +	{ .name = "ehci-irq", .irq = 77 },
> +	{ .irq = -1 }
> +};
> +
> +static struct omap_hwmod_addr_space omap34xx_usbhs_ehci_addrs[] = {
> +	{
> +		.name		= "ehci",
> +		.pa_start	= 0x48064800,
> +		.pa_end		= 0x48064cff,

This is missing a

                .flags          = ADDR_TYPE_RT

since this address range is a register target.

> +	},
> +	{}
> +};
> +
> +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,
> +	.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		= "usb_ehci_hs",
> +	.class		= &omap34xx_usbhs_ehci_hwmod_class,

This is missing a main_clk.

> +	.mpu_irqs	= omap34xx_usbhs_ehci_irqs,
> +	.slaves		= omap34xx_usbhs_ehci_slaves,
> +	.slaves_cnt	= ARRAY_SIZE(omap34xx_usbhs_ehci_slaves),
> +/*
> + * The ehci hwmod does not has any sysconfig , so
> + * there is no RESET and IDLE settings.
> + */
> +	.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 |
> +			SYSC_HAS_SOFTRESET | SYSC_HAS_ENAWAKEUP	|
> +			SYSC_HAS_CLOCKACTIVITY),

These flags should be aligned with the character after the first 
parenthesis.

> +	.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 = "usb_tll_hs",
> +	.sysc = &omap34xx_usb_tll_hs_sysc,
> +};
> +
> +static struct omap_hwmod_irq_info omap34xx_usb_tll_hs_irqs[] = {
> +	{ .name = "tll-irq", .irq = 78 },
> +	{ .irq = -1 }
> +};
> +
> +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,
> +};

Same comments as for omap34xx_f128m_cfg__usb_host_hs above.

> +
> +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,
> +	.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		= "usb_tll_hs",
> +	.class		= &omap34xx_usb_tll_hs_hwmod_class,
> +	.mpu_irqs	= omap34xx_usb_tll_hs_irqs,
> +	.main_clk	= "usbtll_ick",

Is this really the main clock?  The main clock is the clock that drives 
the register logic in the IP block.  Looks to me, based on the integration 
document in the 34xx TRM vZR, that this module has a functional clock.  

> +	.prcm = {
> +		.omap2 = {
> +			.module_offs = CORE_MOD,
> +			.prcm_reg_id = 3,
> +			.module_bit = 2,
> +			.idlest_reg_id = 3,
> +			.idlest_idle_bit = 2,

These bit shifts should use macros, rather than numbers, which is the
existing practice for the other hwmods in the 
omap_hwmod_3xxx_data.c file.

> +		},
> +	},
> +	.slaves		= omap34xx_usb_tll_hs_slaves,
> +	.slaves_cnt	= ARRAY_SIZE(omap34xx_usb_tll_hs_slaves),
> +/*
> + * The usbhs controller prevents the enter omap to low power mode
> + * if other than FORCE IDLE for TLL mode too.
> + */
> +	.flags		= HWMOD_SWSUP_SIDLE,
> +	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +};
> +
>  static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
>  	&omap3xxx_l3_main_hwmod,
>  	&omap3xxx_l4_core_hwmod,
> @@ -3225,6 +3489,13 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
>  	&omap3xxx_uart2_hwmod,
>  	&omap3xxx_uart3_hwmod,
>  	&omap3xxx_uart4_hwmod,
> +
> +	/* usb host class */
> +	&omap34xx_usb_host_hs_hwmod,
> +	&omap34xx_usbhs_ohci_hwmod,
> +	&omap34xx_usbhs_ehci_hwmod,
> +	&omap34xx_usb_tll_hs_hwmod,
> +
>  	/* dss class */
>  	&omap3430es1_dss_core_hwmod,
>  	&omap3xxx_dss_core_hwmod,
> -- 
> 1.6.0.4
> 


- Paul

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

[Paul Walmsley]

Hi

On Thu, 22 Sep 2011, Keshava Munegowda wrote:

> Following 4 hwmod structures are added
> 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
> 2. usb_ehci_hs hwmod with irq and base address,
> 3. usb_ohci_hs hwmod with irq and base address,
>    - The ehci and ohci hwmods does not require functional clock
>      because usb_host_hs has functional clock which is sufficient
>      to access ehci and ohci address space.
>    - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods
>      which should not be combined. This is needed because ehci and
>      ohci will have separate dedicated ports & in omap4 there is a
>      clock per port.We should be able to configure the IO-Wakeup
>      capability of pins specific to EHCI & OHCI separately and depending
>      on the I/O wakeup event  the  only port clocks corresponding
>      to the wakeup source will be enabled internally by the usb host driver.
> 
> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.

Many of the same issues with the OMAP3 data (missing main_clks, missing 
ADDR_TYPE_RT, etc.) exist with this patch also.


- Paul

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

[Cousson, Benoit]

Hi Paul,

On 9/22/2011 8:14 PM, Paul Walmsley wrote:
> Hi
>
> On Thu, 22 Sep 2011, Keshava Munegowda wrote:
>
>> Following 4 hwmod structures are added
>> 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
>> 2. usb_ehci_hs hwmod with irq and base address,
>> 3. usb_ohci_hs hwmod with irq and base address,
>>     - The ehci and ohci hwmods does not require functional clock
>>       because usb_host_hs has functional clock which is sufficient
>>       to access ehci and ohci address space.
>>     - The usb_ehci_hs and usb_ohci_hs should be two separate hwmods
>>       which should not be combined. This is needed because ehci and
>>       ohci will have separate dedicated ports&  in omap4 there is a
>>       clock per port.We should be able to configure the IO-Wakeup
>>       capability of pins specific to EHCI&  OHCI separately and depending
>>       on the I/O wakeup event  the  only port clocks corresponding
>>       to the wakeup source will be enabled internally by the usb host driver.
>>
>> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
>
> Many of the same issues with the OMAP3 data (missing main_clks, missing
> ADDR_TYPE_RT, etc.) exist with this patch also.

ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, 
so why should we use it in this case?
To be honest, I've been confused with that flag for some time :-)
  * ADDR_TYPE_RT: Address space contains module register target data.
Maybe that's my English, but what does "register target data" mean exactly?
What was the intent? Providing a way to identify some register vs memory 
space?

Regarding main_clk, I do not think that some internal IPs like ohci/ehci 
should have a main_clk, since this is not visible at that level. These 
blocks do not have any PRCM / OCP connection. In fact they should not 
even be hwmod in theory, these are just some internal IPs inside the 
usb_host controller.
These are hwmods just because of the dynamic mux support needed by these 
IPs.
That was one of the comments I made on these, and Keshava explained me 
the rational and added it in the changelog.
Hopefully, we will not have that limitation once we will have migrated 
that to DT :-)


Regards,
Benoit

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

[Paul Walmsley]

Hi

On Thu, 22 Sep 2011, Cousson, Benoit wrote:

> ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so
> why should we use it in this case?

Just for consistency, since the flag isn't defined to be 
SYSCONFIG-specific.

That way, if there's another need -- other than SYSCONFIG access -- to 
identify the chunk of address space that's used for register access, we 
won't have to dig through and possibly repatch the hwmod data, just 
because some people didn't want to follow the rule.

If the flag was simply defined to be SYSCONFIG-specific, then you're 
right, it wouldn't be needed.

> To be honest, I've been confused with that flag for some time :-)
>  * ADDR_TYPE_RT: Address space contains module register target data.
> Maybe that's my English, but what does "register target data" mean exactly?

The name comes from the L3 section of the 34xx TRM - see for example 
section 9.1.1 "Terminology" of the rev ZR TRM.  The L3 has several address 
space sections, and whoever wrote that text -- Sonics? -- called the one 
with the L3's own internal registers the "register target."  And I was 
looking for a name that I did not have to make up, so I personally 
wouldn't have to defend the name ;-)

> What was the intent? Providing a way to identify some register vs memory
> space?

As you suggest, the original impetus for the flag was to identify which 
chunk of address space needs to be mapped by the hwmod code for SYSCONFIG 
accesses to work.  On current OMAPs, this seems to be the same thing as 
identifying the IP block's primary register area for every module with 
SYS* and REVISION registers.  And I probably thought at the time that 
specifying the IP block's main register mapping seemed more useful and 
generally applicable than designating where the SYSCONFIG register was.  
Hence the current definition.

> Regarding main_clk, I do not think that some internal IPs like ohci/ehci
> should have a main_clk, since this is not visible at that level. 

Do you not agree that every IP block that contains sequential logic on 
current and foreseeable future SoCs must have some clock signal to drive 
that logic?

The idea of the main_clk was not intended to be PRCM or OCP or even 
OMAP-specific.  It's just intended to represent a clock that is used to 
drive the register logic inside the IP block.  Therefore it must be 
enabled before any register access may occur.  Even if clock gating is 
handled by some higher-level interface (e.g., at the IP block level), the 
main_clk has a rate, so it also implies an upper limit on how quickly 
register operations can occur.  I suppose that all of the IP block's 
clocks could be "optional clocks," but we know that every IP block with 
registers requires at least one clock to work, and that should be the 
main_clk.

> These blocks do not have any PRCM / OCP connection.  In fact they should 
> not even be hwmod in theory, these are just some internal IPs inside the 
> usb_host controller. 

After looking at the OMAP USB host controller drivers, particularly 
drivers/mfd/omap-usb-host.c, I completely agree: it's not just theory: 
OMAP shouldn't have EHCI and OHCI hwmods in practice, either :-)

> These are hwmods just because of the dynamic mux support needed by these 
> IPs. That was one of the comments I made on these, and Keshava explained 
> me the rational and added it in the changelog.

On OMAP, since the EHCI and OHCI IP blocks are integrated into the UHH IP 
block, an MFD driver creates the EHCI and OHCI platform_devices (see 
drivers/mfd/omap-usb-host.c:omap_usbhs_alloc_children()).  On OMAP, the 
EHCI and OHCI aren't hwmod-backed devices, and so they shouldn't be using 
the hwmod dynamic pin remuxing code directly.

So I'd suggest one of two approaches:

1. If the pin muxing can follow the PM runtime status of the UHH IP block, 
   then the pin mux data should be associated with the UHH hwmod.

2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status, 
   then drivers/mfd/omap-usb-host.c is what should be handling the 
   remuxing.  omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI
   platform_devices to point to functions either in 
   arch/arm/mach-omap2/usb-host.c, or local functions that call into 
   mach-omap2/usb-host.c functions to handle pin remuxing.  (Those 
   function pointers should be provided to the MFD driver in some clean way, 
   like via platform_data.)

> Hopefully, we will not have that limitation once we will have migrated 
> that to DT :-)

Even if the data is coming from some other source, if the code still 
relies on main_clk, then it will need to be present.

But why do you perceive that specifying a main_clk is a limitation?  Or, 
put differently: what problem is caused by specifying a main_clk here?


- Paul

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

[Paul Walmsley]

Hi Keshava

so based on a closer look at how the EHCI/OHCI IP blocks are actually 
implemented on OMAP, they should not have hwmod entries associated with 
them.  Please take a look at 

http://marc.info/?l=linux-omap&m=131673433121673&w=2


- Paul

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

[Paul Walmsley]

Hi

On Thu, 22 Sep 2011, Paul Walmsley wrote:

> On Thu, 22 Sep 2011, Cousson, Benoit wrote:
> 
> > ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so
> > why should we use it in this case?
> 
> Just for consistency, since the flag isn't defined to be 
> SYSCONFIG-specific.
> 
> That way, if there's another need -- other than SYSCONFIG access -- to 
> identify the chunk of address space that's used for register access, we 
> won't have to dig through and possibly repatch the hwmod data, just 
> because some people didn't want to follow the rule.
> 
> If the flag was simply defined to be SYSCONFIG-specific, then you're 
> right, it wouldn't be needed.
> 
> > To be honest, I've been confused with that flag for some time :-)
> >  * ADDR_TYPE_RT: Address space contains module register target data.
> > Maybe that's my English, but what does "register target data" mean exactly?
> 
> The name comes from the L3 section of the 34xx TRM - see for example 
> section 9.1.1 "Terminology" of the rev ZR TRM.  The L3 has several address 
> space sections, and whoever wrote that text -- Sonics? -- called the one 
> with the L3's own internal registers the "register target."  And I was 
> looking for a name that I did not have to make up, so I personally 
> wouldn't have to defend the name ;-)
> 
> > What was the intent? Providing a way to identify some register vs memory
> > space?
> 
> As you suggest, the original impetus for the flag was to identify which 
> chunk of address space needs to be mapped by the hwmod code for SYSCONFIG 
> accesses to work.  On current OMAPs, this seems to be the same thing as 
> identifying the IP block's primary register area for every module with 
> SYS* and REVISION registers.  And I probably thought at the time that 
> specifying the IP block's main register mapping seemed more useful and 
> generally applicable than designating where the SYSCONFIG register was.  
> Hence the current definition.

Hopefully this should go without saying, but if you think there's a good 
rationale for renaming this flag to indicate that an address space 
contains the SYSCONFIG registers, or maybe just changing the 
documentation for the flag to indicate that, that's fine with me, just 
send a patch.


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi
>
> a few comments
>
> On Thu, 22 Sep 2011, Keshava Munegowda wrote:
>
>> following 4 hwmod structures are added
>> 1. usb_host_hs hwmod of usbhs with uhh base address and functional clock,
>> 2. usb_ehci_hs hwmod with irq and base address,
>> 3. usb_ohci_hs hwmod with irq and base address,
>>       - the ehci and ohci hwmods does not require functional clock
>>         because usb_host_hs has functional clock which is sufficient
>>           to access ehci and ohci address space.
>
> Every hwmod should have a functional clock defined.  If they use the same
> functional clock as usb_host_hs, then the same main_clk should be listed
> for ehci/ohci also.

yes it is right in general convention.
But USB host is different case. :-)

Yes, Ehci and Ohci are embedded within usbhs ; hence they do not need
separate functional clocks.

But the question arises here , why do we need these ehci and ohci as two
different hwmods containing only irq and base address?
It is required for future - to implement remote wakeup feature for ehci
and ohci ports depending on irq-chain handler patches by Tero.
Separate hwmods for ehci and ohci are needed to enable prcm chain-handler
to uniquely identify the wakeup source as ehci or ohci and call only the
corresponding interrupt handler.
We will be using omap_hwmod_mux_init for ehci and ohci hwmods to enable
I/O wakeup capability for respective IO-pads. Depending on the particular
wakeup source(ehci/ohci), the corresponding ehci or ohci irq handler will
be called.

If ehci and ohci are combined with usbhs hwmod as a single hwmod , then
for every wakeup (either ehci or ohci port wakeup) only the first
interrupt handler will be called (please look at the function
omap_hwmod_mux_handle_irq of

/arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
, so in this
case, if ehci interrupt is the first interrupt , then even for ohci wakeup
, only ehci interrupt will get called; which will break the functionality.

>> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
>>
>> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> Reviewed-by: Partha Basak <parthab@india.ti.com>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
>>  1 files changed, 271 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 59fdb9f..d79f728 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 = {
>> @@ -3196,6 +3200,266 @@ 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),
>
> This is missing a bunch of SYSC bits, according to Table 24-152
> "UHH_SYSCONFIG" of the 34xx public TRM, version ZR.

 some time back i had extended this , but it was causing system resets;
I will check this and add the remaining flags.



>> +     .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 = "usb_host_hs",
>> +     .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,
>> +     .user           = OCP_USER_MPU | OCP_USER_SDMA,
>> +};
>> +
>> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
>> +     .clk            = "usbhost_120m_fck",
>
> This doesn't look right.  This is an interface structure record, so it
> should be associated with an interface clock.  Is the hardware really
> using the functional clock as the interface clock?  Or, as seems more
> likely...


Agreed, how about:

main clock: usbhost_120m_fck
optional f clock: usbhost_48m_fck
interface clock: usbhost_ick.



>> +     .user           = OCP_USER_MPU,
>> +     .flags          = OCPIF_SWSUP_IDLE,
>
> ... is this just a hack?  OCPIF_SWSUP_IDLE is intended to work around
> hardware autoidle bugs only.  Are you sure this shouldn't be defined as an
> optional clock instead?

yes ! it was causeing resets, hence you this flag.

>
>> +};
>> +
>> +static struct omap_hwmod_ocp_if omap34xx_f48m_cfg__usb_host_hs = {
>> +     .clk            = "usbhost_48m_fck",
>> +     .user           = OCP_USER_MPU,
>> +     .flags          = OCPIF_SWSUP_IDLE,
>> +};
>
> Same comments as the above.

>> +
>> +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           = "usb_host_hs",
>> +     .class          = &omap34xx_usb_host_hs_hwmod_class,
>> +     .main_clk       = "usbhost_ick",
>
> Is this really the main clock?  The main clock is the clock that drives
> the register logic in the IP block.  Looks to me, based on the integration
> document in the 34xx TRM vZR, that this module has a functional clock.  In
> general, the only time that the main_clk should be an interface clock is
> when the clock is a combined interface and functional clock.  The mailbox
> IP block is a classic example.

As I mentioned above;  I can make

main clock: usbhost_120m_fck
optional f clock: usbhost_48m_fck
interface clock: usbhost_ick.

do you agree?

>> +     .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,
>
> These bit shifts should use macros, rather than numbers, which is the
> existing practice for the other hwmods in the
> omap_hwmod_3xxx_data.c file.

Ok, I will make this change.

>
>> +             },
>> +     },
>> +     .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),
>> +/*
>> + * The usbhs controller prevents the enter omap to low power mode
>> + * if other than FORCE IDLE and FORCE STANDBY are used.
>> + */
>> +     .flags          = HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
>> +     .omap_chip      = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>
> Please rebase this series on Tony's current master branch, which gets rid
> of the omap_chip structure field.


yes, I will do this;

>> +};
>> +
>> +/* 'usb_ohci_hs' class  */
>> +static struct omap_hwmod_class omap34xx_usbhs_ohci_hwmod_class = {
>> +     .name = "usb_ohci_hs",
>> +};
>> +
>> +static struct omap_hwmod_irq_info omap34xx_usbhs_ohci_irqs[] = {
>> +     { .name = "ohci-irq", .irq = 76 },
>> +     { .irq = -1 }
>> +};
>> +
>> +static struct omap_hwmod_addr_space omap34xx_usbhs_ohci_addrs[] = {
>> +     {
>> +             .name           = "ohci",
>> +             .pa_start       = 0x48064400,
>> +             .pa_end         = 0x480647ff,
>
> This is missing a
>
>                .flags          = ADDR_TYPE_RT
>
> since this address range is a register target.
>
>> +     },
>> +     {}
>> +};
>> +
>> +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,
>> +     .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           = "usb_ohci_hs",
>
> This is missing a main_clk.
>
>> +     .class          = &omap34xx_usbhs_ohci_hwmod_class,
>> +     .mpu_irqs       = omap34xx_usbhs_ohci_irqs,
>> +     .slaves         = omap34xx_usbhs_ohci_slaves,
>> +     .slaves_cnt     = ARRAY_SIZE(omap34xx_usbhs_ohci_slaves),
>> +/*
>> + * The ohci hwmod does not has any sysconfig , so
>> + * there is no RESET and IDLE settings.
>> + */
>> +     .flags          = HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
>> +     .omap_chip      = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +};
>> +
>> +/* 'usb_ehci_hs' class  */
>> +static struct omap_hwmod_class omap34xx_usbhs_ehci_hwmod_class = {
>> +     .name = "usb_ehci_hs",
>> +};
>> +
>> +static struct omap_hwmod_irq_info omap34xx_usbhs_ehci_irqs[] = {
>> +     { .name = "ehci-irq", .irq = 77 },
>> +     { .irq = -1 }
>> +};
>> +
>> +static struct omap_hwmod_addr_space omap34xx_usbhs_ehci_addrs[] = {
>> +     {
>> +             .name           = "ehci",
>> +             .pa_start       = 0x48064800,
>> +             .pa_end         = 0x48064cff,
>
> This is missing a
>
>                .flags          = ADDR_TYPE_RT
>
> since this address range is a register target.
>
>> +     },
>> +     {}
>> +};
>> +
>> +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,
>> +     .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           = "usb_ehci_hs",
>> +     .class          = &omap34xx_usbhs_ehci_hwmod_class,
>
> This is missing a main_clk.
>
>> +     .mpu_irqs       = omap34xx_usbhs_ehci_irqs,
>> +     .slaves         = omap34xx_usbhs_ehci_slaves,
>> +     .slaves_cnt     = ARRAY_SIZE(omap34xx_usbhs_ehci_slaves),
>> +/*
>> + * The ehci hwmod does not has any sysconfig , so
>> + * there is no RESET and IDLE settings.
>> + */
>> +     .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 |
>> +                     SYSC_HAS_SOFTRESET | SYSC_HAS_ENAWAKEUP |
>> +                     SYSC_HAS_CLOCKACTIVITY),
>
> These flags should be aligned with the character after the first
> parenthesis.

Sorry !  It was comment from benoit; I will fixed in may places; some how
i have missed this.

>
>> +     .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 = "usb_tll_hs",
>> +     .sysc = &omap34xx_usb_tll_hs_sysc,
>> +};
>> +
>> +static struct omap_hwmod_irq_info omap34xx_usb_tll_hs_irqs[] = {
>> +     { .name = "tll-irq", .irq = 78 },
>> +     { .irq = -1 }
>> +};
>> +
>> +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,
>> +};
>
> Same comments as for omap34xx_f128m_cfg__usb_host_hs above.
>
>> +
>> +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,
>> +     .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           = "usb_tll_hs",
>> +     .class          = &omap34xx_usb_tll_hs_hwmod_class,
>> +     .mpu_irqs       = omap34xx_usb_tll_hs_irqs,
>> +     .main_clk       = "usbtll_ick",
>
> Is this really the main clock?  The main clock is the clock that drives
> the register logic in the IP block.  Looks to me, based on the integration
> document in the 34xx TRM vZR, that this module has a functional clock.

I can make

main clock: usbtll_fck
interface clock: usbtll_ick.

do you agree?



>> +     .prcm = {
>> +             .omap2 = {
>> +                     .module_offs = CORE_MOD,
>> +                     .prcm_reg_id = 3,
>> +                     .module_bit = 2,
>> +                     .idlest_reg_id = 3,
>> +                     .idlest_idle_bit = 2,
>
> These bit shifts should use macros, rather than numbers, which is the
> existing practice for the other hwmods in the
> omap_hwmod_3xxx_data.c file.

yes, I will do this.

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

[Munegowda, Keshava]

> So I'd suggest one of two approaches:
>
> 1. If the pin muxing can follow the PM runtime status of the UHH IP block,
>   then the pin mux data should be associated with the UHH hwmod.


No, the mux is applicable only to ehci and ohci , where as
sysconfig is applicable to uhh ( usb_host_hs class).


> 2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status,
>   then drivers/mfd/omap-usb-host.c is what should be handling the
>   remuxing.  omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI
>   platform_devices to point to functions either in
>   arch/arm/mach-omap2/usb-host.c, or local functions that call into
>   mach-omap2/usb-host.c functions to handle pin remuxing.  (Those
>   function pointers should be provided to the MFD driver in some clean way,
>   like via platform_data.)

The dev_pm_ops of ehci should exist in  /drivers/usb/host/ehci-omap.c and
dev_pm_ops of phci should exist in  /drivers/usb/host/ohci-omap3.c.
In the existing design, the omap-usb-host.c host can not access the
platform driver
of ehci and ohci.
But, through function pointers it is possible to send the platform
data to ehci and ohci
drivers to handle pin remuxing; but we will not able to use tero's
patches; and it will prevent
our future design activity for remote wakeup of ehci and ohci.

please let me know if you have thoughts on this.


Regards
Keshava

Re: [PATCH 0/5 v11] mfd: omap: usb: Runtime PM support for EHCI and OHCI drivers

[Kevin Hilman]

"Munegowda, Keshava" <keshava_mgowda@ti.com> writes:

> On Thu, Sep 22, 2011 at 7:02 PM, Ming Lei <tom.leiming@gmail.com> wrote:
>> Hi,
>>
>> On Thu, Sep 22, 2011 at 7:37 PM, Keshava Munegowda
>> <keshava_mgowda@ti.com> 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.
>>>
>>> These patches are re-based to Kevin's pm branch :
>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
>>
>> Is kernel.org up now? :-)
>>
>> thanks,
>> --
>> Ming Lei
>
>
> no!
>
> its kevin tree, which  i took it 3 weeks back. :-(
>

There shouldn't be any reason to base these patches on my tree.  You
should be using a current mainline.

Kevin

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

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1922 bytes --]

Hi

On Fri, 23 Sep 2011, Munegowda, Keshava wrote:

> Paul Walmsley <paul@pwsan.com> wrote:
>
> > So I'd suggest one of two approaches:
> >
> > 1. If the pin muxing can follow the PM runtime status of the UHH IP block,
> >   then the pin mux data should be associated with the UHH hwmod.
> 
> No, the mux is applicable only to ehci and ohci , where as sysconfig is 
> applicable to uhh ( usb_host_hs class).

My point is that, as far as I can tell, I/O pad wakeup (caused by USB 
remote wakeup) is only going to matter when the entire UHH IP block has 
its clocks cut.  Otherwise, while the UHH is clocked, the EHCI and/or OHCI 
IP blocks will also be clocked, so no I/O pad wakeup will be needed. Do 
you agree?

> > 2. If the pin muxing must follow the EHCI/OHCI IP block PM runtime status,
> >   then drivers/mfd/omap-usb-host.c is what should be handling the
> >   remuxing.  omap-usb-host.c can set the dev_pm_ops of the EHCI/OHCI
> >   platform_devices to point to functions either in
> >   arch/arm/mach-omap2/usb-host.c, or local functions that call into
> >   mach-omap2/usb-host.c functions to handle pin remuxing.  (Those
> >   function pointers should be provided to the MFD driver in some clean way,
> >   like via platform_data.)
> 
> The dev_pm_ops of ehci should exist in /drivers/usb/host/ehci-omap.c and 
> dev_pm_ops of phci should exist in /drivers/usb/host/ohci-omap3.c. In 
> the existing design, the omap-usb-host.c host can not access the 
> platform driver of ehci and ohci. But, through function pointers it is 
> possible to send the platform data to ehci and ohci drivers to handle 
> pin remuxing; but we will not able to use tero's patches; and it will 
> prevent our future design activity for remote wakeup of ehci and ohci.

Could you please explain in more detail why the dynamic remuxing can't 
be done when the UHH enters or exits idle?


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

On Fri, 23 Sep 2011, Munegowda, Keshava wrote:

> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> But the question arises here , why do we need these ehci and ohci as two 
> different hwmods containing only irq and base address? It is required 
> for future - to implement remote wakeup feature for ehci and ohci ports 
> depending on irq-chain handler patches by Tero. Separate hwmods for ehci 
> and ohci are needed to enable prcm chain-handler to uniquely identify 
> the wakeup source as ehci or ohci and call only the corresponding 
> interrupt handler. We will be using omap_hwmod_mux_init for ehci and 
> ohci hwmods to enable I/O wakeup capability for respective IO-pads. 
> Depending on the particular wakeup source(ehci/ohci), the corresponding 
> ehci or ohci irq handler will be called.
> 
> If ehci and ohci are combined with usbhs hwmod as a single hwmod , then 
> for every wakeup (either ehci or ohci port wakeup) only the first 
> interrupt handler will be called (please look at the function 
> omap_hwmod_mux_handle_irq of
> 
> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
> , so in this
> case, if ehci interrupt is the first interrupt , then even for ohci wakeup
> , only ehci interrupt will get called; which will break the functionality.

Any reason why this couldn't be handled either by:

1. adding an IRQ number field to struct omap_hwmod_mux_info, and changing
_omap_hwmod_mux_handle_irq() to raise that IRQ number?

or 

2. using shared interrupts?


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3952 bytes --]

On Fri, 23 Sep 2011, Munegowda, Keshava wrote:

> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > On Thu, 22 Sep 2011, Keshava Munegowda wrote:
> >> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
> >>
> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> >> Reviewed-by: Partha Basak <parthab@india.ti.com>
> >> ---
> >>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
> >>  1 files changed, 271 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 59fdb9f..d79f728 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c

> >> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
> >> +     .clk            = "usbhost_120m_fck",
> >
> > This doesn't look right.  This is an interface structure record, so it
> > should be associated with an interface clock.  Is the hardware really
> > using the functional clock as the interface clock?  Or, as seems more
> > likely...
> 
> 
> Agreed, how about:
> 
> main clock: usbhost_120m_fck
> optional f clock: usbhost_48m_fck

Assuming the interface clock is enabled, which one of these clocks is 
needed for UHH register accesses to complete successfully?

> interface clock: usbhost_ick.
> 
> 
> 
> >> +     .user           = OCP_USER_MPU,
> >> +     .flags          = OCPIF_SWSUP_IDLE,
> >
> > ... is this just a hack?  OCPIF_SWSUP_IDLE is intended to work around
> > hardware autoidle bugs only.  Are you sure this shouldn't be defined as an
> > optional clock instead?
> 
> yes ! it was causeing resets, hence you this flag.

usbhost_120m_fck is not an interface clock.  That is probably what was
causing the problem you describe.

> >> +static struct omap_hwmod omap34xx_usb_host_hs_hwmod = {
> >> +     .name           = "usb_host_hs",
> >> +     .class          = &omap34xx_usb_host_hs_hwmod_class,
> >> +     .main_clk       = "usbhost_ick",
> >
> > Is this really the main clock?  The main clock is the clock that drives
> > the register logic in the IP block.  Looks to me, based on the integration
> > document in the 34xx TRM vZR, that this module has a functional clock.  In
> > general, the only time that the main_clk should be an interface clock is
> > when the clock is a combined interface and functional clock.  The mailbox
> > IP block is a classic example.
> 
> As I mentioned above;  I can make
> 
> main clock: usbhost_120m_fck
> optional f clock: usbhost_48m_fck
> interface clock: usbhost_ick.
> 
> do you agree?

The interface clock should definitely be usbhost_ick, no doubt about it.

But, as I mentioned above, to determine which functional clock should be 
the main clock, you first need to know which of those two clocks need to 
be enabled for register accesses to that module to succeed.

I did this work a few years ago, by trial and error since it was largely 
undocumented.  It was needed since the OMAP3 clk_enable() code waits for 
the PRCM to request that the IP block exit idle before returning.  
You might want to take a look at arch/arm/mach-omap2/clock3xxx_data.c.

> >> +static struct omap_hwmod omap34xx_usb_tll_hs_hwmod = {
> >> +     .name           = "usb_tll_hs",
> >> +     .class          = &omap34xx_usb_tll_hs_hwmod_class,
> >> +     .mpu_irqs       = omap34xx_usb_tll_hs_irqs,
> >> +     .main_clk       = "usbtll_ick",
> >
> > Is this really the main clock?  The main clock is the clock that drives
> > the register logic in the IP block.  Looks to me, based on the integration
> > document in the 34xx TRM vZR, that this module has a functional clock.
> 
> I can make
> 
> main clock: usbtll_fck
> interface clock: usbtll_ick.
> 
> do you agree?

Doesn't that make more sense?


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>
>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>>
>> But the question arises here , why do we need these ehci and ohci as two
>> different hwmods containing only irq and base address? It is required
>> for future - to implement remote wakeup feature for ehci and ohci ports
>> depending on irq-chain handler patches by Tero. Separate hwmods for ehci
>> and ohci are needed to enable prcm chain-handler to uniquely identify
>> the wakeup source as ehci or ohci and call only the corresponding
>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>> Depending on the particular wakeup source(ehci/ohci), the corresponding
>> ehci or ohci irq handler will be called.
>>
>> If ehci and ohci are combined with usbhs hwmod as a single hwmod , then
>> for every wakeup (either ehci or ohci port wakeup) only the first
>> interrupt handler will be called (please look at the function
>> omap_hwmod_mux_handle_irq of
>>
>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>> , so in this
>> case, if ehci interrupt is the first interrupt , then even for ohci wakeup
>> , only ehci interrupt will get called; which will break the functionality.
>
> Any reason why this couldn't be handled either by:
>
> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and changing
> _omap_hwmod_mux_handle_irq() to raise that IRQ number?


yes, it is possible by changing the existing irq-chain handler by tero Kristo

I am looping tero too.

So here are new requirements for the irq-chain handler

1. The hwmod should have have option to have multiple mux structures

This is something like:

The existing mux structure definition in omap_hwmod [file:
/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure

	struct omap_hwmod_mux_info	*mux;

it should changed to

	struct omap_hwmod_mux_info	**pmux;
         U32                                            mux_cnt;


pmux - pointers to mux ; array of mux structures.
mux_cnt - number of mux per hwmod.


2. The mux  omap_hwmod_mux_info  structure should have new member
called irq, like as follows:

struct omap_hwmod_mux_info {
	int				nr_pads;
	...
        ....
        u32                           irq;

};

Upon wakeup of the I/O pad of the mux , the irq-chain handler should
invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>

3.  There should be "SOME WAY" to supply the irqs  from hwmod
structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)


if you , tero and other opensource people are aligned on the proposed
changes on the irq-handler ;
then it is possible to have two hwmods ( usbhs and tll) for usbhost driver.
please let me know you comments on the above approach.


>
> or
>
> 2. using shared interrupts?
>
>
> - Paul
>

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Sat, Sep 24, 2011 at 12:45 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>
>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >
>> > On Thu, 22 Sep 2011, Keshava Munegowda wrote:
>> >> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
>> >>
>> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> >> Reviewed-by: Partha Basak <parthab@india.ti.com>
>> >> ---
>> >>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
>> >>  1 files changed, 271 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 59fdb9f..d79f728 100644
>> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>
>> >> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
>> >> +     .clk            = "usbhost_120m_fck",
>> >
>> > This doesn't look right.  This is an interface structure record, so it
>> > should be associated with an interface clock.  Is the hardware really
>> > using the functional clock as the interface clock?  Or, as seems more
>> > likely...
>>
>>
>> Agreed, how about:
>>
>> main clock: usbhost_120m_fck
>> optional f clock: usbhost_48m_fck
>
> Assuming the interface clock is enabled, which one of these clocks is
> needed for UHH register accesses to complete successfully?


it is usbhost_48m_fck ;
so,
main clock: usbhost_48m_fck
optional clock : usbhost_120m_fck

do you agree?

>
>> interface clock: usbhost_ick.
>>
>>
>>
>> >> +     .user           = OCP_USER_MPU,
>> >> +     .flags          = OCPIF_SWSUP_IDLE,
>> >
>> > ... is this just a hack?  OCPIF_SWSUP_IDLE is intended to work around
>> > hardware autoidle bugs only.  Are you sure this shouldn't be defined as an
>> > optional clock instead?
>>
>> yes ! it was causeing resets, hence you this flag.
>
> usbhost_120m_fck is not an interface clock.  That is probably what was
> causing the problem you describe.

yes , you are right ! its not an interface clock.


>
>> >> +static struct omap_hwmod omap34xx_usb_host_hs_hwmod = {
>> >> +     .name           = "usb_host_hs",
>> >> +     .class          = &omap34xx_usb_host_hs_hwmod_class,
>> >> +     .main_clk       = "usbhost_ick",
>> >
>> > Is this really the main clock?  The main clock is the clock that drives
>> > the register logic in the IP block.  Looks to me, based on the integration
>> > document in the 34xx TRM vZR, that this module has a functional clock.  In
>> > general, the only time that the main_clk should be an interface clock is
>> > when the clock is a combined interface and functional clock.  The mailbox
>> > IP block is a classic example.
>>
>> As I mentioned above;  I can make
>>
>> main clock: usbhost_120m_fck
>> optional f clock: usbhost_48m_fck
>> interface clock: usbhost_ick.
>>
>> do you agree?
>
> The interface clock should definitely be usbhost_ick, no doubt about it.
>
> But, as I mentioned above, to determine which functional clock should be
> the main clock, you first need to know which of those two clocks need to
> be enabled for register accesses to that module to succeed.
>
> I did this work a few years ago, by trial and error since it was largely
> undocumented.  It was needed since the OMAP3 clk_enable() code waits for
> the PRCM to request that the IP block exit idle before returning.
> You might want to take a look at arch/arm/mach-omap2/clock3xxx_data.c.
>
>> >> +static struct omap_hwmod omap34xx_usb_tll_hs_hwmod = {
>> >> +     .name           = "usb_tll_hs",
>> >> +     .class          = &omap34xx_usb_tll_hs_hwmod_class,
>> >> +     .mpu_irqs       = omap34xx_usb_tll_hs_irqs,
>> >> +     .main_clk       = "usbtll_ick",
>> >
>> > Is this really the main clock?  The main clock is the clock that drives
>> > the register logic in the IP block.  Looks to me, based on the integration
>> > document in the 34xx TRM vZR, that this module has a functional clock.
>>
>> I can make
>>
>> main clock: usbtll_fck
>> interface clock: usbtll_ick.
>>
>> do you agree?
>
> Doesn't that make more sense?

yes, I will do this change.


keshava

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1769 bytes --]

On Mon, 26 Sep 2011, Munegowda, Keshava wrote:

> On Sat, Sep 24, 2011 at 12:45 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
> >
> >> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> >
> >> > On Thu, 22 Sep 2011, Keshava Munegowda wrote:
> >> >> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
> >> >>
> >> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> >> >> Reviewed-by: Partha Basak <parthab@india.ti.com>
> >> >> ---
> >> >>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
> >> >>  1 files changed, 271 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 59fdb9f..d79f728 100644
> >> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >
> >> >> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
> >> >> +     .clk            = "usbhost_120m_fck",
> >> >
> >> > This doesn't look right.  This is an interface structure record, so it
> >> > should be associated with an interface clock.  Is the hardware really
> >> > using the functional clock as the interface clock?  Or, as seems more
> >> > likely...
> >>
> >>
> >> Agreed, how about:
> >>
> >> main clock: usbhost_120m_fck
> >> optional f clock: usbhost_48m_fck
> >
> > Assuming the interface clock is enabled, which one of these clocks is
> > needed for UHH register accesses to complete successfully?
> 
> it is usbhost_48m_fck ;
> so,
> main clock: usbhost_48m_fck
> optional clock : usbhost_120m_fck
> 
> do you agree?

Yes.


- Paul

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

[Cousson, Benoit]

Hi Paul,

On 9/23/2011 1:31 AM, Paul Walmsley wrote:
> Hi
>
> On Thu, 22 Sep 2011, Cousson, Benoit wrote:
>
>> ADDR_TYPE_RT is mainly use for sysconfig access inside hwmod core today, so
>> why should we use it in this case?
>
> Just for consistency, since the flag isn't defined to be
> SYSCONFIG-specific.
>
> That way, if there's another need -- other than SYSCONFIG access -- to
> identify the chunk of address space that's used for register access, we
> won't have to dig through and possibly repatch the hwmod data, just
> because some people didn't want to follow the rule.
>
> If the flag was simply defined to be SYSCONFIG-specific, then you're
> right, it wouldn't be needed.
>
>> To be honest, I've been confused with that flag for some time :-)
>>   * ADDR_TYPE_RT: Address space contains module register target data.
>> Maybe that's my English, but what does "register target data" mean exactly?
>
> The name comes from the L3 section of the 34xx TRM - see for example
> section 9.1.1 "Terminology" of the rev ZR TRM.  The L3 has several address
> space sections, and whoever wrote that text -- Sonics? -- called the one
> with the L3's own internal registers the "register target."  And I was
> looking for a name that I did not have to make up, so I personally
> wouldn't have to defend the name ;-)

OK, so the registers target is not even the IP itself but the 4k memory 
space before every L3/L4 IPs?

>> What was the intent? Providing a way to identify some register vs memory
>> space?
>
> As you suggest, the original impetus for the flag was to identify which
> chunk of address space needs to be mapped by the hwmod code for SYSCONFIG
> accesses to work.  On current OMAPs, this seems to be the same thing as
> identifying the IP block's primary register area for every module with
> SYS* and REVISION registers.  And I probably thought at the time that
> specifying the IP block's main register mapping seemed more useful and
> generally applicable than designating where the SYSCONFIG register was.
> Hence the current definition.

I think we are lucky that the sysconfig is always in the first memory 
region in the case of multiple address spaces like for the usb_host. 
This is clearly something we will have to enforce in the future, 
especially in the strongly order DT world :-)

>> Regarding main_clk, I do not think that some internal IPs like ohci/ehci
>> should have a main_clk, since this is not visible at that level.
>
> Do you not agree that every IP block that contains sequential logic on
> current and foreseeable future SoCs must have some clock signal to drive
> that logic?

Yes I do. My point was that some time we cannot necessarily identify 
this clock or this clock is not relevant due to the container around it.

> The idea of the main_clk was not intended to be PRCM or OCP or even
> OMAP-specific.  It's just intended to represent a clock that is used to
> drive the register logic inside the IP block.  Therefore it must be
> enabled before any register access may occur.  Even if clock gating is
> handled by some higher-level interface (e.g., at the IP block level), the
> main_clk has a rate, so it also implies an upper limit on how quickly
> register operations can occur.  I suppose that all of the IP block's
> clocks could be "optional clocks," but we know that every IP block with
> registers requires at least one clock to work, and that should be the
> main_clk.

OK, I'm too PRCM centric... A clock that is not clearly represented by 
the PRCM should not necessarily exist in the current hwmod 
representation for my PRCM centric point of view :-). And in this case, 
enabling the clock might not be enough since this is the modulemode that 
does matter in OMAP4. That's why I'd rather not put any clock instead of 
a clock that will anyway not be enough to do anything useful due to the 
dependency with the container.

Exposing too many fake nodes might lead to false interpretation and some 
increase of the apparent complexity wrt the real HW capacity.

Bottom-line, I'm not opposed to add such information, but I'd rather 
populate the real HW information available and not extrapolate too much 
if this does not bring some real advantage.

Regards,
Benoit

RE: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Partha Basak]

>-----Original Message-----
>From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
>Sent: Monday, September 26, 2011 7:49 PM
>To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
>parthab@india.ti.com
>Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
>tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
>vishwanath.bs@ti.com
>Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>structures for omap3
>
>On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>>
>>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
>wrote:
>>>
>>> But the question arises here , why do we need these ehci and ohci as
>two
>>> different hwmods containing only irq and base address? It is required
>>> for future - to implement remote wakeup feature for ehci and ohci
>ports
>>> depending on irq-chain handler patches by Tero. Separate hwmods for
>ehci
>>> and ohci are needed to enable prcm chain-handler to uniquely identify
>>> the wakeup source as ehci or ohci and call only the corresponding
>>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>>> Depending on the particular wakeup source(ehci/ohci), the
>corresponding
>>> ehci or ohci irq handler will be called.
>>>
>>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
>then
>>> for every wakeup (either ehci or ohci port wakeup) only the first
>>> interrupt handler will be called (please look at the function
>>> omap_hwmod_mux_handle_irq of
>>>
>>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>>> , so in this
>>> case, if ehci interrupt is the first interrupt , then even for ohci
>wakeup
>>> , only ehci interrupt will get called; which will break the
>functionality.
>>
>> Any reason why this couldn't be handled either by:
>>
>> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
>changing
>> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
>
>
>yes, it is possible by changing the existing irq-chain handler by tero
>Kristo
>
>I am looping tero too.
>
>So here are new requirements for the irq-chain handler
>
>1. The hwmod should have have option to have multiple mux structures
>
>This is something like:
>
>The existing mux structure definition in omap_hwmod [file:
>/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
>
>	struct omap_hwmod_mux_info	*mux;
>
>it should changed to
>
>	struct omap_hwmod_mux_info	**pmux;
>         U32                                            mux_cnt;
>
>
>pmux - pointers to mux ; array of mux structures.
>mux_cnt - number of mux per hwmod.
>
>
>2. The mux  omap_hwmod_mux_info  structure should have new member
>called irq, like as follows:
>
>struct omap_hwmod_mux_info {
>	int				nr_pads;
>	...
>        ....
>        u32                           irq;
>
>};
>
>Upon wakeup of the I/O pad of the mux , the irq-chain handler should
>invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
>
>3.  There should be "SOME WAY" to supply the irqs  from hwmod
>structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
>
>
>if you , tero and other opensource people are aligned on the proposed
>changes on the irq-handler ;
>then it is possible to have two hwmods ( usbhs and tll) for usbhost
>driver.
>please let me know you comments on the above approach.
>

Hello Tero,

I would like to draw your attention to the following thread:

We need to support the following:
1. Ability to associate multiple mux info to a hwmod.
2. Able to associate a particular irq handler to a mux info.
3. PRCM Chain handler should loop through all mux-info arrays
   for a particular hwmod to identify the possible wakeup source(s)
   and call the appropriate irq handler for that mux-info.
   (It is possible that both mux-info are woken up in which case both
handlers should be called).

To give you a little more perspective, EHCI & OHCI are co-controllers
under UHH/TLL.
They do not get presented separately to the OCP bus, hence do not qualify
as separate hwmods
(Paul had objected to the design approach representing EHCI & OHCI as
different hwmods).

However, we need a mechanism to efficiently identify/distinguish
remote-wakeup, connect/disconnect
On to an EHCI port vs an OHCI port & call only the correct interrupt
handler(EHCI or OHCI).

 To incorporate this, chain handler implementation would need some
enhancements.
 We can look into the details in the next merge window cycle in
conjunction with aggressive clock management support for EHCI/OHCI.
 But fundamentally, if you are aligned to the approach, we can go ahead
collapsing the EHCI & OHCI hwmods into one.
>
>>
>> or
>>
>> 2. using shared interrupts?
>>
>>
>> - Paul
>>

RE: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Tero Kristo]

On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
> >
Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
 
-----Original Message-----

> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
> >Sent: Monday, September 26, 2011 7:49 PM
> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
> >parthab@india.ti.com
> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
> >vishwanath.bs@ti.com
> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
> >structures for omap3
> >
> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
> >>
> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
> >wrote:
> >>>
> >>> But the question arises here , why do we need these ehci and ohci as
> >two
> >>> different hwmods containing only irq and base address? It is required
> >>> for future - to implement remote wakeup feature for ehci and ohci
> >ports
> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
> >ehci
> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
> >>> the wakeup source as ehci or ohci and call only the corresponding
> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
> >>> Depending on the particular wakeup source(ehci/ohci), the
> >corresponding
> >>> ehci or ohci irq handler will be called.
> >>>
> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
> >then
> >>> for every wakeup (either ehci or ohci port wakeup) only the first
> >>> interrupt handler will be called (please look at the function
> >>> omap_hwmod_mux_handle_irq of
> >>>
> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
> >>> , so in this
> >>> case, if ehci interrupt is the first interrupt , then even for ohci
> >wakeup
> >>> , only ehci interrupt will get called; which will break the
> >functionality.
> >>
> >> Any reason why this couldn't be handled either by:
> >>
> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
> >changing
> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
> >
> >
> >yes, it is possible by changing the existing irq-chain handler by tero
> >Kristo
> >
> >I am looping tero too.
> >
> >So here are new requirements for the irq-chain handler
> >
> >1. The hwmod should have have option to have multiple mux structures
> >
> >This is something like:
> >
> >The existing mux structure definition in omap_hwmod [file:
> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
> >
> >	struct omap_hwmod_mux_info	*mux;
> >
> >it should changed to
> >
> >	struct omap_hwmod_mux_info	**pmux;
> >         U32                                            mux_cnt;
> >
> >
> >pmux - pointers to mux ; array of mux structures.
> >mux_cnt - number of mux per hwmod.
> >
> >
> >2. The mux  omap_hwmod_mux_info  structure should have new member
> >called irq, like as follows:
> >
> >struct omap_hwmod_mux_info {
> >	int				nr_pads;
> >	...
> >        ....
> >        u32                           irq;
> >
> >};
> >
> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
> >
> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
> >
> >
> >if you , tero and other opensource people are aligned on the proposed
> >changes on the irq-handler ;
> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
> >driver.
> >please let me know you comments on the above approach.
> >
> 
> Hello Tero,
> 
> I would like to draw your attention to the following thread:
> 
> We need to support the following:
> 1. Ability to associate multiple mux info to a hwmod.
> 2. Able to associate a particular irq handler to a mux info.
> 3. PRCM Chain handler should loop through all mux-info arrays
>    for a particular hwmod to identify the possible wakeup source(s)
>    and call the appropriate irq handler for that mux-info.
>    (It is possible that both mux-info are woken up in which case both
> handlers should be called).
> 
> To give you a little more perspective, EHCI & OHCI are co-controllers
> under UHH/TLL.
> They do not get presented separately to the OCP bus, hence do not qualify
> as separate hwmods
> (Paul had objected to the design approach representing EHCI & OHCI as
> different hwmods).
> 
> However, we need a mechanism to efficiently identify/distinguish
> remote-wakeup, connect/disconnect
> On to an EHCI port vs an OHCI port & call only the correct interrupt
> handler(EHCI or OHCI).
> 
>  To incorporate this, chain handler implementation would need some
> enhancements.
>  We can look into the details in the next merge window cycle in
> conjunction with aggressive clock management support for EHCI/OHCI.
>  But fundamentally, if you are aligned to the approach, we can go ahead
> collapsing the EHCI & OHCI hwmods into one.

Hi,

So, you would need a mechanism to do something like this:

pad a or b wakeup detected -> irq0
pad c or d wakeup detected -> irq1?

Is it okay to do this:

pad a...d wakeup -> irq0 and irq1?

I am okay doing something like this, we just need to agree how this
would be represented from the hwmod point of view. Currently the chain
handler set does not change hwmod structures at all to provide what it
does.

-Tero

> >
> >>
> >> or
> >>
> >> 2. using shared interrupts?
> >>
> >>
> >> - Paul
> >>

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
>> >
> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
>
> -----Original Message-----
>
>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
>> >Sent: Monday, September 26, 2011 7:49 PM
>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
>> >parthab@india.ti.com
>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
>> >vishwanath.bs@ti.com
>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>> >structures for omap3
>> >
>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>> >>
>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
>> >wrote:
>> >>>
>> >>> But the question arises here , why do we need these ehci and ohci as
>> >two
>> >>> different hwmods containing only irq and base address? It is required
>> >>> for future - to implement remote wakeup feature for ehci and ohci
>> >ports
>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
>> >ehci
>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
>> >>> the wakeup source as ehci or ohci and call only the corresponding
>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>> >>> Depending on the particular wakeup source(ehci/ohci), the
>> >corresponding
>> >>> ehci or ohci irq handler will be called.
>> >>>
>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
>> >then
>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
>> >>> interrupt handler will be called (please look at the function
>> >>> omap_hwmod_mux_handle_irq of
>> >>>
>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>> >>> , so in this
>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
>> >wakeup
>> >>> , only ehci interrupt will get called; which will break the
>> >functionality.
>> >>
>> >> Any reason why this couldn't be handled either by:
>> >>
>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
>> >changing
>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
>> >
>> >
>> >yes, it is possible by changing the existing irq-chain handler by tero
>> >Kristo
>> >
>> >I am looping tero too.
>> >
>> >So here are new requirements for the irq-chain handler
>> >
>> >1. The hwmod should have have option to have multiple mux structures
>> >
>> >This is something like:
>> >
>> >The existing mux structure definition in omap_hwmod [file:
>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
>> >
>> >     struct omap_hwmod_mux_info      *mux;
>> >
>> >it should changed to
>> >
>> >     struct omap_hwmod_mux_info      **pmux;
>> >         U32                                            mux_cnt;
>> >
>> >
>> >pmux - pointers to mux ; array of mux structures.
>> >mux_cnt - number of mux per hwmod.
>> >
>> >
>> >2. The mux  omap_hwmod_mux_info  structure should have new member
>> >called irq, like as follows:
>> >
>> >struct omap_hwmod_mux_info {
>> >     int                             nr_pads;
>> >     ...
>> >        ....
>> >        u32                           irq;
>> >
>> >};
>> >
>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
>> >
>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
>> >
>> >
>> >if you , tero and other opensource people are aligned on the proposed
>> >changes on the irq-handler ;
>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
>> >driver.
>> >please let me know you comments on the above approach.
>> >
>>
>> Hello Tero,
>>
>> I would like to draw your attention to the following thread:
>>
>> We need to support the following:
>> 1. Ability to associate multiple mux info to a hwmod.
>> 2. Able to associate a particular irq handler to a mux info.
>> 3. PRCM Chain handler should loop through all mux-info arrays
>>    for a particular hwmod to identify the possible wakeup source(s)
>>    and call the appropriate irq handler for that mux-info.
>>    (It is possible that both mux-info are woken up in which case both
>> handlers should be called).
>>
>> To give you a little more perspective, EHCI & OHCI are co-controllers
>> under UHH/TLL.
>> They do not get presented separately to the OCP bus, hence do not qualify
>> as separate hwmods
>> (Paul had objected to the design approach representing EHCI & OHCI as
>> different hwmods).
>>
>> However, we need a mechanism to efficiently identify/distinguish
>> remote-wakeup, connect/disconnect
>> On to an EHCI port vs an OHCI port & call only the correct interrupt
>> handler(EHCI or OHCI).
>>
>>  To incorporate this, chain handler implementation would need some
>> enhancements.
>>  We can look into the details in the next merge window cycle in
>> conjunction with aggressive clock management support for EHCI/OHCI.
>>  But fundamentally, if you are aligned to the approach, we can go ahead
>> collapsing the EHCI & OHCI hwmods into one.
>
> Hi,
>
> So, you would need a mechanism to do something like this:
>
> pad a or b wakeup detected -> irq0
> pad c or d wakeup detected -> irq1?

yes, if get something like this , its perfect.


>
> Is it okay to do this:
>
> pad a...d wakeup -> irq0 and irq1?

No, we dont need multiple irq handlers for single wakeup source.

>
> I am okay doing something like this, we just need to agree how this
> would be represented from the hwmod point of view. Currently the chain
> handler set does not change hwmod structures at all to provide what it
> does.

paul and benoit are the people to design the mechanisam for this.

paul/Benoit
            please give you thoughts on this.

regards
keshava


>
> -Tero
>
>> >
>> >>
>> >> or
>> >>
>> >> 2. using shared interrupts?
>> >>
>> >>
>> >> - Paul
>> >>
>
>
>

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Felipe Balbi]

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

Hi,

On Tue, Sep 27, 2011 at 06:48:35PM +0530, Munegowda, Keshava wrote:
> > So, you would need a mechanism to do something like this:
> >
> > pad a or b wakeup detected -> irq0
> > pad c or d wakeup detected -> irq1?
> 
> yes, if get something like this , its perfect.

can't you have different IRQs for each pad ? I mean, allocate one
irq_desc for each pad and let drivers request a pad/pin as an IRQ
source. Then, when you detect a pad wakeup, you can:

unsigned	pad_irq = pad_number - pad->irq_base;

handle_nested_thread(pad_irq);

this will make use of threaded IRQ handlers even. Could it be something
like that ?

-- 
balbi

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

RE: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Partha Basak]

>-----Original Message-----
>From: Felipe Balbi [mailto:balbi@ti.com]
>Sent: Tuesday, September 27, 2011 6:55 PM
>To: Munegowda, Keshava
>Cc: t-kristo@ti.com; Paul Walmsley; Cousson, Benoit; Basak, Partha;
>Balbi, Felipe; parthab@india.ti.com; linux-usb@vger.kernel.org; linux-
>omap@vger.kernel.org; linux-kernel@vger.kernel.org; Gadiyar, Anand;
>sameo@linux.intel.com; tony@atomide.com; Hilman, Kevin;
>johnstul@us.ibm.com; Sripathy, Vishwanath
>Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>structures for omap3
>
>Hi,
>
>On Tue, Sep 27, 2011 at 06:48:35PM +0530, Munegowda, Keshava wrote:
>> > So, you would need a mechanism to do something like this:
>> >
>> > pad a or b wakeup detected -> irq0
>> > pad c or d wakeup detected -> irq1?
>>
>> yes, if get something like this , its perfect.
>
>can't you have different IRQs for each pad ? I mean, allocate one
>irq_desc for each pad and let drivers request a pad/pin as an IRQ
>source. Then, when you detect a pad wakeup, you can:
>
>unsigned	pad_irq = pad_number - pad->irq_base;
>
>handle_nested_thread(pad_irq);
>
>this will make use of threaded IRQ handlers even. Could it be something
>like that ?

Felipe, your suggestion would mean more design change from the existing
implementation of Tero.

I would propose something like what Tero said initially:
For each mux-info have an associated irq handler.
So, say pads a..d form mux info1. This gets associated to irq_handler1.
Similarly, say pads e..h form mux info2. This gets associated to
irq_handler2.
Both get associated to the same uhh_hwmod. Now, when chain handler scans
for wakeup sources,
it scans both mux-info1 & mux-info2.
If at-least one pad in mux-info1 is woken up, irqhandler1 is called & same
for irqhandler2.
This mechanism would need multiple mux-infos to be attached to the same
hwmod.

So, fundamentally, if we are in alignment, can we go ahead now to collapse
the ehci & ohci hwmods into one?

>
>--
>balbi

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Tero Kristo]

On Tue, 2011-09-27 at 15:24 +0200, Balbi, Felipe wrote:
> Hi,
> 
> On Tue, Sep 27, 2011 at 06:48:35PM +0530, Munegowda, Keshava wrote:
> > > So, you would need a mechanism to do something like this:
> > >
> > > pad a or b wakeup detected -> irq0
> > > pad c or d wakeup detected -> irq1?
> > 
> > yes, if get something like this , its perfect.
> 
> can't you have different IRQs for each pad ? I mean, allocate one
> irq_desc for each pad and let drivers request a pad/pin as an IRQ
> source. Then, when you detect a pad wakeup, you can:

Direct pad to irq mapping was turned down on some previous version of
the prcm chain handler set, but yes, it is a potential approach. However
the irq_desc allocation mechanism should be dynamic... there are some
200 programmable pads anyway.

>
> unsigned	pad_irq = pad_number - pad->irq_base;
> 
> handle_nested_thread(pad_irq);
> 
> this will make use of threaded IRQ handlers even. Could it be something
> like that ?
> 



Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
 

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Felipe Balbi]

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

hi,

On Tue, Sep 27, 2011 at 05:38:51PM +0300, Tero Kristo wrote:
> > On Tue, Sep 27, 2011 at 06:48:35PM +0530, Munegowda, Keshava wrote:
> > > > So, you would need a mechanism to do something like this:
> > > >
> > > > pad a or b wakeup detected -> irq0
> > > > pad c or d wakeup detected -> irq1?
> > > 
> > > yes, if get something like this , its perfect.
> > 
> > can't you have different IRQs for each pad ? I mean, allocate one
> > irq_desc for each pad and let drivers request a pad/pin as an IRQ
> > source. Then, when you detect a pad wakeup, you can:
> 
> Direct pad to irq mapping was turned down on some previous version of
> the prcm chain handler set, but yes, it is a potential approach. However

do you remember what was the reason for that being turned down ? Maybe I
can search the archives...

> the irq_desc allocation mechanism should be dynamic... there are some
> 200 programmable pads anyway.

I wouldn't worry too much about that. If you try to allocate irq_desc
dynamically, you would have to keep track of an "irq_base" for each
irq_desc you allocate, otherwise it's difficult to map it back to mux
number.

-- 
balbi

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

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

[Ming Lei]

Hi Paul,

On Fri, Sep 23, 2011 at 7:31 AM, Paul Walmsley <paul@pwsan.com> wrote:

> The idea of the main_clk was not intended to be PRCM or OCP or even
> OMAP-specific.  It's just intended to represent a clock that is used to
> drive the register logic inside the IP block.  Therefore it must be
> enabled before any register access may occur.  Even if clock gating is
> handled by some higher-level interface (e.g., at the IP block level), the
> main_clk has a rate, so it also implies an upper limit on how quickly
> register operations can occur.  I suppose that all of the IP block's
> clocks could be "optional clocks," but we know that every IP block with
> registers requires at least one clock to work, and that should be the
> main_clk.

I am a bit confused about you comment on "main_clk".

>From hwmod related source code, main_clk is the function clock
of one module(hwmod), such as: on omap4, for uart3, main_clk is
uart3_fck.

But from[1] and [2] of omap4 PRM, we can find that interface clock
is required to provide register access instead of function clock.

This is a bit conflictive with what you description, so could you
give a further comments about main_clk, function clock and interface
clock?


[1], 23.3.4.2 Clock Configuration
	Each UART uses a 48-MHz functional clock for its logic
	and to generate external interface signals. Each UART
	uses an interface clock for register accesses.

[2], 3.1.1.1.1 Module Interface and Functional Clocks
	The interface clocks have the following characteristics:
	• They ensure proper communication between any module/subsystem
	and the interconnect.
	• In most cases, they supply the system interconnect interface
	and registers of the module.

thanks,
-- 
Ming Lei

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Mon, Sep 26, 2011 at 8:15 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Mon, 26 Sep 2011, Munegowda, Keshava wrote:
>
>> On Sat, Sep 24, 2011 at 12:45 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>> >
>> >> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >> >
>> >> > On Thu, 22 Sep 2011, Keshava Munegowda wrote:
>> >> >> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
>> >> >>
>> >> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> >> >> Reviewed-by: Partha Basak <parthab@india.ti.com>
>> >> >> ---
>> >> >>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  271 ++++++++++++++++++++++++++++
>> >> >>  1 files changed, 271 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 59fdb9f..d79f728 100644
>> >> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> >> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> >
>> >> >> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
>> >> >> +     .clk            = "usbhost_120m_fck",
>> >> >
>> >> > This doesn't look right.  This is an interface structure record, so it
>> >> > should be associated with an interface clock.  Is the hardware really
>> >> > using the functional clock as the interface clock?  Or, as seems more
>> >> > likely...
>> >>
>> >>
>> >> Agreed, how about:
>> >>
>> >> main clock: usbhost_120m_fck
>> >> optional f clock: usbhost_48m_fck
>> >
>> > Assuming the interface clock is enabled, which one of these clocks is
>> > needed for UHH register accesses to complete successfully?
>>
>> it is usbhost_48m_fck ;
>> so,
>> main clock: usbhost_48m_fck
>> optional clock : usbhost_120m_fck
>>
>> do you agree?
>
> Yes.

Thanks paul,

But In USB Host case, there is a challenge!

I need both usbhost_48m_fck and usbhost_120m_fck to be turned on when
I invoke pm_runtime_get_sync ; so there are couple of options to do this

1. use clk_get with hard coded  usbhost_120m_fck name in  usbhs driver
       enable this clock after invoking pm_runtime_get_sync

2. add an additional flag in hwmod ; so that pm_runtime_get_sync will enable
           main clock as well as optional clocks

please comment on these two approaches..


regards
keshava

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

On Wed, 28 Sep 2011, Munegowda, Keshava wrote:

> Thanks paul,
> 
> But In USB Host case, there is a challenge!
> 
> I need both usbhost_48m_fck and usbhost_120m_fck to be turned on when
> I invoke pm_runtime_get_sync ; so there are couple of options to do this
> 
> 1. use clk_get with hard coded  usbhost_120m_fck name in  usbhs driver
>        enable this clock after invoking pm_runtime_get_sync
> 
> 2. add an additional flag in hwmod ; so that pm_runtime_get_sync will enable
>            main clock as well as optional clocks
> 
> please comment on these two approaches..

Looks like you picked approach #1 which is fine with me.  

Here's what I don't understand about how this clock is used, though.  
According to the 34xx TRM vZR Section 24.2.3.1.2 "High-Speed USB Host 
Subsystem Clocks", this is only used to "clock EHCI controller internal 
logic".  So if EHCI is not being used, it would seem to me that this clock 
shouldn't be enabled?  But patch #5 seems to enable and disable it 
unconditionally.  Am I missing something here?


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Fri, Sep 30, 2011 at 2:02 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Wed, 28 Sep 2011, Munegowda, Keshava wrote:
>
>> Thanks paul,
>>
>> But In USB Host case, there is a challenge!
>>
>> I need both usbhost_48m_fck and usbhost_120m_fck to be turned on when
>> I invoke pm_runtime_get_sync ; so there are couple of options to do this
>>
>> 1. use clk_get with hard coded  usbhost_120m_fck name in  usbhs driver
>>        enable this clock after invoking pm_runtime_get_sync
>>
>> 2. add an additional flag in hwmod ; so that pm_runtime_get_sync will enable
>>            main clock as well as optional clocks
>>
>> please comment on these two approaches..
>
> Looks like you picked approach #1 which is fine with me.
>
> Here's what I don't understand about how this clock is used, though.
> According to the 34xx TRM vZR Section 24.2.3.1.2 "High-Speed USB Host
> Subsystem Clocks", this is only used to "clock EHCI controller internal
> logic".  So if EHCI is not being used, it would seem to me that this clock
> shouldn't be enabled?  But patch #5 seems to enable and disable it
> unconditionally.  Am I missing something here?

OK. are you suggesting that this clock should be enabled/disabled; only
if atleast one port is selected for ehci?

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1530 bytes --]

On Fri, 30 Sep 2011, Munegowda, Keshava wrote:

> On Fri, Sep 30, 2011 at 2:02 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Wed, 28 Sep 2011, Munegowda, Keshava wrote:
> >
> >> Thanks paul,
> >>
> >> But In USB Host case, there is a challenge!
> >>
> >> I need both usbhost_48m_fck and usbhost_120m_fck to be turned on when
> >> I invoke pm_runtime_get_sync ; so there are couple of options to do this
> >>
> >> 1. use clk_get with hard coded  usbhost_120m_fck name in  usbhs driver
> >>        enable this clock after invoking pm_runtime_get_sync
> >>
> >> 2. add an additional flag in hwmod ; so that pm_runtime_get_sync will enable
> >>            main clock as well as optional clocks
> >>
> >> please comment on these two approaches..
> >
> > Looks like you picked approach #1 which is fine with me.
> >
> > Here's what I don't understand about how this clock is used, though.
> > According to the 34xx TRM vZR Section 24.2.3.1.2 "High-Speed USB Host
> > Subsystem Clocks", this is only used to "clock EHCI controller internal
> > logic".  So if EHCI is not being used, it would seem to me that this clock
> > shouldn't be enabled?  But patch #5 seems to enable and disable it
> > unconditionally.  Am I missing something here?
> 
> OK. are you suggesting that this clock should be enabled/disabled; only
> if atleast one port is selected for ehci?

Seems to make sense to me?  If the clock really isn't needed for OHCI, 
then it shouldn't be enabled when only OHCI is in use?


- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Fri, Sep 30, 2011 at 11:46 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Fri, 30 Sep 2011, Munegowda, Keshava wrote:
>
>> On Fri, Sep 30, 2011 at 2:02 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Wed, 28 Sep 2011, Munegowda, Keshava wrote:
>> >
>> >> Thanks paul,
>> >>
>> >> But In USB Host case, there is a challenge!
>> >>
>> >> I need both usbhost_48m_fck and usbhost_120m_fck to be turned on when
>> >> I invoke pm_runtime_get_sync ; so there are couple of options to do this
>> >>
>> >> 1. use clk_get with hard coded  usbhost_120m_fck name in  usbhs driver
>> >>        enable this clock after invoking pm_runtime_get_sync
>> >>
>> >> 2. add an additional flag in hwmod ; so that pm_runtime_get_sync will enable
>> >>            main clock as well as optional clocks
>> >>
>> >> please comment on these two approaches..
>> >
>> > Looks like you picked approach #1 which is fine with me.
>> >
>> > Here's what I don't understand about how this clock is used, though.
>> > According to the 34xx TRM vZR Section 24.2.3.1.2 "High-Speed USB Host
>> > Subsystem Clocks", this is only used to "clock EHCI controller internal
>> > logic".  So if EHCI is not being used, it would seem to me that this clock
>> > shouldn't be enabled?  But patch #5 seems to enable and disable it
>> > unconditionally.  Am I missing something here?
>>
>> OK. are you suggesting that this clock should be enabled/disabled; only
>> if atleast one port is selected for ehci?
>
> Seems to make sense to me?  If the clock really isn't needed for OHCI,
> then it shouldn't be enabled when only OHCI is in use?

thanks. I will make this :-)

regards
keshava

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

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3416 bytes --]

Hello Ming,

On Wed, 28 Sep 2011, Ming Lei wrote:

> On Fri, Sep 23, 2011 at 7:31 AM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> > The idea of the main_clk was not intended to be PRCM or OCP or even
> > OMAP-specific.  It's just intended to represent a clock that is used to
> > drive the register logic inside the IP block.  Therefore it must be
> > enabled before any register access may occur.  Even if clock gating is
> > handled by some higher-level interface (e.g., at the IP block level), the
> > main_clk has a rate, so it also implies an upper limit on how quickly
> > register operations can occur.  I suppose that all of the IP block's
> > clocks could be "optional clocks," but we know that every IP block with
> > registers requires at least one clock to work, and that should be the
> > main_clk.
> 
> I am a bit confused about you comment on "main_clk".
> 
> >From hwmod related source code, main_clk is the function clock
> of one module(hwmod), such as: on omap4, for uart3, main_clk is
> uart3_fck.
> 
> But from[1] and [2] of omap4 PRM, we can find that interface clock
> is required to provide register access instead of function clock.

As far as I know, the two cases that you cite are basically documentation 
bugs in the TRM.  In my experience, for IP blocks on OMAPs, a functional 
clock has to be enabled in order for register accesses to succeed.  It's 
possible there may be a few odd IP blocks that behave differently, but I 
can't think of any off the top of my head.

There are three cases that I've come across that might be interesting to 
you.

The first involves IP blocks that use their interface clock as their 
functional clock, such as the MAILBOXES IP block.  In this case, there is 
no separate functional clock that is needed for register accesses to 
complete, and therefore the main_clk should be the interface clock.

The second case involves IP blocks that can receive functional clocks from 
an off-chip source (i.e., not via a PRCM-controlled clock), such as the 
McBSP IP block.  For these IP blocks, it could be that register accesses 
might still succeed even if the module's PRCM-provided functional clock is 
off.  I haven't tested this personally.

The third case involves IP blocks with multiple functional clocks, such as 
the OMAP3 USBHOST subsystem.  What we found on OMAP3 was that only one of 
these two clocks needs to be enabled for register accesses to succeed.  
Some functional clocks might control parts of an IP block that have 
nothing to do with register access.

> This is a bit conflictive with what you description, so could you
> give a further comments about main_clk, function clock and interface
> clock?

I don't know why there are these documentation discrepancies.  I can 
guess, but probably I'd better not :-)  Hope the above helped.

> [1], 23.3.4.2 Clock Configuration
> 	Each UART uses a 48-MHz functional clock for its logic
> 	and to generate external interface signals. Each UART
> 	uses an interface clock for register accesses.
> 
> [2], 3.1.1.1.1 Module Interface and Functional Clocks
> 	The interface clocks have the following characteristics:
> 	• They ensure proper communication between any module/subsystem
> 	and the interconnect.
> 	• In most cases, they supply the system interconnect interface
> 	and registers of the module.

regards,

- Paul

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
<keshava_mgowda@ti.com> wrote:
> On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
>>> >
>> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
>>
>> -----Original Message-----
>>
>>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
>>> >Sent: Monday, September 26, 2011 7:49 PM
>>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
>>> >parthab@india.ti.com
>>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
>>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
>>> >vishwanath.bs@ti.com
>>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>>> >structures for omap3
>>> >
>>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
>>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>>> >>
>>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
>>> >wrote:
>>> >>>
>>> >>> But the question arises here , why do we need these ehci and ohci as
>>> >two
>>> >>> different hwmods containing only irq and base address? It is required
>>> >>> for future - to implement remote wakeup feature for ehci and ohci
>>> >ports
>>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
>>> >ehci
>>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
>>> >>> the wakeup source as ehci or ohci and call only the corresponding
>>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>>> >>> Depending on the particular wakeup source(ehci/ohci), the
>>> >corresponding
>>> >>> ehci or ohci irq handler will be called.
>>> >>>
>>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
>>> >then
>>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
>>> >>> interrupt handler will be called (please look at the function
>>> >>> omap_hwmod_mux_handle_irq of
>>> >>>
>>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>>> >>> , so in this
>>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
>>> >wakeup
>>> >>> , only ehci interrupt will get called; which will break the
>>> >functionality.
>>> >>
>>> >> Any reason why this couldn't be handled either by:
>>> >>
>>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
>>> >changing
>>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
>>> >
>>> >
>>> >yes, it is possible by changing the existing irq-chain handler by tero
>>> >Kristo
>>> >
>>> >I am looping tero too.
>>> >
>>> >So here are new requirements for the irq-chain handler
>>> >
>>> >1. The hwmod should have have option to have multiple mux structures
>>> >
>>> >This is something like:
>>> >
>>> >The existing mux structure definition in omap_hwmod [file:
>>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
>>> >
>>> >     struct omap_hwmod_mux_info      *mux;
>>> >
>>> >it should changed to
>>> >
>>> >     struct omap_hwmod_mux_info      **pmux;
>>> >         U32                                            mux_cnt;
>>> >
>>> >
>>> >pmux - pointers to mux ; array of mux structures.
>>> >mux_cnt - number of mux per hwmod.
>>> >
>>> >
>>> >2. The mux  omap_hwmod_mux_info  structure should have new member
>>> >called irq, like as follows:
>>> >
>>> >struct omap_hwmod_mux_info {
>>> >     int                             nr_pads;
>>> >     ...
>>> >        ....
>>> >        u32                           irq;
>>> >
>>> >};
>>> >
>>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
>>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
>>> >
>>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
>>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
>>> >
>>> >
>>> >if you , tero and other opensource people are aligned on the proposed
>>> >changes on the irq-handler ;
>>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
>>> >driver.
>>> >please let me know you comments on the above approach.
>>> >
>>>
>>> Hello Tero,
>>>
>>> I would like to draw your attention to the following thread:
>>>
>>> We need to support the following:
>>> 1. Ability to associate multiple mux info to a hwmod.
>>> 2. Able to associate a particular irq handler to a mux info.
>>> 3. PRCM Chain handler should loop through all mux-info arrays
>>>    for a particular hwmod to identify the possible wakeup source(s)
>>>    and call the appropriate irq handler for that mux-info.
>>>    (It is possible that both mux-info are woken up in which case both
>>> handlers should be called).
>>>
>>> To give you a little more perspective, EHCI & OHCI are co-controllers
>>> under UHH/TLL.
>>> They do not get presented separately to the OCP bus, hence do not qualify
>>> as separate hwmods
>>> (Paul had objected to the design approach representing EHCI & OHCI as
>>> different hwmods).
>>>
>>> However, we need a mechanism to efficiently identify/distinguish
>>> remote-wakeup, connect/disconnect
>>> On to an EHCI port vs an OHCI port & call only the correct interrupt
>>> handler(EHCI or OHCI).
>>>
>>>  To incorporate this, chain handler implementation would need some
>>> enhancements.
>>>  We can look into the details in the next merge window cycle in
>>> conjunction with aggressive clock management support for EHCI/OHCI.
>>>  But fundamentally, if you are aligned to the approach, we can go ahead
>>> collapsing the EHCI & OHCI hwmods into one.
>>
>> Hi,
>>
>> So, you would need a mechanism to do something like this:
>>
>> pad a or b wakeup detected -> irq0
>> pad c or d wakeup detected -> irq1?
>
> yes, if get something like this , its perfect.

Hi Tero
           Are you posting the patches with these changes?
please let me know when you will be able to post the patches.
so that I start the design of usb host remote wakeup using your changes.

regards
keshava




>
>>
>> Is it okay to do this:
>>
>> pad a...d wakeup -> irq0 and irq1?
>
> No, we dont need multiple irq handlers for single wakeup source.
>
>>
>> I am okay doing something like this, we just need to agree how this
>> would be represented from the hwmod point of view. Currently the chain
>> handler set does not change hwmod structures at all to provide what it
>> does.
>
> paul and benoit are the people to design the mechanisam for this.
>
> paul/Benoit
>            please give you thoughts on this.
>
> regards
> keshava
>
>
>>
>> -Tero
>>
>>> >
>>> >>
>>> >> or
>>> >>
>>> >> 2. using shared interrupts?
>>> >>
>>> >>
>>> >> - Paul
>>> >>
>>
>>
>>
>

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Tero Kristo]

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

On Thu, 2011-10-13 at 09:12 +0200, Munegowda, Keshava wrote:
> On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
> <keshava_mgowda@ti.com> wrote:
> > On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
> >>> >
> >> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
> >>
> >> 
Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
 
-----Original Message-----

> >>
> >>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
> >>> >Sent: Monday, September 26, 2011 7:49 PM
> >>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
> >>> >parthab@india.ti.com
> >>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
> >>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
> >>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
> >>> >vishwanath.bs@ti.com
> >>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
> >>> >structures for omap3
> >>> >
> >>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
> >>> >>
> >>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
> >>> >wrote:
> >>> >>>
> >>> >>> But the question arises here , why do we need these ehci and ohci as
> >>> >two
> >>> >>> different hwmods containing only irq and base address? It is required
> >>> >>> for future - to implement remote wakeup feature for ehci and ohci
> >>> >ports
> >>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
> >>> >ehci
> >>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
> >>> >>> the wakeup source as ehci or ohci and call only the corresponding
> >>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
> >>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
> >>> >>> Depending on the particular wakeup source(ehci/ohci), the
> >>> >corresponding
> >>> >>> ehci or ohci irq handler will be called.
> >>> >>>
> >>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
> >>> >then
> >>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
> >>> >>> interrupt handler will be called (please look at the function
> >>> >>> omap_hwmod_mux_handle_irq of
> >>> >>>
> >>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
> >>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
> >>> >>> , so in this
> >>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
> >>> >wakeup
> >>> >>> , only ehci interrupt will get called; which will break the
> >>> >functionality.
> >>> >>
> >>> >> Any reason why this couldn't be handled either by:
> >>> >>
> >>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
> >>> >changing
> >>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
> >>> >
> >>> >
> >>> >yes, it is possible by changing the existing irq-chain handler by tero
> >>> >Kristo
> >>> >
> >>> >I am looping tero too.
> >>> >
> >>> >So here are new requirements for the irq-chain handler
> >>> >
> >>> >1. The hwmod should have have option to have multiple mux structures
> >>> >
> >>> >This is something like:
> >>> >
> >>> >The existing mux structure definition in omap_hwmod [file:
> >>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
> >>> >
> >>> >     struct omap_hwmod_mux_info      *mux;
> >>> >
> >>> >it should changed to
> >>> >
> >>> >     struct omap_hwmod_mux_info      **pmux;
> >>> >         U32                                            mux_cnt;
> >>> >
> >>> >
> >>> >pmux - pointers to mux ; array of mux structures.
> >>> >mux_cnt - number of mux per hwmod.
> >>> >
> >>> >
> >>> >2. The mux  omap_hwmod_mux_info  structure should have new member
> >>> >called irq, like as follows:
> >>> >
> >>> >struct omap_hwmod_mux_info {
> >>> >     int                             nr_pads;
> >>> >     ...
> >>> >        ....
> >>> >        u32                           irq;
> >>> >
> >>> >};
> >>> >
> >>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
> >>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
> >>> >
> >>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
> >>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
> >>> >
> >>> >
> >>> >if you , tero and other opensource people are aligned on the proposed
> >>> >changes on the irq-handler ;
> >>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
> >>> >driver.
> >>> >please let me know you comments on the above approach.
> >>> >
> >>>
> >>> Hello Tero,
> >>>
> >>> I would like to draw your attention to the following thread:
> >>>
> >>> We need to support the following:
> >>> 1. Ability to associate multiple mux info to a hwmod.
> >>> 2. Able to associate a particular irq handler to a mux info.
> >>> 3. PRCM Chain handler should loop through all mux-info arrays
> >>>    for a particular hwmod to identify the possible wakeup source(s)
> >>>    and call the appropriate irq handler for that mux-info.
> >>>    (It is possible that both mux-info are woken up in which case both
> >>> handlers should be called).
> >>>
> >>> To give you a little more perspective, EHCI & OHCI are co-controllers
> >>> under UHH/TLL.
> >>> They do not get presented separately to the OCP bus, hence do not qualify
> >>> as separate hwmods
> >>> (Paul had objected to the design approach representing EHCI & OHCI as
> >>> different hwmods).
> >>>
> >>> However, we need a mechanism to efficiently identify/distinguish
> >>> remote-wakeup, connect/disconnect
> >>> On to an EHCI port vs an OHCI port & call only the correct interrupt
> >>> handler(EHCI or OHCI).
> >>>
> >>>  To incorporate this, chain handler implementation would need some
> >>> enhancements.
> >>>  We can look into the details in the next merge window cycle in
> >>> conjunction with aggressive clock management support for EHCI/OHCI.
> >>>  But fundamentally, if you are aligned to the approach, we can go ahead
> >>> collapsing the EHCI & OHCI hwmods into one.
> >>
> >> Hi,
> >>
> >> So, you would need a mechanism to do something like this:
> >>
> >> pad a or b wakeup detected -> irq0
> >> pad c or d wakeup detected -> irq1?
> >
> > yes, if get something like this , its perfect.
> 
> Hi Tero
>            Are you posting the patches with these changes?
> please let me know when you will be able to post the patches.
> so that I start the design of usb host remote wakeup using your changes.
> 
> regards
> keshava
> 

Sorry for the delay, but I am still not quite sure how this should be
handled for upstream. Attached is a proposal hack patch that should
work, you can at least try it out to see what happens. It applies on top
of my latest PRCM chain handler set (version 9.) Patch desc contains a
guide how to use it.

-Tero


> 
> 
> 
> >
> >>
> >> Is it okay to do this:
> >>
> >> pad a...d wakeup -> irq0 and irq1?
> >
> > No, we dont need multiple irq handlers for single wakeup source.
> >
> >>
> >> I am okay doing something like this, we just need to agree how this
> >> would be represented from the hwmod point of view. Currently the chain
> >> handler set does not change hwmod structures at all to provide what it
> >> does.
> >
> > paul and benoit are the people to design the mechanisam for this.
> >
> > paul/Benoit
> >            please give you thoughts on this.
> >
> > regards
> > keshava
> >
> >
> >>
> >> -Tero
> >>
> >>> >
> >>> >>
> >>> >> or
> >>> >>
> >>> >> 2. using shared interrupts?
> >>> >>
> >>> >>
> >>> >> - Paul
> >>> >>
> >>
> >>
> >>
> >


[-- Attachment #2: 0001-HACK-omap-mux-add-support-for-separate-wakeup-irq-fo.patch --]
[-- Type: text/x-patch, Size: 3849 bytes --]

>From fff5f3ef281eafe2229811eaa2661176e9582db2 Mon Sep 17 00:00:00 2001
From: Tero Kristo <t-kristo@ti.com>
Date: Thu, 13 Oct 2011 14:21:09 +0300
Subject: [PATCH] HACK: omap: mux: add support for separate wakeup irq for each pad

By default all registered pads will trigger mpu_irqs[0]. This patch
adds an API that allows separate irqs for each pad.

Usage:

int irqs[nr_pads] = { 1, 2, 3 };
...
mux = omap_hwmod_mux_init(pads, nr_pads);
omap_hwmod_mux_init_irqs(mux, irqs);

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/mux.c                    |   21 +++++++++++++++------
 arch/arm/mach-omap2/mux.h                    |    2 ++
 arch/arm/mach-omap2/omap_hwmod.c             |    7 -------
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 642fe3f..e9ac904 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -354,6 +354,14 @@ err1:
 	return NULL;
 }
 
+int omap_hwmod_mux_init_irqs(struct omap_hwmod_mux_info *mux, int *irqs)
+{
+	if (!mux || !irqs)
+		return -EINVAL;
+	mux->irqs = irqs;
+	return 0;
+}
+
 /**
  * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
  * @hmux:		Pads for a hwmod
@@ -362,11 +370,10 @@ err1:
  * Returns true if wakeup event is set for pad else false
  * if wakeup is not occured or pads are not avialable.
  */
-bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+static bool omap_hwmod_mux_scan_wakeups(struct omap_hwmod_mux_info *hmux)
 {
 	int i;
 	unsigned int val;
-	u8 ret = false;
 
 	for (i = 0; i < hmux->nr_pads; i++) {
 		struct omap_device_pad *pad = &hmux->pads[i];
@@ -375,15 +382,17 @@ bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
 			val = omap_mux_read(pad->partition,
 					pad->mux->reg_offset);
 			if (val & OMAP_WAKEUP_EVENT) {
-				ret = true;
 				pr_info("wkup detected: %04x\n",
 					pad->mux->reg_offset);
-				break;
+				if (hmux->irqs)
+					generic_handle_irq(hmux->irqs[i]);
+				else
+					return true;
 			}
 		}
 	}
 
-	return ret;
+	return false;
 }
 
 /**
@@ -396,7 +405,7 @@ static int _omap_hwmod_mux_handle_irq(struct omap_hwmod *oh, void *data)
 {
 	if (!oh->mux || !oh->mux->enabled)
 		return 0;
-	if (omap_hwmod_mux_get_wake_status(oh->mux))
+	if (omap_hwmod_mux_scan_wakeups(oh->mux))
 		generic_handle_irq(oh->mpu_irqs[0].irq);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
index 8b2150a..24e1981 100644
--- a/arch/arm/mach-omap2/mux.h
+++ b/arch/arm/mach-omap2/mux.h
@@ -216,6 +216,8 @@ int omap_mux_init_signal(const char *muxname, int val);
 extern struct omap_hwmod_mux_info *
 omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
 
+extern int omap_hwmod_mux_init_irqs(struct omap_hwmod_mux_info *mux, int *irqs);
+
 /**
  * omap_hwmod_mux - omap hwmod specific pin muxing
  * @hmux:		Pads for a hwmod
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index a8b24d7..e751dd9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2724,10 +2724,3 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod *oh)
 
 	return 0;
 }
-
-int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
-{
-	if (oh && oh->mux)
-		return omap_hwmod_mux_get_wake_status(oh->mux);
-	return -EINVAL;
-}
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 9c70cc8..2a9ce37 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -97,6 +97,7 @@ struct omap_hwmod_mux_info {
 	struct omap_device_pad		*pads;
 	int				nr_pads_dynamic;
 	struct omap_device_pad		**pads_dynamic;
+	int				*irqs;
 	bool				enabled;
 };
 
-- 
1.7.4.1

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Thu, Oct 13, 2011 at 4:58 PM, Tero Kristo <t-kristo@ti.com> wrote:
> On Thu, 2011-10-13 at 09:12 +0200, Munegowda, Keshava wrote:
>> On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
>> <keshava_mgowda@ti.com> wrote:
>> > On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
>> >>> >
>> >> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
>> >>
>> >>
> Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
>
> -----Original Message-----
>
>> >>
>> >>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
>> >>> >Sent: Monday, September 26, 2011 7:49 PM
>> >>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
>> >>> >parthab@india.ti.com
>> >>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>> >>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
>> >>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
>> >>> >vishwanath.bs@ti.com
>> >>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>> >>> >structures for omap3
>> >>> >
>> >>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>> >>> >>
>> >>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
>> >>> >wrote:
>> >>> >>>
>> >>> >>> But the question arises here , why do we need these ehci and ohci as
>> >>> >two
>> >>> >>> different hwmods containing only irq and base address? It is required
>> >>> >>> for future - to implement remote wakeup feature for ehci and ohci
>> >>> >ports
>> >>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
>> >>> >ehci
>> >>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
>> >>> >>> the wakeup source as ehci or ohci and call only the corresponding
>> >>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>> >>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>> >>> >>> Depending on the particular wakeup source(ehci/ohci), the
>> >>> >corresponding
>> >>> >>> ehci or ohci irq handler will be called.
>> >>> >>>
>> >>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
>> >>> >then
>> >>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
>> >>> >>> interrupt handler will be called (please look at the function
>> >>> >>> omap_hwmod_mux_handle_irq of
>> >>> >>>
>> >>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>> >>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>> >>> >>> , so in this
>> >>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
>> >>> >wakeup
>> >>> >>> , only ehci interrupt will get called; which will break the
>> >>> >functionality.
>> >>> >>
>> >>> >> Any reason why this couldn't be handled either by:
>> >>> >>
>> >>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
>> >>> >changing
>> >>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
>> >>> >
>> >>> >
>> >>> >yes, it is possible by changing the existing irq-chain handler by tero
>> >>> >Kristo
>> >>> >
>> >>> >I am looping tero too.
>> >>> >
>> >>> >So here are new requirements for the irq-chain handler
>> >>> >
>> >>> >1. The hwmod should have have option to have multiple mux structures
>> >>> >
>> >>> >This is something like:
>> >>> >
>> >>> >The existing mux structure definition in omap_hwmod [file:
>> >>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
>> >>> >
>> >>> >     struct omap_hwmod_mux_info      *mux;
>> >>> >
>> >>> >it should changed to
>> >>> >
>> >>> >     struct omap_hwmod_mux_info      **pmux;
>> >>> >         U32                                            mux_cnt;
>> >>> >
>> >>> >
>> >>> >pmux - pointers to mux ; array of mux structures.
>> >>> >mux_cnt - number of mux per hwmod.
>> >>> >
>> >>> >
>> >>> >2. The mux  omap_hwmod_mux_info  structure should have new member
>> >>> >called irq, like as follows:
>> >>> >
>> >>> >struct omap_hwmod_mux_info {
>> >>> >     int                             nr_pads;
>> >>> >     ...
>> >>> >        ....
>> >>> >        u32                           irq;
>> >>> >
>> >>> >};
>> >>> >
>> >>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
>> >>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
>> >>> >
>> >>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
>> >>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
>> >>> >
>> >>> >
>> >>> >if you , tero and other opensource people are aligned on the proposed
>> >>> >changes on the irq-handler ;
>> >>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
>> >>> >driver.
>> >>> >please let me know you comments on the above approach.
>> >>> >
>> >>>
>> >>> Hello Tero,
>> >>>
>> >>> I would like to draw your attention to the following thread:
>> >>>
>> >>> We need to support the following:
>> >>> 1. Ability to associate multiple mux info to a hwmod.
>> >>> 2. Able to associate a particular irq handler to a mux info.
>> >>> 3. PRCM Chain handler should loop through all mux-info arrays
>> >>>    for a particular hwmod to identify the possible wakeup source(s)
>> >>>    and call the appropriate irq handler for that mux-info.
>> >>>    (It is possible that both mux-info are woken up in which case both
>> >>> handlers should be called).
>> >>>
>> >>> To give you a little more perspective, EHCI & OHCI are co-controllers
>> >>> under UHH/TLL.
>> >>> They do not get presented separately to the OCP bus, hence do not qualify
>> >>> as separate hwmods
>> >>> (Paul had objected to the design approach representing EHCI & OHCI as
>> >>> different hwmods).
>> >>>
>> >>> However, we need a mechanism to efficiently identify/distinguish
>> >>> remote-wakeup, connect/disconnect
>> >>> On to an EHCI port vs an OHCI port & call only the correct interrupt
>> >>> handler(EHCI or OHCI).
>> >>>
>> >>>  To incorporate this, chain handler implementation would need some
>> >>> enhancements.
>> >>>  We can look into the details in the next merge window cycle in
>> >>> conjunction with aggressive clock management support for EHCI/OHCI.
>> >>>  But fundamentally, if you are aligned to the approach, we can go ahead
>> >>> collapsing the EHCI & OHCI hwmods into one.
>> >>
>> >> Hi,
>> >>
>> >> So, you would need a mechanism to do something like this:
>> >>
>> >> pad a or b wakeup detected -> irq0
>> >> pad c or d wakeup detected -> irq1?
>> >
>> > yes, if get something like this , its perfect.
>>
>> Hi Tero
>>            Are you posting the patches with these changes?
>> please let me know when you will be able to post the patches.
>> so that I start the design of usb host remote wakeup using your changes.
>>
>> regards
>> keshava
>>
>
> Sorry for the delay, but I am still not quite sure how this should be
> handled for upstream. Attached is a proposal hack patch that should
> work, you can at least try it out to see what happens. It applies on top
> of my latest PRCM chain handler set (version 9.) Patch desc contains a
> guide how to use it.
>
> -Tero


Thanks tero,
          we are still in the internal design discussion of usbhs remote wakeup.
but, we are looking for the upstreamable code.
I request please send it as formal patch so that we will align with
Paul, benoit, kevin
Felipe and all other USB and PM experts.

regards
keshava

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Tero Kristo]

Hi Again,

I created a new version of the patch which should be better than this
hack, I'll send it as an RFC to the l-o list in a bit.

On Thu, 2011-10-13 at 13:49 +0200, Munegowda, Keshava wrote:
> On Thu, Oct 13, 2011 at 4:58 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > On Thu, 2011-10-13 at 09:12 +0200, Munegowda, Keshava wrote:
> >> On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
> >> <keshava_mgowda@ti.com> wrote:
> >> > On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >> >> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
> >> >>> >
> >> >> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
> >> >>
> >> >>
> > Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
> >
> > 
Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
 
-----Original Message-----

> >
> >> >>
> >> >>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
> >> >>> >Sent: Monday, September 26, 2011 7:49 PM
> >> >>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
> >> >>> >parthab@india.ti.com
> >> >>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
> >> >>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
> >> >>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
> >> >>> >vishwanath.bs@ti.com
> >> >>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
> >> >>> >structures for omap3
> >> >>> >
> >> >>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> >>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
> >> >>> >>
> >> >>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
> >> >>> >wrote:
> >> >>> >>>
> >> >>> >>> But the question arises here , why do we need these ehci and ohci as
> >> >>> >two
> >> >>> >>> different hwmods containing only irq and base address? It is required
> >> >>> >>> for future - to implement remote wakeup feature for ehci and ohci
> >> >>> >ports
> >> >>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
> >> >>> >ehci
> >> >>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
> >> >>> >>> the wakeup source as ehci or ohci and call only the corresponding
> >> >>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
> >> >>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
> >> >>> >>> Depending on the particular wakeup source(ehci/ohci), the
> >> >>> >corresponding
> >> >>> >>> ehci or ohci irq handler will be called.
> >> >>> >>>
> >> >>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
> >> >>> >then
> >> >>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
> >> >>> >>> interrupt handler will be called (please look at the function
> >> >>> >>> omap_hwmod_mux_handle_irq of
> >> >>> >>>
> >> >>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
> >> >>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
> >> >>> >>> , so in this
> >> >>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
> >> >>> >wakeup
> >> >>> >>> , only ehci interrupt will get called; which will break the
> >> >>> >functionality.
> >> >>> >>
> >> >>> >> Any reason why this couldn't be handled either by:
> >> >>> >>
> >> >>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
> >> >>> >changing
> >> >>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
> >> >>> >
> >> >>> >
> >> >>> >yes, it is possible by changing the existing irq-chain handler by tero
> >> >>> >Kristo
> >> >>> >
> >> >>> >I am looping tero too.
> >> >>> >
> >> >>> >So here are new requirements for the irq-chain handler
> >> >>> >
> >> >>> >1. The hwmod should have have option to have multiple mux structures
> >> >>> >
> >> >>> >This is something like:
> >> >>> >
> >> >>> >The existing mux structure definition in omap_hwmod [file:
> >> >>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
> >> >>> >
> >> >>> >     struct omap_hwmod_mux_info      *mux;
> >> >>> >
> >> >>> >it should changed to
> >> >>> >
> >> >>> >     struct omap_hwmod_mux_info      **pmux;
> >> >>> >         U32                                            mux_cnt;
> >> >>> >
> >> >>> >
> >> >>> >pmux - pointers to mux ; array of mux structures.
> >> >>> >mux_cnt - number of mux per hwmod.
> >> >>> >
> >> >>> >
> >> >>> >2. The mux  omap_hwmod_mux_info  structure should have new member
> >> >>> >called irq, like as follows:
> >> >>> >
> >> >>> >struct omap_hwmod_mux_info {
> >> >>> >     int                             nr_pads;
> >> >>> >     ...
> >> >>> >        ....
> >> >>> >        u32                           irq;
> >> >>> >
> >> >>> >};
> >> >>> >
> >> >>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
> >> >>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
> >> >>> >
> >> >>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
> >> >>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
> >> >>> >
> >> >>> >
> >> >>> >if you , tero and other opensource people are aligned on the proposed
> >> >>> >changes on the irq-handler ;
> >> >>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
> >> >>> >driver.
> >> >>> >please let me know you comments on the above approach.
> >> >>> >
> >> >>>
> >> >>> Hello Tero,
> >> >>>
> >> >>> I would like to draw your attention to the following thread:
> >> >>>
> >> >>> We need to support the following:
> >> >>> 1. Ability to associate multiple mux info to a hwmod.
> >> >>> 2. Able to associate a particular irq handler to a mux info.
> >> >>> 3. PRCM Chain handler should loop through all mux-info arrays
> >> >>>    for a particular hwmod to identify the possible wakeup source(s)
> >> >>>    and call the appropriate irq handler for that mux-info.
> >> >>>    (It is possible that both mux-info are woken up in which case both
> >> >>> handlers should be called).
> >> >>>
> >> >>> To give you a little more perspective, EHCI & OHCI are co-controllers
> >> >>> under UHH/TLL.
> >> >>> They do not get presented separately to the OCP bus, hence do not qualify
> >> >>> as separate hwmods
> >> >>> (Paul had objected to the design approach representing EHCI & OHCI as
> >> >>> different hwmods).
> >> >>>
> >> >>> However, we need a mechanism to efficiently identify/distinguish
> >> >>> remote-wakeup, connect/disconnect
> >> >>> On to an EHCI port vs an OHCI port & call only the correct interrupt
> >> >>> handler(EHCI or OHCI).
> >> >>>
> >> >>>  To incorporate this, chain handler implementation would need some
> >> >>> enhancements.
> >> >>>  We can look into the details in the next merge window cycle in
> >> >>> conjunction with aggressive clock management support for EHCI/OHCI.
> >> >>>  But fundamentally, if you are aligned to the approach, we can go ahead
> >> >>> collapsing the EHCI & OHCI hwmods into one.
> >> >>
> >> >> Hi,
> >> >>
> >> >> So, you would need a mechanism to do something like this:
> >> >>
> >> >> pad a or b wakeup detected -> irq0
> >> >> pad c or d wakeup detected -> irq1?
> >> >
> >> > yes, if get something like this , its perfect.
> >>
> >> Hi Tero
> >>            Are you posting the patches with these changes?
> >> please let me know when you will be able to post the patches.
> >> so that I start the design of usb host remote wakeup using your changes.
> >>
> >> regards
> >> keshava
> >>
> >
> > Sorry for the delay, but I am still not quite sure how this should be
> > handled for upstream. Attached is a proposal hack patch that should
> > work, you can at least try it out to see what happens. It applies on top
> > of my latest PRCM chain handler set (version 9.) Patch desc contains a
> > guide how to use it.
> >
> > -Tero
> 
> 
> Thanks tero,
>           we are still in the internal design discussion of usbhs remote wakeup.
> but, we are looking for the upstreamable code.
> I request please send it as formal patch so that we will align with
> Paul, benoit, kevin
> Felipe and all other USB and PM experts.
> 
> regards
> keshava

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Munegowda, Keshava]

On Fri, Oct 28, 2011 at 5:33 PM, Tero Kristo <t-kristo@ti.com> wrote:
> Hi Again,
>
> I created a new version of the patch which should be better than this
> hack, I'll send it as an RFC to the l-o list in a bit.

cool ! our discussion helped me.
please send the patch..



> On Thu, 2011-10-13 at 13:49 +0200, Munegowda, Keshava wrote:
>> On Thu, Oct 13, 2011 at 4:58 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> > On Thu, 2011-10-13 at 09:12 +0200, Munegowda, Keshava wrote:
>> >> On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
>> >> <keshava_mgowda@ti.com> wrote:
>> >> > On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
>> >> >> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
>> >> >>> >
>> >> >> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
>> >> >>
>> >> >>
>> > Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
>> >
>> >
> Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
>
> -----Original Message-----
>
>> >
>> >> >>
>> >> >>> >From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
>> >> >>> >Sent: Monday, September 26, 2011 7:49 PM
>> >> >>> >To: Paul Walmsley; Tero Kristo; b-cousson@ti.com; balbi@ti.com;
>> >> >>> >parthab@india.ti.com
>> >> >>> >Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>> >> >>> >kernel@vger.kernel.org; gadiyar@ti.com; sameo@linux.intel.com;
>> >> >>> >tony@atomide.com; khilman@ti.com; johnstul@us.ibm.com;
>> >> >>> >vishwanath.bs@ti.com
>> >> >>> >Subject: Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod
>> >> >>> >structures for omap3
>> >> >>> >
>> >> >>> >On Sat, Sep 24, 2011 at 12:00 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >> >>> >> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>> >> >>> >>
>> >> >>> >>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@pwsan.com>
>> >> >>> >wrote:
>> >> >>> >>>
>> >> >>> >>> But the question arises here , why do we need these ehci and ohci as
>> >> >>> >two
>> >> >>> >>> different hwmods containing only irq and base address? It is required
>> >> >>> >>> for future - to implement remote wakeup feature for ehci and ohci
>> >> >>> >ports
>> >> >>> >>> depending on irq-chain handler patches by Tero. Separate hwmods for
>> >> >>> >ehci
>> >> >>> >>> and ohci are needed to enable prcm chain-handler to uniquely identify
>> >> >>> >>> the wakeup source as ehci or ohci and call only the corresponding
>> >> >>> >>> interrupt handler. We will be using omap_hwmod_mux_init for ehci and
>> >> >>> >>> ohci hwmods to enable I/O wakeup capability for respective IO-pads.
>> >> >>> >>> Depending on the particular wakeup source(ehci/ohci), the
>> >> >>> >corresponding
>> >> >>> >>> ehci or ohci irq handler will be called.
>> >> >>> >>>
>> >> >>> >>> If ehci and ohci are combined with usbhs hwmod as a single hwmod ,
>> >> >>> >then
>> >> >>> >>> for every wakeup (either ehci or ohci port wakeup) only the first
>> >> >>> >>> interrupt handler will be called (please look at the function
>> >> >>> >>> omap_hwmod_mux_handle_irq of
>> >> >>> >>>
>> >> >>> >>> /arch/arm/mach-omap2/mux.c file ; in tero's latest patch:
>> >> >>> >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg53139.html)
>> >> >>> >>> , so in this
>> >> >>> >>> case, if ehci interrupt is the first interrupt , then even for ohci
>> >> >>> >wakeup
>> >> >>> >>> , only ehci interrupt will get called; which will break the
>> >> >>> >functionality.
>> >> >>> >>
>> >> >>> >> Any reason why this couldn't be handled either by:
>> >> >>> >>
>> >> >>> >> 1. adding an IRQ number field to struct omap_hwmod_mux_info, and
>> >> >>> >changing
>> >> >>> >> _omap_hwmod_mux_handle_irq() to raise that IRQ number?
>> >> >>> >
>> >> >>> >
>> >> >>> >yes, it is possible by changing the existing irq-chain handler by tero
>> >> >>> >Kristo
>> >> >>> >
>> >> >>> >I am looping tero too.
>> >> >>> >
>> >> >>> >So here are new requirements for the irq-chain handler
>> >> >>> >
>> >> >>> >1. The hwmod should have have option to have multiple mux structures
>> >> >>> >
>> >> >>> >This is something like:
>> >> >>> >
>> >> >>> >The existing mux structure definition in omap_hwmod [file:
>> >> >>> >/arch/arm/plat-omap/include/plat/omap_hwmod.h ] structure
>> >> >>> >
>> >> >>> >     struct omap_hwmod_mux_info      *mux;
>> >> >>> >
>> >> >>> >it should changed to
>> >> >>> >
>> >> >>> >     struct omap_hwmod_mux_info      **pmux;
>> >> >>> >         U32                                            mux_cnt;
>> >> >>> >
>> >> >>> >
>> >> >>> >pmux - pointers to mux ; array of mux structures.
>> >> >>> >mux_cnt - number of mux per hwmod.
>> >> >>> >
>> >> >>> >
>> >> >>> >2. The mux  omap_hwmod_mux_info  structure should have new member
>> >> >>> >called irq, like as follows:
>> >> >>> >
>> >> >>> >struct omap_hwmod_mux_info {
>> >> >>> >     int                             nr_pads;
>> >> >>> >     ...
>> >> >>> >        ....
>> >> >>> >        u32                           irq;
>> >> >>> >
>> >> >>> >};
>> >> >>> >
>> >> >>> >Upon wakeup of the I/O pad of the mux , the irq-chain handler should
>> >> >>> >invoke the irq handler of the irq numbered <map_hwmod_mux_info.irq>
>> >> >>> >
>> >> >>> >3.  There should be "SOME WAY" to supply the irqs  from hwmod
>> >> >>> >structure (omap_hwmod) to mux structure (omap_hwmod_mux_info)
>> >> >>> >
>> >> >>> >
>> >> >>> >if you , tero and other opensource people are aligned on the proposed
>> >> >>> >changes on the irq-handler ;
>> >> >>> >then it is possible to have two hwmods ( usbhs and tll) for usbhost
>> >> >>> >driver.
>> >> >>> >please let me know you comments on the above approach.
>> >> >>> >
>> >> >>>
>> >> >>> Hello Tero,
>> >> >>>
>> >> >>> I would like to draw your attention to the following thread:
>> >> >>>
>> >> >>> We need to support the following:
>> >> >>> 1. Ability to associate multiple mux info to a hwmod.
>> >> >>> 2. Able to associate a particular irq handler to a mux info.
>> >> >>> 3. PRCM Chain handler should loop through all mux-info arrays
>> >> >>>    for a particular hwmod to identify the possible wakeup source(s)
>> >> >>>    and call the appropriate irq handler for that mux-info.
>> >> >>>    (It is possible that both mux-info are woken up in which case both
>> >> >>> handlers should be called).
>> >> >>>
>> >> >>> To give you a little more perspective, EHCI & OHCI are co-controllers
>> >> >>> under UHH/TLL.
>> >> >>> They do not get presented separately to the OCP bus, hence do not qualify
>> >> >>> as separate hwmods
>> >> >>> (Paul had objected to the design approach representing EHCI & OHCI as
>> >> >>> different hwmods).
>> >> >>>
>> >> >>> However, we need a mechanism to efficiently identify/distinguish
>> >> >>> remote-wakeup, connect/disconnect
>> >> >>> On to an EHCI port vs an OHCI port & call only the correct interrupt
>> >> >>> handler(EHCI or OHCI).
>> >> >>>
>> >> >>>  To incorporate this, chain handler implementation would need some
>> >> >>> enhancements.
>> >> >>>  We can look into the details in the next merge window cycle in
>> >> >>> conjunction with aggressive clock management support for EHCI/OHCI.
>> >> >>>  But fundamentally, if you are aligned to the approach, we can go ahead
>> >> >>> collapsing the EHCI & OHCI hwmods into one.
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> So, you would need a mechanism to do something like this:
>> >> >>
>> >> >> pad a or b wakeup detected -> irq0
>> >> >> pad c or d wakeup detected -> irq1?
>> >> >
>> >> > yes, if get something like this , its perfect.
>> >>
>> >> Hi Tero
>> >>            Are you posting the patches with these changes?
>> >> please let me know when you will be able to post the patches.
>> >> so that I start the design of usb host remote wakeup using your changes.
>> >>
>> >> regards
>> >> keshava
>> >>
>> >
>> > Sorry for the delay, but I am still not quite sure how this should be
>> > handled for upstream. Attached is a proposal hack patch that should
>> > work, you can at least try it out to see what happens. It applies on top
>> > of my latest PRCM chain handler set (version 9.) Patch desc contains a
>> > guide how to use it.
>> >
>> > -Tero
>>
>>
>> Thanks tero,
>>           we are still in the internal design discussion of usbhs remote wakeup.
>> but, we are looking for the upstreamable code.
>> I request please send it as formal patch so that we will align with
>> Paul, benoit, kevin
>> Felipe and all other USB and PM experts.
>>
>> regards
>> keshava
>
>
>

Re: [PATCH 2/5 v11] arm: omap: usb: ehci and ohci hwmod structures for omap3

[Valdis.Kletnieks]

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

On Fri, 28 Oct 2011 15:03:36 +0300, Tero Kristo said:
> Hi Again,

> I created a new version of the patch which should be better than this
> hack, I'll send it as an RFC to the l-o list in a bit.
> On Thu, 2011-10-13 at 13:49 +0200, Munegowda, Keshava wrote:
> > On Thu, Oct 13, 2011 at 4:58 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > > On Thu, 2011-10-13 at 09:12 +0200, Munegowda, Keshava wrote:
> > >> On Tue, Sep 27, 2011 at 6:48 PM, Munegowda, Keshava
> > >> <keshava_mgowda@ti.com> wrote:
> > >> > On Tue, Sep 27, 2011 at 6:12 PM, Tero Kristo <t-kristo@ti.com> wrote:
> > >> >> On Tue, 2011-09-27 at 08:04 +0200, Basak, Partha wrote:
> > >> >> Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
> > > Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki
> Texas Instruments Oy, Porkkalankatu 22, 00180 Helsinki, Finland. Business ID: 0115040-6. Domicile: Helsinki

Moral: Just leave current street addresses out of it. :)

And can we *please* trim irrelevant stuff?  You top-posted a 2 line reply,
followed by the entire note, which a bunch of kernel developers got to scroll
through and wonder if they missed an in-line comment. *Especially* after the
top part had one line that it wasn't clear if it was a top-posted sig line gone
wrong, or 3 attempts to get an address right for inclusion in a patch.


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