diff for duplicates of <6176616fe77333f80f0cfaae4fc8aff3@mail.gmail.com> diff --git a/a/1.txt b/N1/1.txt index 7699e19..958efc9 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,9 +1,9 @@ > -----Original Message----- -> From: Jean Pihet [mailto:jean.pihet@newoldbits.com] +> From: Jean Pihet [mailto:jean.pihet at newoldbits.com] > Sent: Monday, February 21, 2011 3:49 PM > To: Santosh Shilimkar -> Cc: linux-omap@vger.kernel.org; khilman@ti.com; linux-arm- -> kernel@lists.infradead.org; Rajendra Nayak +> Cc: linux-omap at vger.kernel.org; khilman at ti.com; linux-arm- +> kernel at lists.infradead.org; Rajendra Nayak > Subject: Re: [PATCH 14/17] omap4: cpuidle: Add MPUSS RET OFF states > > Hi Santosh, @@ -17,48 +17,48 @@ > > + * not board dependent and hence set directly here instead of > > + * passing it from board files. > > + */ -> > static struct cpuidle_params cpuidle_params_table[] = { -> > - /* C1 */ -> > - {1, 2, 2, 5}, -> > + /* C1 - CPU0 WFI + CPU1 ON/OFF + MPU ON + CORE ON */ -> > + {1, 2, 2, 5}, -> > + /* C2 - CPU0 ON + CPU1 OFF + MPU ON + CORE ON */ -> > + {1, 140, 160, 300}, -> > + /* C3 - CPU0 OFF + CPU1 OFF + MPU CSWR + CORE ON */ -> > + {1, 200, 300, 700}, -> > + /* C4 - CPU0 OFF + CPU1 OFF + MPU OFF + CORE ON */ -> > + {1, 1400, 600, 5000}, -> > }; +> > ?static struct cpuidle_params cpuidle_params_table[] = { +> > - ? ? ? /* C1 */ +> > - ? ? ? {1, 2, 2, 5}, +> > + ? ? ? /* C1 - CPU0 WFI + CPU1 ON/OFF + MPU ON ? + CORE ON */ +> > + ? ? ? {1, ? ? 2, ? ? ?2, ? ? ?5}, +> > + ? ? ? /* C2 - CPU0 ON + CPU1 OFF + MPU ON ?+ CORE ON */ +> > + ? ? ? {1, ? ? 140, ? ?160, ? ?300}, +> > + ? ? ? /* C3 - CPU0 OFF + CPU1 OFF + MPU CSWR + CORE ON */ +> > + ? ? ? {1, ? ? 200, ? ?300, ? ?700}, +> > + ? ? ? /* C4 - CPU0 OFF + CPU1 OFF + MPU OFF + CORE ON */ +> > + ? ? ? {1, ? ? 1400, ? 600, ? ?5000}, +> > ?}; > > > > +DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev); > > + > > +static int omap4_idle_bm_check(void) > > +{ -> > + /* FIXME: Populate this with CORE retention support */ -> > + return 0; +> > + ? ? ? /* FIXME: Populate this with CORE retention support */ +> > + ? ? ? return 0; > > +} > > + -> > /** -> > * omap4_enter_idle - Programs OMAP4 to enter the specified state -> > * @dev: cpuidle device +> > ?/** +> > ?* omap4_enter_idle - Programs OMAP4 to enter the specified state +> > ?* @dev: cpuidle device > > @@ -57,7 +92,9 @@ static struct cpuidle_params > cpuidle_params_table[] = { -> > static int omap4_enter_idle(struct cpuidle_device *dev, -> > struct cpuidle_state *state) -> > { -> > + struct omap4_processor_cx *cx = +> > ?static int omap4_enter_idle(struct cpuidle_device *dev, +> > ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_state *state) +> > ?{ +> > + ? ? ? struct omap4_processor_cx *cx = > cpuidle_get_statedata(state); -> > struct timespec ts_preidle, ts_postidle, ts_idle; -> > + u32 cpu1_state; +> > ? ? ? ?struct timespec ts_preidle, ts_postidle, ts_idle; +> > + ? ? ? u32 cpu1_state; > > -> > /* Used to keep track of the total time in idle */ -> > getnstimeofday(&ts_preidle); +> > ? ? ? ?/* Used to keep track of the total time in idle */ +> > ? ? ? ?getnstimeofday(&ts_preidle); > > @@ -65,28 +102,74 @@ static int omap4_enter_idle(struct > cpuidle_device *dev, -> > local_irq_disable(); -> > local_fiq_disable(); +> > ? ? ? ?local_irq_disable(); +> > ? ? ? ?local_fiq_disable(); > > -> > - cpu_do_idle(); +> > - ? ? ? cpu_do_idle(); > > I think the piece of code below is rather difficult to read and > understand. Based on the patch description and the comment here @@ -74,10 +74,10 @@ The hardware constraint is back-ground behind the conditions. > through hotplug infrastructure. " > The test below does not seem to match this comment. > -> > + /* -> > + * Special hardware/software considerations: -> > + * 1. Do only WFI for secondary CPU(non-boot - CPU1). -> > + * Secondary cores are taken down only via hotplug +> > + ? ? ? /* +> > + ? ? ? ?* Special hardware/software considerations: +> > + ? ? ? ?* 1. Do only WFI for secondary CPU(non-boot - CPU1). +> > + ? ? ? ?* ? ?Secondary cores are taken down only via hotplug > path. > The comment looks contradictory. Which one is taken OFF using this > code, which one from hotplug? @@ -86,30 +86,30 @@ The hardware constraint is back-ground behind the conditions. Yes. > -> > + * 2. Do only a WFI as long as in SMP mode. +> > + ? ? ? ?* 2. Do only a WFI as long as in SMP mode. > Does this correspond to the condition '(num_online_cpus() > 1)' in > the > test below? If so it this one triggering the low power mode for > cpu0? yes > -> > + * 3. Continue to do only WFI till CPU1 hits OFF state. -> > + * This is necessary to honour hardware recommondation -> > + * of triggeing all the possible low power modes once +> > + ? ? ? ?* 3. Continue to do only WFI till CPU1 hits OFF state. +> > + ? ? ? ?* ? ?This is necessary to honour hardware recommondation +> > + ? ? ? ?* ? ?of triggeing all the possible low power modes once > CPU1 is -> > + * out of coherency and in OFF mode. +> > + ? ? ? ?* ? ?out of coherency and in OFF mode. > Does this correspond to the condition '(cpu1_state != > PWRDM_POWER_OFF)' in the test below? > Yes -> > + * Update dev->last_state so that governor stats reflects +> > + ? ? ? ?* Update dev->last_state so that governor stats reflects > right -> > + * data. -> > + */ -> > + cpu1_state = pwrdm_read_pwrst(cpu1_pd); -> > + if ((dev->cpu) || (num_online_cpus() > 1) || -> > + (cpu1_state != PWRDM_POWER_OFF)) { +> > + ? ? ? ?* data. +> > + ? ? ? ?*/ +> > + ? ? ? cpu1_state = pwrdm_read_pwrst(cpu1_pd); +> > + ? ? ? if ((dev->cpu) || (num_online_cpus() > 1) || +> > + ? ? ? ? ? ? ? ? ? ? ? (cpu1_state != PWRDM_POWER_OFF)) { > Are '||' correct here? Yes. @@ -123,7 +123,3 @@ And you got all of them correctly. Regards, Santosh --- -To unsubscribe from this list: send the line "unsubscribe linux-omap" in -the body of a message to majordomo@vger.kernel.org -More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N1/content_digest index 68dd44c..8ccadd8 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,22 +1,18 @@ "ref\01298112158-28469-1-git-send-email-santosh.shilimkar@ti.com\0" "ref\01298112158-28469-15-git-send-email-santosh.shilimkar@ti.com\0" "ref\0AANLkTik-hj+-ZPnfVKLCH0h-QeyziH4H-j6EE+_QHHCU@mail.gmail.com\0" - "From\0Santosh Shilimkar <santosh.shilimkar@ti.com>\0" - "Subject\0RE: [PATCH 14/17] omap4: cpuidle: Add MPUSS RET OFF states\0" + "From\0santosh.shilimkar@ti.com (Santosh Shilimkar)\0" + "Subject\0[PATCH 14/17] omap4: cpuidle: Add MPUSS RET OFF states\0" "Date\0Mon, 21 Feb 2011 15:56:57 +0530\0" - "To\0Jean Pihet <jean.pihet@newoldbits.com>\0" - "Cc\0linux-omap@vger.kernel.org" - Kevin Hilman <khilman@ti.com> - linux-arm-kernel@lists.infradead.org - " Rajendra Nayak <rnayak@ti.com>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "> -----Original Message-----\n" - "> From: Jean Pihet [mailto:jean.pihet@newoldbits.com]\n" + "> From: Jean Pihet [mailto:jean.pihet at newoldbits.com]\n" "> Sent: Monday, February 21, 2011 3:49 PM\n" "> To: Santosh Shilimkar\n" - "> Cc: linux-omap@vger.kernel.org; khilman@ti.com; linux-arm-\n" - "> kernel@lists.infradead.org; Rajendra Nayak\n" + "> Cc: linux-omap at vger.kernel.org; khilman at ti.com; linux-arm-\n" + "> kernel at lists.infradead.org; Rajendra Nayak\n" "> Subject: Re: [PATCH 14/17] omap4: cpuidle: Add MPUSS RET OFF states\n" ">\n" "> Hi Santosh,\n" @@ -30,48 +26,48 @@ "> > + * not board dependent and hence set directly here instead of\n" "> > + * passing it from board files.\n" "> > + */\n" - "> > \302\240static struct cpuidle_params cpuidle_params_table[] = {\n" - "> > - \302\240 \302\240 \302\240 /* C1 */\n" - "> > - \302\240 \302\240 \302\240 {1, 2, 2, 5},\n" - "> > + \302\240 \302\240 \302\240 /* C1 - CPU0 WFI + CPU1 ON/OFF + MPU ON \302\240 + CORE ON */\n" - "> > + \302\240 \302\240 \302\240 {1, \302\240 \302\240 2, \302\240 \302\240 \302\2402, \302\240 \302\240 \302\2405},\n" - "> > + \302\240 \302\240 \302\240 /* C2 - CPU0 ON + CPU1 OFF + MPU ON \302\240+ CORE ON */\n" - "> > + \302\240 \302\240 \302\240 {1, \302\240 \302\240 140, \302\240 \302\240160, \302\240 \302\240300},\n" - "> > + \302\240 \302\240 \302\240 /* C3 - CPU0 OFF + CPU1 OFF + MPU CSWR + CORE ON */\n" - "> > + \302\240 \302\240 \302\240 {1, \302\240 \302\240 200, \302\240 \302\240300, \302\240 \302\240700},\n" - "> > + \302\240 \302\240 \302\240 /* C4 - CPU0 OFF + CPU1 OFF + MPU OFF + CORE ON */\n" - "> > + \302\240 \302\240 \302\240 {1, \302\240 \302\240 1400, \302\240 600, \302\240 \302\2405000},\n" - "> > \302\240};\n" + "> > ?static struct cpuidle_params cpuidle_params_table[] = {\n" + "> > - ? ? ? /* C1 */\n" + "> > - ? ? ? {1, 2, 2, 5},\n" + "> > + ? ? ? /* C1 - CPU0 WFI + CPU1 ON/OFF + MPU ON ? + CORE ON */\n" + "> > + ? ? ? {1, ? ? 2, ? ? ?2, ? ? ?5},\n" + "> > + ? ? ? /* C2 - CPU0 ON + CPU1 OFF + MPU ON ?+ CORE ON */\n" + "> > + ? ? ? {1, ? ? 140, ? ?160, ? ?300},\n" + "> > + ? ? ? /* C3 - CPU0 OFF + CPU1 OFF + MPU CSWR + CORE ON */\n" + "> > + ? ? ? {1, ? ? 200, ? ?300, ? ?700},\n" + "> > + ? ? ? /* C4 - CPU0 OFF + CPU1 OFF + MPU OFF + CORE ON */\n" + "> > + ? ? ? {1, ? ? 1400, ? 600, ? ?5000},\n" + "> > ?};\n" "> >\n" "> > +DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev);\n" "> > +\n" "> > +static int omap4_idle_bm_check(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 /* FIXME: Populate this with CORE retention support */\n" - "> > + \302\240 \302\240 \302\240 return 0;\n" + "> > + ? ? ? /* FIXME: Populate this with CORE retention support */\n" + "> > + ? ? ? return 0;\n" "> > +}\n" "> > +\n" - "> > \302\240/**\n" - "> > \302\240* omap4_enter_idle - Programs OMAP4 to enter the specified state\n" - "> > \302\240* @dev: cpuidle device\n" + "> > ?/**\n" + "> > ?* omap4_enter_idle - Programs OMAP4 to enter the specified state\n" + "> > ?* @dev: cpuidle device\n" "> > @@ -57,7 +92,9 @@ static struct cpuidle_params\n" "> cpuidle_params_table[] = {\n" - "> > \302\240static int omap4_enter_idle(struct cpuidle_device *dev,\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240struct cpuidle_state *state)\n" - "> > \302\240{\n" - "> > + \302\240 \302\240 \302\240 struct omap4_processor_cx *cx =\n" + "> > ?static int omap4_enter_idle(struct cpuidle_device *dev,\n" + "> > ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_state *state)\n" + "> > ?{\n" + "> > + ? ? ? struct omap4_processor_cx *cx =\n" "> cpuidle_get_statedata(state);\n" - "> > \302\240 \302\240 \302\240 \302\240struct timespec ts_preidle, ts_postidle, ts_idle;\n" - "> > + \302\240 \302\240 \302\240 u32 cpu1_state;\n" + "> > ? ? ? ?struct timespec ts_preidle, ts_postidle, ts_idle;\n" + "> > + ? ? ? u32 cpu1_state;\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\240/* Used to keep track of the total time in idle */\n" - "> > \302\240 \302\240 \302\240 \302\240getnstimeofday(&ts_preidle);\n" + "> > ? ? ? ?/* Used to keep track of the total time in idle */\n" + "> > ? ? ? ?getnstimeofday(&ts_preidle);\n" "> > @@ -65,28 +102,74 @@ static int omap4_enter_idle(struct\n" "> cpuidle_device *dev,\n" - "> > \302\240 \302\240 \302\240 \302\240local_irq_disable();\n" - "> > \302\240 \302\240 \302\240 \302\240local_fiq_disable();\n" + "> > ? ? ? ?local_irq_disable();\n" + "> > ? ? ? ?local_fiq_disable();\n" "> >\n" - "> > - \302\240 \302\240 \302\240 cpu_do_idle();\n" + "> > - ? ? ? cpu_do_idle();\n" ">\n" "> I think the piece of code below is rather difficult to read and\n" "> understand. Based on the patch description and the comment here\n" @@ -87,10 +83,10 @@ "> through hotplug infrastructure. \"\n" "> The test below does not seem to match this comment.\n" ">\n" - "> > + \302\240 \302\240 \302\240 /*\n" - "> > + \302\240 \302\240 \302\240 \302\240* Special hardware/software considerations:\n" - "> > + \302\240 \302\240 \302\240 \302\240* 1. Do only WFI for secondary CPU(non-boot - CPU1).\n" - "> > + \302\240 \302\240 \302\240 \302\240* \302\240 \302\240Secondary cores are taken down only via hotplug\n" + "> > + ? ? ? /*\n" + "> > + ? ? ? ?* Special hardware/software considerations:\n" + "> > + ? ? ? ?* 1. Do only WFI for secondary CPU(non-boot - CPU1).\n" + "> > + ? ? ? ?* ? ?Secondary cores are taken down only via hotplug\n" "> path.\n" "> The comment looks contradictory. Which one is taken OFF using this\n" "> code, which one from hotplug?\n" @@ -99,30 +95,30 @@ "\n" "Yes.\n" ">\n" - "> > + \302\240 \302\240 \302\240 \302\240* 2. Do only a WFI as long as in SMP mode.\n" + "> > + ? ? ? ?* 2. Do only a WFI as long as in SMP mode.\n" "> Does this correspond to the condition '(num_online_cpus() > 1)' in\n" "> the\n" "> test below? If so it this one triggering the low power mode for\n" "> cpu0?\n" "yes\n" ">\n" - "> > + \302\240 \302\240 \302\240 \302\240* 3. Continue to do only WFI till CPU1 hits OFF state.\n" - "> > + \302\240 \302\240 \302\240 \302\240* \302\240 \302\240This is necessary to honour hardware recommondation\n" - "> > + \302\240 \302\240 \302\240 \302\240* \302\240 \302\240of triggeing all the possible low power modes once\n" + "> > + ? ? ? ?* 3. Continue to do only WFI till CPU1 hits OFF state.\n" + "> > + ? ? ? ?* ? ?This is necessary to honour hardware recommondation\n" + "> > + ? ? ? ?* ? ?of triggeing all the possible low power modes once\n" "> CPU1 is\n" - "> > + \302\240 \302\240 \302\240 \302\240* \302\240 \302\240out of coherency and in OFF mode.\n" + "> > + ? ? ? ?* ? ?out of coherency and in OFF mode.\n" "> Does this correspond to the condition '(cpu1_state !=\n" "> PWRDM_POWER_OFF)' in the test below?\n" ">\n" "Yes\n" "\n" - "> > + \302\240 \302\240 \302\240 \302\240* Update dev->last_state so that governor stats reflects\n" + "> > + ? ? ? ?* Update dev->last_state so that governor stats reflects\n" "> right\n" - "> > + \302\240 \302\240 \302\240 \302\240* data.\n" - "> > + \302\240 \302\240 \302\240 \302\240*/\n" - "> > + \302\240 \302\240 \302\240 cpu1_state = pwrdm_read_pwrst(cpu1_pd);\n" - "> > + \302\240 \302\240 \302\240 if ((dev->cpu) || (num_online_cpus() > 1) ||\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 (cpu1_state != PWRDM_POWER_OFF)) {\n" + "> > + ? ? ? ?* data.\n" + "> > + ? ? ? ?*/\n" + "> > + ? ? ? cpu1_state = pwrdm_read_pwrst(cpu1_pd);\n" + "> > + ? ? ? if ((dev->cpu) || (num_online_cpus() > 1) ||\n" + "> > + ? ? ? ? ? ? ? ? ? ? ? (cpu1_state != PWRDM_POWER_OFF)) {\n" "> Are '||' correct here?\n" "Yes.\n" "\n" @@ -135,10 +131,6 @@ "And you got all of them correctly.\n" "\n" "Regards,\n" - "Santosh\n" - "--\n" - "To unsubscribe from this list: send the line \"unsubscribe linux-omap\" in\n" - "the body of a message to majordomo@vger.kernel.org\n" - More majordomo info at http://vger.kernel.org/majordomo-info.html + Santosh -d96bab8cd90accfbd102f72ca99e3caa48f0b01d8d5fa351328c14788e735858 +d561d4a2e643c932e77d70f256a9c4ae8abbb158fbd7b542a141d366627dacdc
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.