>
> Check wether serial can sleep is missing from
> omap3_enter_idle and it should be removed from omap3_can_sleep:
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
> b/arch/arm/mach-omap2/cpuidle34xx.c
> index a02da6d..16ff30b 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -431,7 +431,7 @@ static int omap3_enter_idle(struct
> cpuidle_device *dev,
>
> current_cx_state = *cx;
>
> - if (cx->type == OMAP3_STATE_C0) {
> + if (cx->type == OMAP3_STATE_C0 || !omap_serial_can_sleep()) {
> /* Do nothing for C0, not even a wfi */
> return 0;
> }
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c
> b/arch/arm/mach-omap2/pm34xx.c index f68fa0a..f9b3676 100644
> @@ -391,8 +391,6 @@ int omap3_can_sleep(void)
> return 0;
> if (atomic_read(&sleep_block) > 0)
> return 0;
> - if (!omap_serial_can_sleep())
> - return 0;
> return 1;
> }
>
> Doing this would make serial console to work faster.
Yes, I removed these in my patches and put in the changes suggested by Richard
in 8250.c
I thought the final conclusion of the discussion was that it was too expensive to the keep the
system in C0 all the time while UART inactivity runs, or did I miss something?
>
> _omap_sram_idle should be non-static:
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c
> b/arch/arm/mach-omap2/pm34xx.c index f68fa0a..133a666 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -60,7 +60,7 @@ u32 restore_pointer_address;
>
> static LIST_HEAD(pwrst_list);
>
> -static void (*_omap_sram_idle)(u32 *addr, int save_state);
> +void (*_omap_sram_idle)(u32 *addr, int save_state);
>
> static void (*saved_idle)(void);
I thought this is already part of the patches. It is now non-static.
>
> Serial and gpio clock disabling and gpio_prepare/resume can
> be removed from omap3_pm_idle because they are already done
> in omap_sram_idle. And if omap_serial_can_sleep is removed
> from omap3_can_sleep it should be added to omap3_pm_idle.
> Omap3_pm_idle can be also put behind #ifndef CONFIG_CPU_IDLE:
>
> +#ifndef CONFIG_CPU_IDLE
> static void omap3_pm_idle(void)
> {
> local_irq_disable();
> @@ -454,33 +455,16 @@ static void omap3_pm_idle(void)
> if (omap_irq_pending())
> goto out;
>
> - omap2_gpio_prepare_for_retention();
> -
> - if (clocks_off_while_idle) {
> - omap_serial_enable_clocks(0, 0);
> - omap_serial_enable_clocks(0, 1);
> - omap_serial_enable_clocks(0, 2);
> - /* XXX This is for gpio fclk hack. Will be removed as
> - * gpio driver * handles fcks correctly */
> - per_gpio_clk_disable();
> - }
> + if (!omap_serial_can_sleep())
> + goto out;
>
> omap_sram_idle();
>
> - if (clocks_off_while_idle) {
> - omap_serial_enable_clocks(1, 0);
> - omap_serial_enable_clocks(1, 1);
> - omap_serial_enable_clocks(1, 2);
> - /* XXX This is for gpio fclk hack. Will be removed as
> - * gpio driver * handles fcks correctly */
> - per_gpio_clk_enable();
> - }
> -
> - omap2_gpio_resume_after_retention();
> out:
> local_fiq_enable();
> local_irq_enable();
> }
> +#endif /* CONFIG_CPU_IDLE */
These are also done as part of the last patch in the series.
>
> I would like to see also some reformatting. E.g. patch 11
> contains lots of code which is not related to "CORE context
> save/restore". It might be also good idea to enable only
> non-off mode C states and have a pwrdms_setup function which
> is something like this:
>
> static int __init pwrdms_setup(struct powerdomain *pwrdm) {
> struct power_state *pwrst;
>
> if (!pwrdm->pwrsts)
> return 0;
>
> pwrst = kmalloc(sizeof(struct power_state), GFP_KERNEL);
> if (!pwrst)
> return -ENOMEM;
> pwrst->pwrdm = pwrdm;
> pwrst->next_state = PWRDM_POWER_RET;
> list_add(&pwrst->node, &pwrst_list);
>
> if (pwrdm_has_hdwr_sar(pwrdm))
> pwrdm_enable_hdwr_sar(pwrdm);
>
> #ifdef CONFIG_CPU_IDLE
> /* Let cpuidle do selection here */
> if (!strcmp(pwrst->pwrdm->name, "core_pwrdm") ||
> !strcmp(pwrst->pwrdm->name, "mpu_pwrdm") ||
> !strcmp(pwrst->pwrdm->name, "neon_pwrdm"))
> return set_pwrdm_state(pwrst->pwrdm, PWRDM_POWER_ON);
> else
> #endif
> return set_pwrdm_state(pwrst->pwrdm,
> pwrst->next_state); }
>
> This way cpuidle code would be in a state that it could be
> applied on linux-omap tree and it wouldn't break anything.
> Then have e.g. separate patch for testing off state. This
> patch could modify code to set next states to off and mark
> off mode C states as a valid.
Yes, will do that. Maybe the couple of comments above are also due to
the last patch doing a lot more than it actually should.
>
> >
> >> Hi,
> >>
> >> I am sending an updated patch set for CPUidle which includes all
> >> fixes/comments posted on the previous set by Jouni/Richard W/Peter
> >> and others.
> >>
> >> The Following are the fixes
> >> 1) Uart clock enable/disable moved out of the context save/restore
> >> patch
> >> 2) GPIO IRQENABLE save/restore fix from Richard
> >> 3) Fixes from Jouni which do the following
> >> 1. Add wkdep between neon and mpu
> >> 2. Add wkdep between per and core
> >> 3. Deny hwsup mode before writing next pwrst state
> >> 4. Make sure that order in idle loop is such that clocks are
> >> _really_
> >> enabled before accessing registers
> (serial & gpio).
> >> 4) Safe state idle fix from Richard
> >> 5) Uart smart-force fix from Richard
> >> 6) Toggle IO-PAD enable/disable in idle
> >>
> >> As earlier these patches apply on top of Jouni's
> workaround patch set
> >> ([PATCH 0/6] 34XX: PM: Workarounds to get omap3 to retention 4th.)
> >>
> >> The following is neccessay even with a minimal config to
> achieve OFF.
> >> $ echo 1 > /sys/power/sleep_while_idle $ echo 1 >
> >> /sys/power/clocks_off_while_idle
> >>
> >> regards,
> >> Rajendra
> >>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> linux-omap"
> > in the body of a message to majordomo@xxxxxxxxxxxxxxx More
> majordomo
> > info at http://vger.kernel.org/majordomo-info.html
> >
> >
>
> --
> Jouni Högander
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
