[PATCH 0/2] arm: omap3: am35x: Convert emac to hwmod & disable hlt when open

[PATCH 0/2] arm: omap3: am35x: Convert emac to hwmod & disable hlt when open

[Mark A. Greer]

From: "Mark A. Greer" <mgreer@animalcreek.com>

Paul, Kevin,

These patches convert the davinci emac support for the am35x SoC
to use hwmod and add enable_hlt()/disable_hlt() calls to the
pm_runtime hooks for that driver.

I have converted the davinci_emac driver to use pm_runtime but I
can't formally submit it yet since it requires some changes on the
mach-davinci side that haven't happened yet.  I will send it as an
RFC in a reply to this thread.

The patches are based on:
	git://git.pwsan.com/linux-2.6 am35xx_hwmod_data_fixes_a_3.5

Mark
--

Mark A. Greer (2):
  arm: omap3: am35x: Add Davinci EMAC/MDIO hwmod support
  arm: omap3: am35x: Disable hlt when using Davinci EMAC

 arch/arm/mach-omap2/am35xx-emac.c          |  120 ++++++++++++++++++----------
 arch/arm/mach-omap2/am35xx-emac.h          |   16 +++-
 arch/arm/mach-omap2/board-am3517evm.c      |    3 +-
 arch/arm/mach-omap2/board-cm-t3517.c       |    3 +-
 arch/arm/mach-omap2/clock3xxx_data.c       |    2 +-
 arch/arm/mach-omap2/include/mach/am35xx.h  |    2 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   85 ++++++++++++++++++++
 7 files changed, 183 insertions(+), 48 deletions(-)

-- 
1.7.9.4

[PATCH 1/2] arm: omap3: am35x: Add Davinci EMAC/MDIO hwmod support

[Mark A. Greer]

From: "Mark A. Greer" <mgreer@animalcreek.com>

Add hwmod support for the EMAC (and MDIO)
ethernet controller that's on the am35x
family of SoC's.

Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
---
 arch/arm/mach-omap2/am35xx-emac.c          |   92 ++++++++++++++--------------
 arch/arm/mach-omap2/clock3xxx_data.c       |    2 +-
 arch/arm/mach-omap2/include/mach/am35xx.h  |    2 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   85 +++++++++++++++++++++++++
 4 files changed, 135 insertions(+), 46 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 1f97e74..3bb5cb3 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -15,27 +15,13 @@
  * General Public License for more details.
  */
 
-#include <linux/clk.h>
+#include <linux/err.h>
 #include <linux/davinci_emac.h>
-#include <linux/platform_device.h>
-#include <plat/irqs.h>
+#include <asm/system.h>
+#include <plat/omap_device.h>
 #include <mach/am35xx.h>
-
 #include "control.h"
-
-static struct mdio_platform_data am35xx_emac_mdio_pdata;
-
-static struct resource am35xx_emac_mdio_resources[] = {
-	DEFINE_RES_MEM(AM35XX_IPSS_EMAC_BASE + AM35XX_EMAC_MDIO_OFFSET, SZ_4K),
-};
-
-static struct platform_device am35xx_emac_mdio_device = {
-	.name		= "davinci_mdio",
-	.id		= 0,
-	.num_resources	= ARRAY_SIZE(am35xx_emac_mdio_resources),
-	.resource	= am35xx_emac_mdio_resources,
-	.dev.platform_data = &am35xx_emac_mdio_pdata,
-};
+#include "am35xx-emac.h"
 
 static void am35xx_enable_emac_int(void)
 {
@@ -72,41 +58,57 @@ static struct emac_platform_data am35xx_emac_pdata = {
 	.interrupt_disable	= am35xx_disable_emac_int,
 };
 
-static struct resource am35xx_emac_resources[] = {
-	DEFINE_RES_MEM(AM35XX_IPSS_EMAC_BASE, 0x30000),
-	DEFINE_RES_IRQ(INT_35XX_EMAC_C0_RXTHRESH_IRQ),
-	DEFINE_RES_IRQ(INT_35XX_EMAC_C0_RX_PULSE_IRQ),
-	DEFINE_RES_IRQ(INT_35XX_EMAC_C0_TX_PULSE_IRQ),
-	DEFINE_RES_IRQ(INT_35XX_EMAC_C0_MISC_PULSE_IRQ),
-};
+static struct mdio_platform_data am35xx_mdio_pdata;
 
-static struct platform_device am35xx_emac_device = {
-	.name		= "davinci_emac",
-	.id		= -1,
-	.num_resources	= ARRAY_SIZE(am35xx_emac_resources),
-	.resource	= am35xx_emac_resources,
-	.dev		= {
-		.platform_data	= &am35xx_emac_pdata,
-	},
-};
+static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
+		void *pdata, int pdata_len)
+{
+	struct platform_device *pdev;
+
+	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
+			NULL, 0, false);
+	if (IS_ERR(pdev)) {
+		WARN(1, "Can't build omap_device for %s:%s.\n",
+					oh->class->name, oh->name);
+		return PTR_ERR(pdev);
+	}
+
+	return 0;
+}
 
 void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 {
-	unsigned int regval;
-	int err;
+	struct omap_hwmod *oh;
+	u32 regval;
+	int ret;
 
-	am35xx_emac_pdata.rmii_en = rmii_en;
-	am35xx_emac_mdio_pdata.bus_freq = mdio_bus_freq;
-	err = platform_device_register(&am35xx_emac_device);
-	if (err) {
-		pr_err("AM35x: failed registering EMAC device: %d\n", err);
+	oh = omap_hwmod_lookup("davinci_mdio");
+	if (!oh) {
+		pr_err("Could not find davinci_mdio hwmod\n");
+		return;
+	}
+
+	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
+
+	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
+					 sizeof(am35xx_mdio_pdata));
+	if (ret) {
+		pr_err("Could not build davinci_mdio hwmod device\n");
 		return;
 	}
 
-	err = platform_device_register(&am35xx_emac_mdio_device);
-	if (err) {
-		pr_err("AM35x: failed registering EMAC MDIO device: %d\n", err);
-		platform_device_unregister(&am35xx_emac_device);
+	oh = omap_hwmod_lookup("davinci_emac");
+	if (!oh) {
+		pr_err("Could not find davinci_emac hwmod\n");
+		return;
+	}
+
+	am35xx_emac_pdata.rmii_en = rmii_en;
+
+	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
+					 sizeof(am35xx_emac_pdata));
+	if (ret) {
+		pr_err("Could not build davinci_emac hwmod device\n");
 		return;
 	}
 
diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 12c64db..f2f422e 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3478,7 +3478,7 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"ipss_ick",	&ipss_ick,	CK_AM35XX),
 	CLK(NULL,	"rmii_ck",	&rmii_ck,	CK_AM35XX),
 	CLK(NULL,	"pclk_ck",	&pclk_ck,	CK_AM35XX),
-	CLK("davinci_emac",	NULL,	&emac_ick,	CK_AM35XX),
+	CLK("davinci_emac.0",	NULL,	&emac_ick,	CK_AM35XX),
 	CLK("davinci_mdio.0",	NULL,	&emac_fck,	CK_AM35XX),
 	CLK("vpfe-capture",	"master",	&vpfe_ick,	CK_AM35XX),
 	CLK("vpfe-capture",	"slave",	&vpfe_fck,	CK_AM35XX),
diff --git a/arch/arm/mach-omap2/include/mach/am35xx.h b/arch/arm/mach-omap2/include/mach/am35xx.h
index f1e13d1..9559449 100644
--- a/arch/arm/mach-omap2/include/mach/am35xx.h
+++ b/arch/arm/mach-omap2/include/mach/am35xx.h
@@ -36,6 +36,8 @@
 #define AM35XX_EMAC_CNTRL_MOD_OFFSET	(0x0)
 #define AM35XX_EMAC_CNTRL_RAM_OFFSET	(0x20000)
 #define AM35XX_EMAC_MDIO_OFFSET		(0x30000)
+#define AM35XX_IPSS_MDIO_BASE		(AM35XX_IPSS_EMAC_BASE + \
+						AM35XX_EMAC_MDIO_OFFSET)
 #define AM35XX_EMAC_CNTRL_RAM_SIZE	(0x2000)
 #define AM35XX_EMAC_RAM_ADDR		(AM3517_EMAC_BASE + \
 						AM3517_EMAC_CNTRL_RAM_OFFSET)
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index c6653a80..87d817d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3075,6 +3075,87 @@ static struct omap_hwmod_ocp_if omap3xxx_l4_core__usb_tll_hs = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* am35xx has Davinci MDIO & EMAC */
+static struct omap_hwmod_class am35xx_mdio_class = {
+	.name = "davinci_mdio",
+};
+
+static struct omap_hwmod am35xx_mdio_hwmod = {
+	.name		= "davinci_mdio",
+	.class		= &am35xx_mdio_class,
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+static struct omap_hwmod_ocp_if am35xx_mdio__l3 = {
+	.master		= &am35xx_mdio_hwmod,
+	.slave		= &omap3xxx_l3_main_hwmod,
+	.clk		= "emac_fck",
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am35xx_mdio_addrs[] = {
+	{
+		.pa_start	= AM35XX_IPSS_MDIO_BASE,
+		.pa_end		= AM35XX_IPSS_MDIO_BASE + SZ_4K - 1,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l4_core -> davinci mdio  */
+static struct omap_hwmod_ocp_if am35xx_l4_core__mdio = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &am35xx_mdio_hwmod,
+	.clk		= "emac_fck",
+	.addr		= am35xx_mdio_addrs,
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_irq_info am35xx_emac_mpu_irqs[] = {
+	{ .name = "rxthresh",	.irq = INT_35XX_EMAC_C0_RXTHRESH_IRQ },
+	{ .name = "rx_pulse",	.irq = INT_35XX_EMAC_C0_RX_PULSE_IRQ },
+	{ .name = "tx_pulse",	.irq = INT_35XX_EMAC_C0_TX_PULSE_IRQ },
+	{ .name = "misc_pulse",	.irq = INT_35XX_EMAC_C0_MISC_PULSE_IRQ },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_class am35xx_emac_class = {
+	.name = "davinci_emac",
+};
+
+static struct omap_hwmod am35xx_emac_hwmod = {
+	.name		= "davinci_emac",
+	.mpu_irqs	= am35xx_emac_mpu_irqs,
+	.class		= &am35xx_emac_class,
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+/* l3_core -> davinci emac interface */
+static struct omap_hwmod_ocp_if am35xx_emac__l3 = {
+	.master		= &am35xx_emac_hwmod,
+	.slave		= &omap3xxx_l3_main_hwmod,
+	.clk		= "emac_ick",
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space am35xx_emac_addrs[] = {
+	{
+		.pa_start	= AM35XX_IPSS_EMAC_BASE,
+		.pa_end		= AM35XX_IPSS_EMAC_BASE + 0x30000 - 1,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l4_core -> davinci emac  */
+static struct omap_hwmod_ocp_if am35xx_l4_core__emac = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &am35xx_emac_hwmod,
+	.clk		= "emac_ick",
+	.addr		= am35xx_emac_addrs,
+	.user		= OCP_USER_MPU,
+};
+
 static struct omap_hwmod_ocp_if *omap3xxx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l3_main__l4_core,
 	&omap3xxx_l3_main__l4_per,
@@ -3200,6 +3281,10 @@ static struct omap_hwmod_ocp_if *am35xx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l4_core__usb_tll_hs,
 	&omap3xxx_l4_core__es3plus_mmc1,
 	&omap3xxx_l4_core__es3plus_mmc2,
+	&am35xx_mdio__l3,
+	&am35xx_l4_core__mdio,
+	&am35xx_emac__l3,
+	&am35xx_l4_core__emac,
 	NULL
 };
 
-- 
1.7.9.4

[PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

From: "Mark A. Greer" <mgreer@animalcreek.com>

The am35x family of SoCs has a Davinci EMAC ethernet
controller on-chip.  Unfortunately, the EMAC is unable
to wake the PRCM when there is network activity which
leads to a hung or extremely slow system when the MPU
has executed a 'wfi' instruction (because of pm_idle
or CPUidle).  To prevent this, add hooks to the EMAC
pm_runtime suspend/resume calls so that hlt is disabled
whenever the EMAC is in use.

Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
---
 arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
 arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
 arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
 arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 3bb5cb3..22ff968 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -23,6 +23,37 @@
 #include "control.h"
 #include "am35xx-emac.h"
 
+/*
+ * Default pm_lats for the am35x.
+ * The net effect of using am35xx_emac_pm_lats[] is that
+ * pm_idle or CPUidle won't be called while the emac
+ * interface is open.  This is required because the
+ * EMAC can't wake up PRCM so if the MPU is executing
+ * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
+ * it won't break out of it due to emac activity.
+ */
+static int am35xx_emac_deactivate_func(struct omap_device *od)
+{
+	enable_hlt();
+	return omap_device_idle_hwmods(od);
+}
+
+static int am35xx_emac_activate_func(struct omap_device *od)
+{
+	disable_hlt();
+	return omap_device_enable_hwmods(od);
+}
+
+struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
+	{
+		.deactivate_func	= am35xx_emac_deactivate_func,
+		.activate_func		= am35xx_emac_activate_func,
+		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	},
+};
+
+int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
+
 static void am35xx_enable_emac_int(void)
 {
 	u32 regval;
@@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
 static struct mdio_platform_data am35xx_mdio_pdata;
 
 static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
-		void *pdata, int pdata_len)
+		void *pdata, int pdata_len,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
 {
 	struct platform_device *pdev;
 
 	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
-			NULL, 0, false);
+			pm_lats, pm_lats_size, false);
 	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 					oh->class->name, oh->name);
@@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
 	return 0;
 }
 
-void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
+void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
 {
 	struct omap_hwmod *oh;
 	u32 regval;
@@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
-					 sizeof(am35xx_mdio_pdata));
+					 sizeof(am35xx_mdio_pdata), NULL, 0);
 	if (ret) {
 		pr_err("Could not build davinci_mdio hwmod device\n");
 		return;
@@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_emac_pdata.rmii_en = rmii_en;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
-					 sizeof(am35xx_emac_pdata));
+					 sizeof(am35xx_emac_pdata),
+					 pm_lats, pm_lats_size);
 	if (ret) {
 		pr_err("Could not build davinci_emac hwmod device\n");
 		return;
diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
index 15c6f9c..7c23808 100644
--- a/arch/arm/mach-omap2/am35xx-emac.h
+++ b/arch/arm/mach-omap2/am35xx-emac.h
@@ -6,10 +6,20 @@
  * published by the Free Software Foundation.
  */
 
+#include <plat/omap_device.h>
+
 #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
 
-#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
-void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
+#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
+extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
+extern int am35xx_emac_pm_lats_size;
+
+void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
 #else
-static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
+#define am35xx_emac_pm_lats		NULL
+#define am35xx_emac_pm_lats_size	0
+
+static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
 #endif
diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
index 3645285..fe3a304 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
 	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
 				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
 	/*Ethernet*/
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 
 	/* MUSB */
 	am3517_evm_musb_init();
diff --git a/arch/arm/mach-omap2/board-cm-t3517.c b/arch/arm/mach-omap2/board-cm-t3517.c
index 9e66e16..0a3dc7a 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -292,7 +292,8 @@ static void __init cm_t3517_init(void)
 	cm_t3517_init_rtc();
 	cm_t3517_init_usbh();
 	cm_t3517_init_hecc();
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 }
 
 MACHINE_START(CM_T3517, "Compulab CM-T3517")
-- 
1.7.9.4

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Igor Grinberg]

Hi Mark,

Thanks for the great work!

On 05/12/12 00:12, Mark A. Greer wrote:
> From: "Mark A. Greer" <mgreer@animalcreek.com>
> 
> The am35x family of SoCs has a Davinci EMAC ethernet
> controller on-chip.  Unfortunately, the EMAC is unable
> to wake the PRCM when there is network activity which
> leads to a hung or extremely slow system when the MPU
> has executed a 'wfi' instruction (because of pm_idle
> or CPUidle).  To prevent this, add hooks to the EMAC
> pm_runtime suspend/resume calls so that hlt is disabled
> whenever the EMAC is in use.
> 
> Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
> ---
>  arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
>  arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
>  arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
>  arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
>  4 files changed, 56 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
> index 3bb5cb3..22ff968 100644
> --- a/arch/arm/mach-omap2/am35xx-emac.c
> +++ b/arch/arm/mach-omap2/am35xx-emac.c
> @@ -23,6 +23,37 @@
>  #include "control.h"
>  #include "am35xx-emac.h"
>  
> +/*
> + * Default pm_lats for the am35x.
> + * The net effect of using am35xx_emac_pm_lats[] is that
> + * pm_idle or CPUidle won't be called while the emac
> + * interface is open.  This is required because the
> + * EMAC can't wake up PRCM so if the MPU is executing
> + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
> + * it won't break out of it due to emac activity.
> + */
> +static int am35xx_emac_deactivate_func(struct omap_device *od)
> +{
> +	enable_hlt();
> +	return omap_device_idle_hwmods(od);
> +}
> +
> +static int am35xx_emac_activate_func(struct omap_device *od)
> +{
> +	disable_hlt();
> +	return omap_device_enable_hwmods(od);
> +}
> +
> +struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
> +	{
> +		.deactivate_func	= am35xx_emac_deactivate_func,
> +		.activate_func		= am35xx_emac_activate_func,
> +		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> +	},
> +};
> +
> +int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
> +
>  static void am35xx_enable_emac_int(void)
>  {
>  	u32 regval;
> @@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
>  static struct mdio_platform_data am35xx_mdio_pdata;
>  
>  static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
> -		void *pdata, int pdata_len)
> +		void *pdata, int pdata_len,
> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>  {
>  	struct platform_device *pdev;
>  
>  	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
> -			NULL, 0, false);
> +			pm_lats, pm_lats_size, false);
>  	if (IS_ERR(pdev)) {
>  		WARN(1, "Can't build omap_device for %s:%s.\n",
>  					oh->class->name, oh->name);
> @@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
>  	return 0;
>  }
>  
> -void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> +void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>  {
>  	struct omap_hwmod *oh;
>  	u32 regval;
> @@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>  	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
>  
>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
> -					 sizeof(am35xx_mdio_pdata));
> +					 sizeof(am35xx_mdio_pdata), NULL, 0);
>  	if (ret) {
>  		pr_err("Could not build davinci_mdio hwmod device\n");
>  		return;
> @@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>  	am35xx_emac_pdata.rmii_en = rmii_en;
>  
>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
> -					 sizeof(am35xx_emac_pdata));
> +					 sizeof(am35xx_emac_pdata),
> +					 pm_lats, pm_lats_size);
>  	if (ret) {
>  		pr_err("Could not build davinci_emac hwmod device\n");
>  		return;
> diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
> index 15c6f9c..7c23808 100644
> --- a/arch/arm/mach-omap2/am35xx-emac.h
> +++ b/arch/arm/mach-omap2/am35xx-emac.h
> @@ -6,10 +6,20 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <plat/omap_device.h>
> +
>  #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
>  
> -#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
> -void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
> +#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
> +extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
> +extern int am35xx_emac_pm_lats_size;
> +
> +void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
>  #else
> -static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
> +#define am35xx_emac_pm_lats		NULL
> +#define am35xx_emac_pm_lats_size	0
> +
> +static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
>  #endif
> diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
> index 3645285..fe3a304 100644
> --- a/arch/arm/mach-omap2/board-am3517evm.c
> +++ b/arch/arm/mach-omap2/board-am3517evm.c
> @@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
>  	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
>  				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
>  	/*Ethernet*/
> -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
> +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
> +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);

Why is it essential for board file to pass these pm structures?
Is it something board specific?
Can't you just use them inside the am35xx-emac.c file?

>  
>  	/* MUSB */
>  	am3517_evm_musb_init();
> diff --git a/arch/arm/mach-omap2/board-cm-t3517.c b/arch/arm/mach-omap2/board-cm-t3517.c
> index 9e66e16..0a3dc7a 100644
> --- a/arch/arm/mach-omap2/board-cm-t3517.c
> +++ b/arch/arm/mach-omap2/board-cm-t3517.c
> @@ -292,7 +292,8 @@ static void __init cm_t3517_init(void)
>  	cm_t3517_init_rtc();
>  	cm_t3517_init_usbh();
>  	cm_t3517_init_hecc();
> -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
> +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
> +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
>  }
>  
>  MACHINE_START(CM_T3517, "Compulab CM-T3517")

-- 
Regards,
Igor.

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Mon, May 14, 2012 at 11:20:58AM +0300, Igor Grinberg wrote:
> Hi Mark,

Hi Igor.

> Thanks for the great work!
> 
> On 05/12/12 00:12, Mark A. Greer wrote:
> > From: "Mark A. Greer" <mgreer@animalcreek.com>
> > 
> > The am35x family of SoCs has a Davinci EMAC ethernet
> > controller on-chip.  Unfortunately, the EMAC is unable
> > to wake the PRCM when there is network activity which
> > leads to a hung or extremely slow system when the MPU
> > has executed a 'wfi' instruction (because of pm_idle
> > or CPUidle).  To prevent this, add hooks to the EMAC
> > pm_runtime suspend/resume calls so that hlt is disabled
> > whenever the EMAC is in use.
> > 
> > Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
> > ---
> >  arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
> >  arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
> >  arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
> >  arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
> >  4 files changed, 56 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
> > index 3bb5cb3..22ff968 100644
> > --- a/arch/arm/mach-omap2/am35xx-emac.c
> > +++ b/arch/arm/mach-omap2/am35xx-emac.c
> > @@ -23,6 +23,37 @@
> >  #include "control.h"
> >  #include "am35xx-emac.h"
> >  
> > +/*
> > + * Default pm_lats for the am35x.
> > + * The net effect of using am35xx_emac_pm_lats[] is that
> > + * pm_idle or CPUidle won't be called while the emac
> > + * interface is open.  This is required because the
> > + * EMAC can't wake up PRCM so if the MPU is executing
> > + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
> > + * it won't break out of it due to emac activity.
> > + */
> > +static int am35xx_emac_deactivate_func(struct omap_device *od)
> > +{
> > +	enable_hlt();
> > +	return omap_device_idle_hwmods(od);
> > +}
> > +
> > +static int am35xx_emac_activate_func(struct omap_device *od)
> > +{
> > +	disable_hlt();
> > +	return omap_device_enable_hwmods(od);
> > +}
> > +
> > +struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
> > +	{
> > +		.deactivate_func	= am35xx_emac_deactivate_func,
> > +		.activate_func		= am35xx_emac_activate_func,
> > +		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> > +	},
> > +};
> > +
> > +int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
> > +
> >  static void am35xx_enable_emac_int(void)
> >  {
> >  	u32 regval;
> > @@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
> >  static struct mdio_platform_data am35xx_mdio_pdata;
> >  
> >  static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
> > -		void *pdata, int pdata_len)
> > +		void *pdata, int pdata_len,
> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
> >  {
> >  	struct platform_device *pdev;
> >  
> >  	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
> > -			NULL, 0, false);
> > +			pm_lats, pm_lats_size, false);
> >  	if (IS_ERR(pdev)) {
> >  		WARN(1, "Can't build omap_device for %s:%s.\n",
> >  					oh->class->name, oh->name);
> > @@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
> >  	return 0;
> >  }
> >  
> > -void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> > +void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
> >  {
> >  	struct omap_hwmod *oh;
> >  	u32 regval;
> > @@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> >  	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
> >  
> >  	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
> > -					 sizeof(am35xx_mdio_pdata));
> > +					 sizeof(am35xx_mdio_pdata), NULL, 0);
> >  	if (ret) {
> >  		pr_err("Could not build davinci_mdio hwmod device\n");
> >  		return;
> > @@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> >  	am35xx_emac_pdata.rmii_en = rmii_en;
> >  
> >  	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
> > -					 sizeof(am35xx_emac_pdata));
> > +					 sizeof(am35xx_emac_pdata),
> > +					 pm_lats, pm_lats_size);
> >  	if (ret) {
> >  		pr_err("Could not build davinci_emac hwmod device\n");
> >  		return;
> > diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
> > index 15c6f9c..7c23808 100644
> > --- a/arch/arm/mach-omap2/am35xx-emac.h
> > +++ b/arch/arm/mach-omap2/am35xx-emac.h
> > @@ -6,10 +6,20 @@
> >   * published by the Free Software Foundation.
> >   */
> >  
> > +#include <plat/omap_device.h>
> > +
> >  #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
> >  
> > -#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
> > -void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
> > +#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
> > +extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
> > +extern int am35xx_emac_pm_lats_size;
> > +
> > +void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
> >  #else
> > -static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
> > +#define am35xx_emac_pm_lats		NULL
> > +#define am35xx_emac_pm_lats_size	0
> > +
> > +static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
> >  #endif
> > diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
> > index 3645285..fe3a304 100644
> > --- a/arch/arm/mach-omap2/board-am3517evm.c
> > +++ b/arch/arm/mach-omap2/board-am3517evm.c
> > @@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
> >  	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
> >  				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
> >  	/*Ethernet*/
> > -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
> > +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
> > +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
> 
> Why is it essential for board file to pass these pm structures?
> Is it something board specific?
> Can't you just use them inside the am35xx-emac.c file?

Great question.  I went back & forth on this mayself (I have 3 different
versions sitting in my repo :).  I ended up passing the pm structures in
case there is a variant that comes along that doesn't need this fixup (or
a board that wants to do this plus additional things).

The variants that I have are:

1) Leave am35xx_emac_init() interface the way it is and automatically
add the necessary things.  The problem is lack of flexibility if & when
some other variant comes along.

2) Add pm_lats & pm_lats_size to am35xx_emac_init() but if they're NULL
or 0, then use the defaults that are the same as what's in this patch.
More flexible but doesn't easily allow a board file to NULL out the pm
structure (have to make up and pass NULL version).

3) Pass the pm struct via am35xx_emac_init() and just make the default
available for use.  This is what I submitted since it was the most flexible.

Which one do you prefer or do you have something else in mind?

Thanks,

Mark

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Kevin Hilman]

"Mark A. Greer" <mgreer@animalcreek.com> writes:

> On Mon, May 14, 2012 at 11:20:58AM +0300, Igor Grinberg wrote:
>> Hi Mark,
>
> Hi Igor.
>
>> Thanks for the great work!
>> 
>> On 05/12/12 00:12, Mark A. Greer wrote:
>> > From: "Mark A. Greer" <mgreer@animalcreek.com>
>> > 
>> > The am35x family of SoCs has a Davinci EMAC ethernet
>> > controller on-chip.  Unfortunately, the EMAC is unable
>> > to wake the PRCM when there is network activity which
>> > leads to a hung or extremely slow system when the MPU
>> > has executed a 'wfi' instruction (because of pm_idle
>> > or CPUidle).  To prevent this, add hooks to the EMAC
>> > pm_runtime suspend/resume calls so that hlt is disabled
>> > whenever the EMAC is in use.
>> > 
>> > Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
>> > ---
>> >  arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
>> >  arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
>> >  arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
>> >  arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
>> >  4 files changed, 56 insertions(+), 10 deletions(-)
>> > 
>> > diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
>> > index 3bb5cb3..22ff968 100644
>> > --- a/arch/arm/mach-omap2/am35xx-emac.c
>> > +++ b/arch/arm/mach-omap2/am35xx-emac.c
>> > @@ -23,6 +23,37 @@
>> >  #include "control.h"
>> >  #include "am35xx-emac.h"
>> >  
>> > +/*
>> > + * Default pm_lats for the am35x.
>> > + * The net effect of using am35xx_emac_pm_lats[] is that
>> > + * pm_idle or CPUidle won't be called while the emac
>> > + * interface is open.  This is required because the
>> > + * EMAC can't wake up PRCM so if the MPU is executing
>> > + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
>> > + * it won't break out of it due to emac activity.
>> > + */
>> > +static int am35xx_emac_deactivate_func(struct omap_device *od)
>> > +{
>> > +	enable_hlt();
>> > +	return omap_device_idle_hwmods(od);
>> > +}
>> > +
>> > +static int am35xx_emac_activate_func(struct omap_device *od)
>> > +{
>> > +	disable_hlt();
>> > +	return omap_device_enable_hwmods(od);
>> > +}
>> > +
>> > +struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
>> > +	{
>> > +		.deactivate_func	= am35xx_emac_deactivate_func,
>> > +		.activate_func		= am35xx_emac_activate_func,
>> > +		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>> > +	},
>> > +};
>> > +
>> > +int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
>> > +
>> >  static void am35xx_enable_emac_int(void)
>> >  {
>> >  	u32 regval;
>> > @@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
>> >  static struct mdio_platform_data am35xx_mdio_pdata;
>> >  
>> >  static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
>> > -		void *pdata, int pdata_len)
>> > +		void *pdata, int pdata_len,
>> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>> >  {
>> >  	struct platform_device *pdev;
>> >  
>> >  	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
>> > -			NULL, 0, false);
>> > +			pm_lats, pm_lats_size, false);
>> >  	if (IS_ERR(pdev)) {
>> >  		WARN(1, "Can't build omap_device for %s:%s.\n",
>> >  					oh->class->name, oh->name);
>> > @@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
>> >  	return 0;
>> >  }
>> >  
>> > -void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>> > +void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>> >  {
>> >  	struct omap_hwmod *oh;
>> >  	u32 regval;
>> > @@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>> >  	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
>> >  
>> >  	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
>> > -					 sizeof(am35xx_mdio_pdata));
>> > +					 sizeof(am35xx_mdio_pdata), NULL, 0);
>> >  	if (ret) {
>> >  		pr_err("Could not build davinci_mdio hwmod device\n");
>> >  		return;
>> > @@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>> >  	am35xx_emac_pdata.rmii_en = rmii_en;
>> >  
>> >  	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
>> > -					 sizeof(am35xx_emac_pdata));
>> > +					 sizeof(am35xx_emac_pdata),
>> > +					 pm_lats, pm_lats_size);
>> >  	if (ret) {
>> >  		pr_err("Could not build davinci_emac hwmod device\n");
>> >  		return;
>> > diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
>> > index 15c6f9c..7c23808 100644
>> > --- a/arch/arm/mach-omap2/am35xx-emac.h
>> > +++ b/arch/arm/mach-omap2/am35xx-emac.h
>> > @@ -6,10 +6,20 @@
>> >   * published by the Free Software Foundation.
>> >   */
>> >  
>> > +#include <plat/omap_device.h>
>> > +
>> >  #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
>> >  
>> > -#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
>> > -void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
>> > +#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
>> > +extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
>> > +extern int am35xx_emac_pm_lats_size;
>> > +
>> > +void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
>> >  #else
>> > -static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
>> > +#define am35xx_emac_pm_lats		NULL
>> > +#define am35xx_emac_pm_lats_size	0
>> > +
>> > +static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>> > +		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
>> >  #endif
>> > diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
>> > index 3645285..fe3a304 100644
>> > --- a/arch/arm/mach-omap2/board-am3517evm.c
>> > +++ b/arch/arm/mach-omap2/board-am3517evm.c
>> > @@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
>> >  	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
>> >  				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
>> >  	/*Ethernet*/
>> > -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
>> > +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
>> > +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
>> 
>> Why is it essential for board file to pass these pm structures?
>> Is it something board specific?
>> Can't you just use them inside the am35xx-emac.c file?
>
> Great question.  I went back & forth on this mayself (I have 3 different
> versions sitting in my repo :).  I ended up passing the pm structures in
> case there is a variant that comes along that doesn't need this fixup (or
> a board that wants to do this plus additional things).
>
> The variants that I have are:
>
> 1) Leave am35xx_emac_init() interface the way it is and automatically
> add the necessary things.  The problem is lack of flexibility if & when
> some other variant comes along.
>
> 2) Add pm_lats & pm_lats_size to am35xx_emac_init() but if they're NULL
> or 0, then use the defaults that are the same as what's in this patch.
> More flexible but doesn't easily allow a board file to NULL out the pm
> structure (have to make up and pass NULL version).
>
> 3) Pass the pm struct via am35xx_emac_init() and just make the default
> available for use.  This is what I submitted since it was the most flexible.
>
> Which one do you prefer or do you have something else in mind?
>

For now, I suggest you stick with (1) until we have a reason to make it
more flexible.  AFAIK, all usages of this IP in OMAP-derivative parts
have this same limitation.

Kevin

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Igor Grinberg]

On 05/15/12 00:32, Kevin Hilman wrote:
> "Mark A. Greer" <mgreer@animalcreek.com> writes:
> 
>> On Mon, May 14, 2012 at 11:20:58AM +0300, Igor Grinberg wrote:
>>> Hi Mark,
>>
>> Hi Igor.
>>
>>> Thanks for the great work!
>>>
>>> On 05/12/12 00:12, Mark A. Greer wrote:
>>>> From: "Mark A. Greer" <mgreer@animalcreek.com>
>>>>
>>>> The am35x family of SoCs has a Davinci EMAC ethernet
>>>> controller on-chip.  Unfortunately, the EMAC is unable
>>>> to wake the PRCM when there is network activity which
>>>> leads to a hung or extremely slow system when the MPU
>>>> has executed a 'wfi' instruction (because of pm_idle
>>>> or CPUidle).  To prevent this, add hooks to the EMAC
>>>> pm_runtime suspend/resume calls so that hlt is disabled
>>>> whenever the EMAC is in use.
>>>>
>>>> Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
>>>> ---
>>>>  arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
>>>>  arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
>>>>  arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
>>>>  arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
>>>>  4 files changed, 56 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
>>>> index 3bb5cb3..22ff968 100644
>>>> --- a/arch/arm/mach-omap2/am35xx-emac.c
>>>> +++ b/arch/arm/mach-omap2/am35xx-emac.c
>>>> @@ -23,6 +23,37 @@
>>>>  #include "control.h"
>>>>  #include "am35xx-emac.h"
>>>>  
>>>> +/*
>>>> + * Default pm_lats for the am35x.
>>>> + * The net effect of using am35xx_emac_pm_lats[] is that
>>>> + * pm_idle or CPUidle won't be called while the emac
>>>> + * interface is open.  This is required because the
>>>> + * EMAC can't wake up PRCM so if the MPU is executing
>>>> + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
>>>> + * it won't break out of it due to emac activity.
>>>> + */
>>>> +static int am35xx_emac_deactivate_func(struct omap_device *od)
>>>> +{
>>>> +	enable_hlt();
>>>> +	return omap_device_idle_hwmods(od);
>>>> +}
>>>> +
>>>> +static int am35xx_emac_activate_func(struct omap_device *od)
>>>> +{
>>>> +	disable_hlt();
>>>> +	return omap_device_enable_hwmods(od);
>>>> +}
>>>> +
>>>> +struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
>>>> +	{
>>>> +		.deactivate_func	= am35xx_emac_deactivate_func,
>>>> +		.activate_func		= am35xx_emac_activate_func,
>>>> +		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>>>> +	},
>>>> +};
>>>> +
>>>> +int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
>>>> +
>>>>  static void am35xx_enable_emac_int(void)
>>>>  {
>>>>  	u32 regval;
>>>> @@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
>>>>  static struct mdio_platform_data am35xx_mdio_pdata;
>>>>  
>>>>  static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
>>>> -		void *pdata, int pdata_len)
>>>> +		void *pdata, int pdata_len,
>>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>>>>  {
>>>>  	struct platform_device *pdev;
>>>>  
>>>>  	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
>>>> -			NULL, 0, false);
>>>> +			pm_lats, pm_lats_size, false);
>>>>  	if (IS_ERR(pdev)) {
>>>>  		WARN(1, "Can't build omap_device for %s:%s.\n",
>>>>  					oh->class->name, oh->name);
>>>> @@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>>>> +void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
>>>>  {
>>>>  	struct omap_hwmod *oh;
>>>>  	u32 regval;
>>>> @@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>>>>  	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
>>>>  
>>>>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
>>>> -					 sizeof(am35xx_mdio_pdata));
>>>> +					 sizeof(am35xx_mdio_pdata), NULL, 0);
>>>>  	if (ret) {
>>>>  		pr_err("Could not build davinci_mdio hwmod device\n");
>>>>  		return;
>>>> @@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
>>>>  	am35xx_emac_pdata.rmii_en = rmii_en;
>>>>  
>>>>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
>>>> -					 sizeof(am35xx_emac_pdata));
>>>> +					 sizeof(am35xx_emac_pdata),
>>>> +					 pm_lats, pm_lats_size);
>>>>  	if (ret) {
>>>>  		pr_err("Could not build davinci_emac hwmod device\n");
>>>>  		return;
>>>> diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
>>>> index 15c6f9c..7c23808 100644
>>>> --- a/arch/arm/mach-omap2/am35xx-emac.h
>>>> +++ b/arch/arm/mach-omap2/am35xx-emac.h
>>>> @@ -6,10 +6,20 @@
>>>>   * published by the Free Software Foundation.
>>>>   */
>>>>  
>>>> +#include <plat/omap_device.h>
>>>> +
>>>>  #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
>>>>  
>>>> -#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
>>>> -void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
>>>> +#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
>>>> +extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
>>>> +extern int am35xx_emac_pm_lats_size;
>>>> +
>>>> +void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
>>>>  #else
>>>> -static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
>>>> +#define am35xx_emac_pm_lats		NULL
>>>> +#define am35xx_emac_pm_lats_size	0
>>>> +
>>>> +static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
>>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
>>>>  #endif
>>>> diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
>>>> index 3645285..fe3a304 100644
>>>> --- a/arch/arm/mach-omap2/board-am3517evm.c
>>>> +++ b/arch/arm/mach-omap2/board-am3517evm.c
>>>> @@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
>>>>  	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
>>>>  				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
>>>>  	/*Ethernet*/
>>>> -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
>>>> +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
>>>> +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
>>>
>>> Why is it essential for board file to pass these pm structures?
>>> Is it something board specific?
>>> Can't you just use them inside the am35xx-emac.c file?
>>
>> Great question.  I went back & forth on this mayself (I have 3 different
>> versions sitting in my repo :).  I ended up passing the pm structures in
>> case there is a variant that comes along that doesn't need this fixup (or
>> a board that wants to do this plus additional things).
>>
>> The variants that I have are:
>>
>> 1) Leave am35xx_emac_init() interface the way it is and automatically
>> add the necessary things.  The problem is lack of flexibility if & when
>> some other variant comes along.
>>
>> 2) Add pm_lats & pm_lats_size to am35xx_emac_init() but if they're NULL
>> or 0, then use the defaults that are the same as what's in this patch.
>> More flexible but doesn't easily allow a board file to NULL out the pm
>> structure (have to make up and pass NULL version).
>>
>> 3) Pass the pm struct via am35xx_emac_init() and just make the default
>> available for use.  This is what I submitted since it was the most flexible.
>>
>> Which one do you prefer or do you have something else in mind?
>>
> 
> For now, I suggest you stick with (1) until we have a reason to make it
> more flexible.  AFAIK, all usages of this IP in OMAP-derivative parts
> have this same limitation.

I agree with Kevin - (1) should be used.

There is no need to complicate things in favor of something that might
or might not exist some day, because even if it will exist some day,
we probably anyway will see a better solution to this and will change
the existing one.


-- 
Regards,
Igor.

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Tue, May 15, 2012 at 03:42:09PM +0300, Igor Grinberg wrote:
> On 05/15/12 00:32, Kevin Hilman wrote:
> > "Mark A. Greer" <mgreer@animalcreek.com> writes:
> > 
> >> On Mon, May 14, 2012 at 11:20:58AM +0300, Igor Grinberg wrote:
> >>> Hi Mark,
> >>
> >> Hi Igor.
> >>
> >>> Thanks for the great work!
> >>>
> >>> On 05/12/12 00:12, Mark A. Greer wrote:
> >>>> From: "Mark A. Greer" <mgreer@animalcreek.com>
> >>>>
> >>>> The am35x family of SoCs has a Davinci EMAC ethernet
> >>>> controller on-chip.  Unfortunately, the EMAC is unable
> >>>> to wake the PRCM when there is network activity which
> >>>> leads to a hung or extremely slow system when the MPU
> >>>> has executed a 'wfi' instruction (because of pm_idle
> >>>> or CPUidle).  To prevent this, add hooks to the EMAC
> >>>> pm_runtime suspend/resume calls so that hlt is disabled
> >>>> whenever the EMAC is in use.
> >>>>
> >>>> Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
> >>>> ---
> >>>>  arch/arm/mach-omap2/am35xx-emac.c     |   44 +++++++++++++++++++++++++++++----
> >>>>  arch/arm/mach-omap2/am35xx-emac.h     |   16 +++++++++---
> >>>>  arch/arm/mach-omap2/board-am3517evm.c |    3 ++-
> >>>>  arch/arm/mach-omap2/board-cm-t3517.c  |    3 ++-
> >>>>  4 files changed, 56 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
> >>>> index 3bb5cb3..22ff968 100644
> >>>> --- a/arch/arm/mach-omap2/am35xx-emac.c
> >>>> +++ b/arch/arm/mach-omap2/am35xx-emac.c
> >>>> @@ -23,6 +23,37 @@
> >>>>  #include "control.h"
> >>>>  #include "am35xx-emac.h"
> >>>>  
> >>>> +/*
> >>>> + * Default pm_lats for the am35x.
> >>>> + * The net effect of using am35xx_emac_pm_lats[] is that
> >>>> + * pm_idle or CPUidle won't be called while the emac
> >>>> + * interface is open.  This is required because the
> >>>> + * EMAC can't wake up PRCM so if the MPU is executing
> >>>> + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
> >>>> + * it won't break out of it due to emac activity.
> >>>> + */
> >>>> +static int am35xx_emac_deactivate_func(struct omap_device *od)
> >>>> +{
> >>>> +	enable_hlt();
> >>>> +	return omap_device_idle_hwmods(od);
> >>>> +}
> >>>> +
> >>>> +static int am35xx_emac_activate_func(struct omap_device *od)
> >>>> +{
> >>>> +	disable_hlt();
> >>>> +	return omap_device_enable_hwmods(od);
> >>>> +}
> >>>> +
> >>>> +struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
> >>>> +	{
> >>>> +		.deactivate_func	= am35xx_emac_deactivate_func,
> >>>> +		.activate_func		= am35xx_emac_activate_func,
> >>>> +		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> >>>> +	},
> >>>> +};
> >>>> +
> >>>> +int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
> >>>> +
> >>>>  static void am35xx_enable_emac_int(void)
> >>>>  {
> >>>>  	u32 regval;
> >>>> @@ -61,12 +92,13 @@ static struct emac_platform_data am35xx_emac_pdata = {
> >>>>  static struct mdio_platform_data am35xx_mdio_pdata;
> >>>>  
> >>>>  static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
> >>>> -		void *pdata, int pdata_len)
> >>>> +		void *pdata, int pdata_len,
> >>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
> >>>>  {
> >>>>  	struct platform_device *pdev;
> >>>>  
> >>>>  	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
> >>>> -			NULL, 0, false);
> >>>> +			pm_lats, pm_lats_size, false);
> >>>>  	if (IS_ERR(pdev)) {
> >>>>  		WARN(1, "Can't build omap_device for %s:%s.\n",
> >>>>  					oh->class->name, oh->name);
> >>>> @@ -76,7 +108,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> -void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> >>>> +void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> >>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
> >>>>  {
> >>>>  	struct omap_hwmod *oh;
> >>>>  	u32 regval;
> >>>> @@ -91,7 +124,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> >>>>  	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
> >>>>  
> >>>>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
> >>>> -					 sizeof(am35xx_mdio_pdata));
> >>>> +					 sizeof(am35xx_mdio_pdata), NULL, 0);
> >>>>  	if (ret) {
> >>>>  		pr_err("Could not build davinci_mdio hwmod device\n");
> >>>>  		return;
> >>>> @@ -106,7 +139,8 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
> >>>>  	am35xx_emac_pdata.rmii_en = rmii_en;
> >>>>  
> >>>>  	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
> >>>> -					 sizeof(am35xx_emac_pdata));
> >>>> +					 sizeof(am35xx_emac_pdata),
> >>>> +					 pm_lats, pm_lats_size);
> >>>>  	if (ret) {
> >>>>  		pr_err("Could not build davinci_emac hwmod device\n");
> >>>>  		return;
> >>>> diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
> >>>> index 15c6f9c..7c23808 100644
> >>>> --- a/arch/arm/mach-omap2/am35xx-emac.h
> >>>> +++ b/arch/arm/mach-omap2/am35xx-emac.h
> >>>> @@ -6,10 +6,20 @@
> >>>>   * published by the Free Software Foundation.
> >>>>   */
> >>>>  
> >>>> +#include <plat/omap_device.h>
> >>>> +
> >>>>  #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
> >>>>  
> >>>> -#if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
> >>>> -void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
> >>>> +#if IS_ENABLED(CONFIG_TI_DAVINCI_EMAC)
> >>>> +extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
> >>>> +extern int am35xx_emac_pm_lats_size;
> >>>> +
> >>>> +void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> >>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
> >>>>  #else
> >>>> -static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
> >>>> +#define am35xx_emac_pm_lats		NULL
> >>>> +#define am35xx_emac_pm_lats_size	0
> >>>> +
> >>>> +static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
> >>>> +		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
> >>>>  #endif
> >>>> diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
> >>>> index 3645285..fe3a304 100644
> >>>> --- a/arch/arm/mach-omap2/board-am3517evm.c
> >>>> +++ b/arch/arm/mach-omap2/board-am3517evm.c
> >>>> @@ -385,7 +385,8 @@ static void __init am3517_evm_init(void)
> >>>>  	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
> >>>>  				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
> >>>>  	/*Ethernet*/
> >>>> -	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
> >>>> +	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
> >>>> +			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
> >>>
> >>> Why is it essential for board file to pass these pm structures?
> >>> Is it something board specific?
> >>> Can't you just use them inside the am35xx-emac.c file?
> >>
> >> Great question.  I went back & forth on this mayself (I have 3 different
> >> versions sitting in my repo :).  I ended up passing the pm structures in
> >> case there is a variant that comes along that doesn't need this fixup (or
> >> a board that wants to do this plus additional things).
> >>
> >> The variants that I have are:
> >>
> >> 1) Leave am35xx_emac_init() interface the way it is and automatically
> >> add the necessary things.  The problem is lack of flexibility if & when
> >> some other variant comes along.
> >>
> >> 2) Add pm_lats & pm_lats_size to am35xx_emac_init() but if they're NULL
> >> or 0, then use the defaults that are the same as what's in this patch.
> >> More flexible but doesn't easily allow a board file to NULL out the pm
> >> structure (have to make up and pass NULL version).
> >>
> >> 3) Pass the pm struct via am35xx_emac_init() and just make the default
> >> available for use.  This is what I submitted since it was the most flexible.
> >>
> >> Which one do you prefer or do you have something else in mind?
> >>
> > 
> > For now, I suggest you stick with (1) until we have a reason to make it
> > more flexible.  AFAIK, all usages of this IP in OMAP-derivative parts
> > have this same limitation.
> 
> I agree with Kevin - (1) should be used.
> 
> There is no need to complicate things in favor of something that might
> or might not exist some day, because even if it will exist some day,
> we probably anyway will see a better solution to this and will change
> the existing one.

Okay, I will submit v2 in a bit.

Mark

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Paul Walmsley]

Hi

just a quick comment on this one.

On Fri, 11 May 2012, Mark A. Greer wrote:

> From: "Mark A. Greer" <mgreer@animalcreek.com>
> 
> The am35x family of SoCs has a Davinci EMAC ethernet
> controller on-chip.  Unfortunately, the EMAC is unable
> to wake the PRCM when there is network activity which
> leads to a hung or extremely slow system when the MPU
> has executed a 'wfi' instruction (because of pm_idle
> or CPUidle).  To prevent this, add hooks to the EMAC
> pm_runtime suspend/resume calls so that hlt is disabled
> whenever the EMAC is in use.
> 
> Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>

...

> --- a/arch/arm/mach-omap2/am35xx-emac.c
> +++ b/arch/arm/mach-omap2/am35xx-emac.c
> @@ -23,6 +23,37 @@
>  #include "control.h"
>  #include "am35xx-emac.h"
>  
> +/*
> + * Default pm_lats for the am35x.
> + * The net effect of using am35xx_emac_pm_lats[] is that
> + * pm_idle or CPUidle won't be called while the emac
> + * interface is open.  This is required because the
> + * EMAC can't wake up PRCM so if the MPU is executing
> + * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
> + * it won't break out of it due to emac activity.
> + */
> +static int am35xx_emac_deactivate_func(struct omap_device *od)
> +{
> +	enable_hlt();
> +	return omap_device_idle_hwmods(od);
> +}
> +
> +static int am35xx_emac_activate_func(struct omap_device *od)
> +{
> +	disable_hlt();
> +	return omap_device_enable_hwmods(od);
> +}

>From the patch description, it doesn't sound like it's WFI entry that's 
the problem.  The EMAC can assert its interrupt lines to the INTC, since 
the EMAC is active.  If the MPU and CORE powerdomains are ON, then the ARM 
core should wake up out of WFI.  (Unless there's some weird bug; always 
possible.)

Probably the MPU DPLL has to stay running for it all to work, since I 
think that is activated and deactivated by the PRCM.  Maybe the CORE DPLL 
has to stay running too (but I doubt it).  But I'll bet that all the 
clocks downstream of the DPLLs can be gated.  If it works, that would save 
a lot of energy over the disable_hlt() approach.  With disable_hlt(), the 
ARM & interconnect is just going to be burning power waiting for the 
interrupt to come in.

Want to try something like this?  It's your patch but modified to not use 
disable/enable_hlt().  If it doesn't work in your test case, maybe 
try uncommenting that second set of deny_idle / allow_idle ...


- Paul

---
 arch/arm/mach-omap2/am35xx-emac.c |   54 +++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 2c90ac6..d09ccd2 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -23,6 +23,41 @@
 #include "control.h"
 #include "am35xx-emac.h"
 
+static struct clk *mpu_dpll_ck, *core_dpll_ck;
+
+/*
+ * Default pm_lats for the am35x.
+ * The net effect of using am35xx_emac_pm_lats[] is that
+ * pm_idle or CPUidle won't be called while the emac
+ * interface is open.  This is required because the
+ * EMAC can't wake up PRCM so if the MPU is executing
+ * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
+ * it won't break out of it due to emac activity.
+ */
+static int am35xx_emac_deactivate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->deny_idle(mpu_dpll_ck);
+	/* core_dpll_ck->ops->deny_idle(core_dpll_ck); */
+	return omap_device_idle_hwmods(od);
+}
+
+static int am35xx_emac_activate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->allow_idle(mpu_dpll_ck);
+	/* core_dpll_ck->ops->allow_idle(core_dpll_ck); */
+	return omap_device_enable_hwmods(od);
+}
+
+struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
+	{
+		.deactivate_func	= am35xx_emac_deactivate_func,
+		.activate_func		= am35xx_emac_activate_func,
+		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	},
+};
+
+int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
+
 static void am35xx_enable_emac_int(void)
 {
 	u32 v;
@@ -58,12 +93,14 @@ static struct emac_platform_data am35xx_emac_pdata = {
 static struct mdio_platform_data am35xx_mdio_pdata;
 
 static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
-		void *pdata, int pdata_len)
+					     void *pdata, int pdata_len,
+					     struct omap_device_pm_latency *pm_lats,
+					     int pm_lats_size)
 {
 	struct platform_device *pdev;
 
 	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
-				 NULL, 0, false);
+				 pm_lats, pm_lats_size, false);
 	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 		     oh->class->name, oh->name);
@@ -73,7 +110,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
 	return 0;
 }
 
-void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
+void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
 {
 	struct omap_hwmod *oh;
 	u32 v;
@@ -88,7 +126,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
-					 sizeof(am35xx_mdio_pdata));
+					 sizeof(am35xx_mdio_pdata), NULL, 0);
 	if (ret) {
 		pr_err("Could not build davinci_mdio hwmod device\n");
 		return;
@@ -103,12 +141,18 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_emac_pdata.rmii_en = rmii_en;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
-					 sizeof(am35xx_emac_pdata));
+					 sizeof(am35xx_emac_pdata),
+					 pm_lats, pm_lats_size);
 	if (ret) {
 		pr_err("Could not build davinci_emac hwmod device\n");
 		return;
 	}
 
+	mpu_dpll_ck = clk_get(NULL, "dpll1_ck");
+	WARN(!mpu_dpll_ck);
+	core_dpll_ck = clk_get(NULL, "dpll3_ck");
+	WARN(!core_dpll_ck);
+
 	v = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
 	v &= ~AM35XX_CPGMACSS_SW_RST;
 	omap_ctrl_writel(v, AM35XX_CONTROL_IP_SW_RESET);
-- 
1.7.10.4

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Tue, Jul 17, 2012 at 09:54:53PM -0600, Paul Walmsley wrote:
> Hi

Hi Paul.

> From the patch description, it doesn't sound like it's WFI entry that's 
> the problem.  The EMAC can assert its interrupt lines to the INTC, since 
> the EMAC is active.  If the MPU and CORE powerdomains are ON, then the ARM 
> core should wake up out of WFI.  (Unless there's some weird bug; always 
> possible.)
> 
> Probably the MPU DPLL has to stay running for it all to work, since I 
> think that is activated and deactivated by the PRCM.  Maybe the CORE DPLL 
> has to stay running too (but I doubt it).  But I'll bet that all the 
> clocks downstream of the DPLLs can be gated.  If it works, that would save 
> a lot of energy over the disable_hlt() approach.  With disable_hlt(), the 
> ARM & interconnect is just going to be burning power waiting for the 
> interrupt to come in.

Makes sense.

> Want to try something like this?  It's your patch but modified to not use 
> disable/enable_hlt().  If it doesn't work in your test case, maybe 
> try uncommenting that second set of deny_idle / allow_idle ...

I tested the modified patch (to get it to compile) below.
It did not work with or without the core_dpll_ck deny_idle/allow_idle
commented out.

Mark
---
 arch/arm/mach-omap2/am35xx-emac.c     |   56 ++++++++++++++++++++++++++++++---
 arch/arm/mach-omap2/am35xx-emac.h     |   13 ++++++-
 arch/arm/mach-omap2/board-am3517evm.c |    3 +-
 arch/arm/mach-omap2/board-cm-t3517.c  |    3 +-
 4 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 2c90ac6..ed695e8 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -16,13 +16,50 @@
  */
 
 #include <linux/err.h>
+#include <linux/clk.h>
 #include <linux/davinci_emac.h>
 #include <asm/system.h>
+#include <plat/clock.h>
 #include <plat/omap_device.h>
 #include <mach/am35xx.h>
 #include "control.h"
 #include "am35xx-emac.h"
 
+static struct clk *mpu_dpll_ck, *core_dpll_ck;
+
+/*
+ * Default pm_lats for the am35x.
+ * The net effect of using am35xx_emac_pm_lats[] is that
+ * pm_idle or CPUidle won't be called while the emac
+ * interface is open.  This is required because the
+ * EMAC can't wake up PRCM so if the MPU is executing
+ * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
+ * it won't break out of it due to emac activity.
+ */
+static int am35xx_emac_deactivate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->deny_idle(mpu_dpll_ck);
+	/* core_dpll_ck->ops->deny_idle(core_dpll_ck); */
+	return omap_device_idle_hwmods(od);
+}
+
+static int am35xx_emac_activate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->allow_idle(mpu_dpll_ck);
+	/* core_dpll_ck->ops->allow_idle(core_dpll_ck); */
+	return omap_device_enable_hwmods(od);
+}
+
+struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
+	{
+		.deactivate_func	= am35xx_emac_deactivate_func,
+		.activate_func		= am35xx_emac_activate_func,
+		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	},
+};
+
+int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
+
 static void am35xx_enable_emac_int(void)
 {
 	u32 v;
@@ -58,12 +95,14 @@ static struct emac_platform_data am35xx_emac_pdata = {
 static struct mdio_platform_data am35xx_mdio_pdata;
 
 static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
-		void *pdata, int pdata_len)
+					     void *pdata, int pdata_len,
+					     struct omap_device_pm_latency *pm_lats,
+					     int pm_lats_size)
 {
 	struct platform_device *pdev;
 
 	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
-				 NULL, 0, false);
+				 pm_lats, pm_lats_size, false);
 	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 		     oh->class->name, oh->name);
@@ -73,7 +112,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
 	return 0;
 }
 
-void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
+void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
 {
 	struct omap_hwmod *oh;
 	u32 v;
@@ -88,7 +128,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
-					 sizeof(am35xx_mdio_pdata));
+					 sizeof(am35xx_mdio_pdata), NULL, 0);
 	if (ret) {
 		pr_err("Could not build davinci_mdio hwmod device\n");
 		return;
@@ -103,12 +143,18 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_emac_pdata.rmii_en = rmii_en;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
-					 sizeof(am35xx_emac_pdata));
+					 sizeof(am35xx_emac_pdata),
+					 pm_lats, pm_lats_size);
 	if (ret) {
 		pr_err("Could not build davinci_emac hwmod device\n");
 		return;
 	}
 
+	mpu_dpll_ck = clk_get(NULL, "dpll1_ck");
+	WARN(!mpu_dpll_ck, "Can't get dpll1_ck\n");
+	core_dpll_ck = clk_get(NULL, "dpll3_ck");
+	WARN(!core_dpll_ck, "Can't get dpll3_ck\n");
+
 	v = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
 	v &= ~AM35XX_CPGMACSS_SW_RST;
 	omap_ctrl_writel(v, AM35XX_CONTROL_IP_SW_RESET);
diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
index 15c6f9c..446a429 100644
--- a/arch/arm/mach-omap2/am35xx-emac.h
+++ b/arch/arm/mach-omap2/am35xx-emac.h
@@ -5,11 +5,20 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <plat/omap_device.h>
 
 #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
 
 #if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
-void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
+extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
+extern int am35xx_emac_pm_lats_size;
+
+void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
 #else
-static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
+#define am35xx_emac_pm_lats		NULL
+#define am35xx_emac_pm_lats_size	0
+
+static inline am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
 #endif
diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
index 18f6010..5348d0d 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -368,7 +368,8 @@ static void __init am3517_evm_init(void)
 	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
 				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
 	/*Ethernet*/
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 
 	/* MUSB */
 	am3517_evm_musb_init();
diff --git a/arch/arm/mach-omap2/board-cm-t3517.c b/arch/arm/mach-omap2/board-cm-t3517.c
index a33ad46..8258057 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -292,7 +292,8 @@ static void __init cm_t3517_init(void)
 	cm_t3517_init_rtc();
 	cm_t3517_init_usbh();
 	cm_t3517_init_hecc();
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 }
 
 MACHINE_START(CM_T3517, "Compulab CM-T3517")

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Paul Walmsley]

On Wed, 18 Jul 2012, Mark A. Greer wrote:

> On Tue, Jul 17, 2012 at 09:54:53PM -0600, Paul Walmsley wrote:
> 
> > Want to try something like this?  It's your patch but modified to not use 
> > disable/enable_hlt().  If it doesn't work in your test case, maybe 
> > try uncommenting that second set of deny_idle / allow_idle ...
> 
> I tested the modified patch (to get it to compile) below.

Doh, sorry about that. 

> It did not work with or without the core_dpll_ck deny_idle/allow_idle
> commented out.

Here's a version with some of the CPUIdle states restricted.  Maybe try 
this one if you're using CPUIdle?

If it happens to work, it would also be interesting to know if it works 
with the CORE DPLL part commented out.


- Paul

---
 arch/arm/mach-omap2/am35xx-emac.c     |   56 ++++++++++++++++++++++++++++++---
 arch/arm/mach-omap2/am35xx-emac.h     |   13 ++++++--
 arch/arm/mach-omap2/board-am3517evm.c |    3 +-
 arch/arm/mach-omap2/board-cm-t3517.c  |    3 +-
 arch/arm/mach-omap2/cpuidle34xx.c     |    5 +++
 5 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 2c90ac6..6b0edcd 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -16,13 +16,50 @@
  */
 
 #include <linux/err.h>
+#include <linux/clk.h>
 #include <linux/davinci_emac.h>
 #include <asm/system.h>
+#include <plat/clock.h>
 #include <plat/omap_device.h>
 #include <mach/am35xx.h>
 #include "control.h"
 #include "am35xx-emac.h"
 
+static struct clk *mpu_dpll_ck, *core_dpll_ck;
+
+/*
+ * Default pm_lats for the am35x.
+ * The net effect of using am35xx_emac_pm_lats[] is that
+ * pm_idle or CPUidle won't be called while the emac
+ * interface is open.  This is required because the
+ * EMAC can't wake up PRCM so if the MPU is executing
+ * a 'wfi' instruction (e.g., from pm_idle or CPUidle),
+ * it won't break out of it due to emac activity.
+ */
+static int am35xx_emac_deactivate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->deny_idle(mpu_dpll_ck);
+	core_dpll_ck->ops->deny_idle(core_dpll_ck);
+	return omap_device_idle_hwmods(od);
+}
+
+static int am35xx_emac_activate_func(struct omap_device *od)
+{
+	mpu_dpll_ck->ops->allow_idle(mpu_dpll_ck);
+	core_dpll_ck->ops->allow_idle(core_dpll_ck);
+	return omap_device_enable_hwmods(od);
+}
+
+struct omap_device_pm_latency am35xx_emac_pm_lats[] = {
+	{
+		.deactivate_func	= am35xx_emac_deactivate_func,
+		.activate_func		= am35xx_emac_activate_func,
+		.flags			= OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	},
+};
+
+int am35xx_emac_pm_lats_size = ARRAY_SIZE(am35xx_emac_pm_lats);
+
 static void am35xx_enable_emac_int(void)
 {
 	u32 v;
@@ -58,12 +95,14 @@ static struct emac_platform_data am35xx_emac_pdata = {
 static struct mdio_platform_data am35xx_mdio_pdata;
 
 static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
-		void *pdata, int pdata_len)
+					     void *pdata, int pdata_len,
+					     struct omap_device_pm_latency *pm_lats,
+					     int pm_lats_size)
 {
 	struct platform_device *pdev;
 
 	pdev = omap_device_build(oh->class->name, 0, oh, pdata, pdata_len,
-				 NULL, 0, false);
+				 pm_lats, pm_lats_size, false);
 	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 		     oh->class->name, oh->name);
@@ -73,7 +112,8 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh,
 	return 0;
 }
 
-void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
+void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size)
 {
 	struct omap_hwmod *oh;
 	u32 v;
@@ -88,7 +128,7 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_mdio_pdata.bus_freq = mdio_bus_freq;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_mdio_pdata,
-					 sizeof(am35xx_mdio_pdata));
+					 sizeof(am35xx_mdio_pdata), NULL, 0);
 	if (ret) {
 		pr_err("Could not build davinci_mdio hwmod device\n");
 		return;
@@ -103,12 +143,18 @@ void __init am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en)
 	am35xx_emac_pdata.rmii_en = rmii_en;
 
 	ret = omap_davinci_emac_dev_init(oh, &am35xx_emac_pdata,
-					 sizeof(am35xx_emac_pdata));
+					 sizeof(am35xx_emac_pdata),
+					 pm_lats, pm_lats_size);
 	if (ret) {
 		pr_err("Could not build davinci_emac hwmod device\n");
 		return;
 	}
 
+	mpu_dpll_ck = clk_get(NULL, "dpll1_ck");
+	WARN(!mpu_dpll_ck, "Can't get dpll1_ck\n");
+	core_dpll_ck = clk_get(NULL, "dpll3_ck");
+	WARN(!core_dpll_ck, "Can't get dpll3_ck\n");
+
 	v = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
 	v &= ~AM35XX_CPGMACSS_SW_RST;
 	omap_ctrl_writel(v, AM35XX_CONTROL_IP_SW_RESET);
diff --git a/arch/arm/mach-omap2/am35xx-emac.h b/arch/arm/mach-omap2/am35xx-emac.h
index 15c6f9c..446a429 100644
--- a/arch/arm/mach-omap2/am35xx-emac.h
+++ b/arch/arm/mach-omap2/am35xx-emac.h
@@ -5,11 +5,20 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <plat/omap_device.h>
 
 #define AM35XX_DEFAULT_MDIO_FREQUENCY	1000000
 
 #if defined(CONFIG_TI_DAVINCI_EMAC) || defined(CONFIG_TI_DAVINCI_EMAC_MODULE)
-void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en);
+extern struct omap_device_pm_latency am35xx_emac_pm_lats[];
+extern int am35xx_emac_pm_lats_size;
+
+void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size);
 #else
-static inline void am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en) {}
+#define am35xx_emac_pm_lats		NULL
+#define am35xx_emac_pm_lats_size	0
+
+static inline am35xx_emac_init(unsigned long mdio_bus_freq, u8 rmii_en,
+		struct omap_device_pm_latency *pm_lats, int pm_lats_size) {}
 #endif
diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
index 18f6010..5348d0d 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -368,7 +368,8 @@ static void __init am3517_evm_init(void)
 	i2c_register_board_info(1, am3517evm_i2c1_boardinfo,
 				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
 	/*Ethernet*/
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 
 	/* MUSB */
 	am3517_evm_musb_init();
diff --git a/arch/arm/mach-omap2/board-cm-t3517.c b/arch/arm/mach-omap2/board-cm-t3517.c
index a33ad46..8258057 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -292,7 +292,8 @@ static void __init cm_t3517_init(void)
 	cm_t3517_init_rtc();
 	cm_t3517_init_usbh();
 	cm_t3517_init_hecc();
-	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1);
+	am35xx_emac_init(AM35XX_DEFAULT_MDIO_FREQUENCY, 1,
+			 am35xx_emac_pm_lats, am35xx_emac_pm_lats_size);
 }
 
 MACHINE_START(CM_T3517, "Compulab CM-T3517")
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index f2a49a4..d95dd8c 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -291,6 +291,7 @@ struct cpuidle_driver omap3_idle_driver = {
 			.flags		  = CPUIDLE_FLAG_TIME_VALID,
 			.name		  = "C3",
 			.desc		  = "MPU RET + CORE ON",
+			.disable	  = 1,
 		},
 		{
 			.enter		  = omap3_enter_idle_bm,
@@ -299,6 +300,7 @@ struct cpuidle_driver omap3_idle_driver = {
 			.flags		  = CPUIDLE_FLAG_TIME_VALID,
 			.name		  = "C4",
 			.desc		  = "MPU OFF + CORE ON",
+			.disable	  = 1,
 		},
 		{
 			.enter		  = omap3_enter_idle_bm,
@@ -307,6 +309,7 @@ struct cpuidle_driver omap3_idle_driver = {
 			.flags		  = CPUIDLE_FLAG_TIME_VALID,
 			.name		  = "C5",
 			.desc		  = "MPU RET + CORE RET",
+			.disable	  = 1,
 		},
 		{
 			.enter		  = omap3_enter_idle_bm,
@@ -315,6 +318,7 @@ struct cpuidle_driver omap3_idle_driver = {
 			.flags		  = CPUIDLE_FLAG_TIME_VALID,
 			.name		  = "C6",
 			.desc		  = "MPU OFF + CORE RET",
+			.disable	  = 1,
 		},
 		{
 			.enter		  = omap3_enter_idle_bm,
@@ -323,6 +327,7 @@ struct cpuidle_driver omap3_idle_driver = {
 			.flags		  = CPUIDLE_FLAG_TIME_VALID,
 			.name		  = "C7",
 			.desc		  = "MPU OFF + CORE OFF",
+			.disable	  = 1,
 		},
 	},
 	.state_count = ARRAY_SIZE(omap3_idle_data),
-- 
1.7.10.4

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Wed, Jul 18, 2012 at 05:25:16PM -0600, Paul Walmsley wrote:
> On Wed, 18 Jul 2012, Mark A. Greer wrote:
> 
> > On Tue, Jul 17, 2012 at 09:54:53PM -0600, Paul Walmsley wrote:
> > 
> > > Want to try something like this?  It's your patch but modified to not use 
> > > disable/enable_hlt().  If it doesn't work in your test case, maybe 
> > > try uncommenting that second set of deny_idle / allow_idle ...
> > 
> > I tested the modified patch (to get it to compile) below.
> 
> Doh, sorry about that. 

No worries.

> > It did not work with or without the core_dpll_ck deny_idle/allow_idle
> > commented out.
> 
> Here's a version with some of the CPUIdle states restricted.  Maybe try 
> this one if you're using CPUIdle?

I wasn't but I tried this lastest patch with and without CPUidle...

> If it happens to work, it would also be interesting to know if it works 
> with the CORE DPLL part commented out.

...and, unfortunately, it didnt' work in either case.

Mark

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Paul Walmsley]

On Thu, 19 Jul 2012, Mark A. Greer wrote:

> ...and, unfortunately, it didnt' work in either case.

OK thanks for the tests.  Is the EMAC/MDIO really active and asserting 
interrupts while all this is happening?  Or has that driver called 
pm_runtime_put*(), and so the EMAC/MDIO isn't waking up? 


- Paul

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Thu, Jul 19, 2012 at 01:19:13PM -0600, Paul Walmsley wrote:
> On Thu, 19 Jul 2012, Mark A. Greer wrote:
> 
> > ...and, unfortunately, it didnt' work in either case.
> 
> OK thanks for the tests.  Is the EMAC/MDIO really active and asserting 
> interrupts while all this is happening?

I should be active but I have not way to tell for sure.

> Or has that driver called 
> pm_runtime_put*(), and so the EMAC/MDIO isn't waking up? 

pm_runtime_get has definitely been called and pm_runtime_put
has definitely _not_ been called.

Mark

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Paul Walmsley]

+ Ilya

Hi Mark

Maybe try something like this on top of the patch that disables the 
MPU DPLL autoidle?

I don't know what am35xx_enable_emac_int() is supposed to do.  It seems 
strange to clear the interrupt status bits when one is supposed to enable 
the interrupts.  Maybe Ilya can shed some light on it.


- Paul

---
 arch/arm/mach-omap2/am35xx-emac.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c
index 2c90ac6..231190e 100644
--- a/arch/arm/mach-omap2/am35xx-emac.c
+++ b/arch/arm/mach-omap2/am35xx-emac.c
@@ -23,23 +23,17 @@
 #include "control.h"
 #include "am35xx-emac.h"
 
-static void am35xx_enable_emac_int(void)
-{
-	u32 v;
-
-	v = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR);
-	v |= (AM35XX_CPGMAC_C0_RX_PULSE_CLR | AM35XX_CPGMAC_C0_TX_PULSE_CLR |
-	      AM35XX_CPGMAC_C0_MISC_PULSE_CLR | AM35XX_CPGMAC_C0_RX_THRESH_CLR);
-	omap_ctrl_writel(v, AM35XX_CONTROL_LVL_INTR_CLEAR);
-	omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); /* OCP barrier */
-}
-
 static void am35xx_disable_emac_int(void)
 {
 	u32 v;
 
-	v = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR);
-	v |= (AM35XX_CPGMAC_C0_RX_PULSE_CLR | AM35XX_CPGMAC_C0_TX_PULSE_CLR);
+	/* XXX What about the misc interrupts? */
+	/*
+	 * XXX MDIO driver should handle its interrupts through the EMAC
+	 * driver
+	 */
+	v = (AM35XX_CPGMAC_C0_RX_PULSE_CLR | AM35XX_CPGMAC_C0_TX_PULSE_CLR |
+	     AM35XX_CPGMAC_C0_MISC_PULSE_CLR | AM35XX_CPGMAC_C0_RX_THRESH_CLR);
 	omap_ctrl_writel(v, AM35XX_CONTROL_LVL_INTR_CLEAR);
 	omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); /* OCP barrier */
 }
@@ -51,7 +45,6 @@ static struct emac_platform_data am35xx_emac_pdata = {
 	.ctrl_ram_size		= AM35XX_EMAC_CNTRL_RAM_SIZE,
 	.hw_ram_addr		= AM35XX_EMAC_HW_RAM_ADDR,
 	.version		= EMAC_VERSION_2,
-	.interrupt_enable	= am35xx_enable_emac_int,
 	.interrupt_disable	= am35xx_disable_emac_int,
 };
 
-- 
1.7.10.4

Re: [PATCH 2/2] arm: omap3: am35x: Disable hlt when using Davinci EMAC

[Mark A. Greer]

On Thu, Jul 19, 2012 at 04:59:06PM -0600, Paul Walmsley wrote:
> + Ilya
> 
> Hi Mark
> 
> Maybe try something like this on top of the patch that disables the 
> MPU DPLL autoidle?
> 
> I don't know what am35xx_enable_emac_int() is supposed to do.  It seems 
> strange to clear the interrupt status bits when one is supposed to enable 
> the interrupts.  Maybe Ilya can shed some light on it.

Just an FYI for anyone following this thread.  Paul has contacted me
privately and said to not bother testing this patch.

Mark

[RFC] net: ethernet: davinci_emac: add pm_runtime support

[Mark A. Greer]

From: "Mark A. Greer" <mgreer@animalcreek.com>

Add pm_runtime support to the TI Davinci EMAC driver.

Signed-off-by: Mark A. Greer <mgreer@animalcreek.com>
---

This patch is RFC only since it will break emac support
for mach-davinci platforms.  Hopefully, mach-davinci will
be fixed soon and this can be submitted.

 drivers/net/ethernet/ti/davinci_emac.c |   43 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 174a334..15c84e4 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -57,6 +57,7 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/uaccess.h>
+#include <linux/pm_runtime.h>
 #include <linux/davinci_emac.h>
 
 #include <asm/irq.h>
@@ -346,10 +347,6 @@ struct emac_priv {
 	void (*int_disable) (void);
 };
 
-/* clock frequency for EMAC */
-static struct clk *emac_clk;
-static unsigned long emac_bus_frequency;
-
 /* EMAC TX Host Error description strings */
 static char *emac_txhost_errcodes[16] = {
 	"No error", "SOP error", "Ownership bit not set in SOP buffer",
@@ -1534,6 +1531,8 @@ static int emac_dev_open(struct net_device *ndev)
 	int k = 0;
 	struct emac_priv *priv = netdev_priv(ndev);
 
+	pm_runtime_get(&priv->pdev->dev);
+
 	netif_carrier_off(ndev);
 	for (cnt = 0; cnt < ETH_ALEN; cnt++)
 		ndev->dev_addr[cnt] = priv->mac_addr[cnt];
@@ -1603,7 +1602,7 @@ static int emac_dev_open(struct net_device *ndev)
 				priv->phy_id);
 			ret = PTR_ERR(priv->phydev);
 			priv->phydev = NULL;
-			return ret;
+			goto err;
 		}
 
 		priv->link = 0;
@@ -1644,7 +1643,11 @@ rollback:
 		res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k-1);
 		m = res->end;
 	}
-	return -EBUSY;
+
+	ret = -EBUSY;
+err:
+	pm_runtime_put(&priv->pdev->dev);
+	return ret;
 }
 
 /**
@@ -1686,6 +1689,7 @@ static int emac_dev_stop(struct net_device *ndev)
 	if (netif_msg_drv(priv))
 		dev_notice(emac_dev, "DaVinci EMAC: %s stopped\n", ndev->name);
 
+	pm_runtime_put(&priv->pdev->dev);
 	return 0;
 }
 
@@ -1779,6 +1783,9 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
 	struct emac_platform_data *pdata;
 	struct device *emac_dev;
 	struct cpdma_params dma_params;
+	struct clk *emac_clk;
+	unsigned long emac_bus_frequency;
+
 
 	/* obtain emac clock from kernel */
 	emac_clk = clk_get(&pdev->dev, NULL);
@@ -1787,12 +1794,14 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 	emac_bus_frequency = clk_get_rate(emac_clk);
+	clk_put(emac_clk);
+
 	/* TODO: Probe PHY here if possible */
 
 	ndev = alloc_etherdev(sizeof(struct emac_priv));
 	if (!ndev) {
 		rc = -ENOMEM;
-		goto free_clk;
+		goto no_ndev;
 	}
 
 	platform_set_drvdata(pdev, ndev);
@@ -1908,15 +1917,13 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
 	SET_ETHTOOL_OPS(ndev, &ethtool_ops);
 	netif_napi_add(ndev, &priv->napi, emac_poll, EMAC_POLL_WEIGHT);
 
-	clk_enable(emac_clk);
-
 	/* register the network device */
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 	rc = register_netdev(ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "error in register_netdev\n");
 		rc = -ENODEV;
-		goto netdev_reg_err;
+		goto no_irq_res;
 	}
 
 
@@ -1925,10 +1932,12 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
 			   "(regs: %p, irq: %d)\n",
 			   (void *)priv->emac_base_phys, ndev->irq);
 	}
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_resume(&pdev->dev);
+
 	return 0;
 
-netdev_reg_err:
-	clk_disable(emac_clk);
 no_irq_res:
 	if (priv->txchan)
 		cpdma_chan_destroy(priv->txchan);
@@ -1942,8 +1951,7 @@ no_dma:
 
 probe_quit:
 	free_netdev(ndev);
-free_clk:
-	clk_put(emac_clk);
+no_ndev:
 	return rc;
 }
 
@@ -1977,9 +1985,6 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
 	iounmap(priv->remap_addr);
 	free_netdev(ndev);
 
-	clk_disable(emac_clk);
-	clk_put(emac_clk);
-
 	return 0;
 }
 
@@ -1991,8 +1996,6 @@ static int davinci_emac_suspend(struct device *dev)
 	if (netif_running(ndev))
 		emac_dev_stop(ndev);
 
-	clk_disable(emac_clk);
-
 	return 0;
 }
 
@@ -2001,8 +2004,6 @@ static int davinci_emac_resume(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct net_device *ndev = platform_get_drvdata(pdev);
 
-	clk_enable(emac_clk);
-
 	if (netif_running(ndev))
 		emac_dev_open(ndev);
 
-- 
1.7.9.4

Re: [PATCH 0/2] arm: omap3: am35x: Convert emac to hwmod & disable hlt when open

[Kevin Hilman]

+Sekhar,

"Mark A. Greer" <mgreer@animalcreek.com> writes:

> From: "Mark A. Greer" <mgreer@animalcreek.com>
>
> Paul, Kevin,
>
> These patches convert the davinci emac support for the am35x SoC
> to use hwmod and add enable_hlt()/disable_hlt() calls to the
> pm_runtime hooks for that driver.

Great.  I didn't look closely at the hwmod data, but the approach is
right here, IMO.

> I have converted the davinci_emac driver to use pm_runtime but I
> can't formally submit it yet since it requires some changes on the
> mach-davinci side that haven't happened yet.  I will send it as an
> RFC in a reply to this thread.

Sekhar, are you planning to add runtime PM core support to davinci?
I recommend looking at the simple OMAP1 layer that implements a basic
clocks-only runtime PM layer.

Kevin

> The patches are based on:
> 	git://git.pwsan.com/linux-2.6 am35xx_hwmod_data_fixes_a_3.5
>
> Mark
> --
>
> Mark A. Greer (2):
>   arm: omap3: am35x: Add Davinci EMAC/MDIO hwmod support
>   arm: omap3: am35x: Disable hlt when using Davinci EMAC
>
>  arch/arm/mach-omap2/am35xx-emac.c          |  120 ++++++++++++++++++----------
>  arch/arm/mach-omap2/am35xx-emac.h          |   16 +++-
>  arch/arm/mach-omap2/board-am3517evm.c      |    3 +-
>  arch/arm/mach-omap2/board-cm-t3517.c       |    3 +-
>  arch/arm/mach-omap2/clock3xxx_data.c       |    2 +-
>  arch/arm/mach-omap2/include/mach/am35xx.h  |    2 +
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   85 ++++++++++++++++++++
>  7 files changed, 183 insertions(+), 48 deletions(-)

Re: [PATCH 0/2] arm: omap3: am35x: Convert emac to hwmod & disable hlt when open

[Sekhar Nori]

Hi Kevin,

On 5/15/2012 4:58 AM, Kevin Hilman wrote:
> +Sekhar,
> 
> "Mark A. Greer" <mgreer@animalcreek.com> writes:
> 
>> From: "Mark A. Greer" <mgreer@animalcreek.com>
>>
>> Paul, Kevin,
>>
>> These patches convert the davinci emac support for the am35x SoC
>> to use hwmod and add enable_hlt()/disable_hlt() calls to the
>> pm_runtime hooks for that driver.
> 
> Great.  I didn't look closely at the hwmod data, but the approach is
> right here, IMO.
> 
>> I have converted the davinci_emac driver to use pm_runtime but I
>> can't formally submit it yet since it requires some changes on the
>> mach-davinci side that haven't happened yet.  I will send it as an
>> RFC in a reply to this thread.
> 
> Sekhar, are you planning to add runtime PM core support to davinci?

Yes, Vaibhav is going to be working on this and will start very soon.

> I recommend looking at the simple OMAP1 layer that implements a basic
> clocks-only runtime PM layer.

Thanks. I have CCed Vaibhav on this mail for his information.

Regards,
Sekhar