Sekhar Nori <[email protected]> writes:

> The patch adds support for DaVinci cpu idle.
>
> Two idle states are defined:
> 1. Wait for interrupt
> 2. Wait for interrupt and DDR self-refresh (or power down)
>
> Some DaVinci SoCs support putting DDR in self-refresh (eg Dm644x, DM6467)
> while others support putting DDR in self-refresh and power down (eg DM35x,
> DA8xx).
>
> Putting DDR (or mDDR) in power down saves more power than self-refresh.
>
> The patch has been tested on DA850/OMAP-L138 EVM.
>
> Signed-off-by: Sekhar Nori <[email protected]>
> ---

[...]

> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> new file mode 100644
> index 0000000..4f94bf1
> --- /dev/null
> +++ b/arch/arm/mach-davinci/cpuidle.c
> @@ -0,0 +1,163 @@
> +/*
> + * CPU idle for DaVinci SoCs
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated. http://www.ti.com/
> + *
> + * Derived from Marvell Kirkwood CPU idle code.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/cpuidle.h>
> +#include <linux/io.h>
> +#include <linux/sched.h>
> +
> +#include <mach/system.h>
> +
> +#define DAVINCI_CPUIDLE_MAX_STATES   2
> +
> +static struct cpuidle_driver davinci_idle_driver = {
> +     .name   = "cpuidle-davinci",
> +     .owner  = THIS_MODULE,
> +};
> +
> +static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
> +static void __iomem *ddr2_reg_base;
> +static int ddr2_pdown;
> +
> +#define DDR2_SDRCR_OFFSET    0xc
> +#define DDR2_SRPD_SHIFT              23
> +#define DDR2_SRPD_BIT                BIT(DDR2_SRPD_SHIFT)
> +#define DDR2_LPMODEN_BIT     BIT(31)
> +
> +static void davinci_save_ddr_power(int enter)
> +{
> +     u32 val;
> +
> +     val = __raw_readl(ddr2_reg_base + DDR2_SDRCR_OFFSET);
> +
> +     if (enter)
> +             val |= DDR2_LPMODEN_BIT | (ddr2_pdown << DDR2_SRPD_SHIFT);
> +     else
> +             val &= ~(DDR2_SRPD_BIT | DDR2_LPMODEN_BIT);
> +
> +     __raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
> +}
> +
> +/* Actual code that puts the SoC in different idle states */
> +static int davinci_enter_idle(struct cpuidle_device *dev,
> +                                             struct cpuidle_state *state)
> +{
> +     struct timeval before, after;
> +     int idle_time;
> +
> +     local_irq_disable();
> +     do_gettimeofday(&before);
> +
> +     if (state == &dev->states[1])
> +             davinci_save_ddr_power(1);

Hmm, don't like this check.  It's error prone when additional states
are added.

How about using the 'driver_data' field of the 'struct cpuidle_state'
to point to a struct that some davinci-specific hooks.   For starters,
entry and exit hooks.  That way, this code would look something like
this (not tested or even compiled):

        struct davinci_ops *ops == state->driver_data;
        [...]

        if (ops && ops->enter)
           ops->enter()

> +     /* Wait for interrupt state */
> +     arch_idle();
> +     if (state == &dev->states[1])
> +             davinci_save_ddr_power(0);

And here,

        if (ops & ops->exit)
           ops->exit()
             
This way, state[0] would have NULL ops and state[1] would have enter
and exit hooks calling the ddr_power func.  This would also allow
states that only need either and enter or an exit hook.

> +
> +     do_gettimeofday(&after);
> +     local_irq_enable();
> +     idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
> +                     (after.tv_usec - before.tv_usec);
>
> +     return idle_time;
> +}
> +
> +static int __init davinci_cpuidle_probe(struct platform_device *pdev)
> +{
> +     int ret;
> +     struct cpuidle_device *device;
> +     int *pdata = pdev->dev.platform_data;
> +     struct resource *ddr2_regs;
> +     resource_size_t len;
> +
> +     device = &per_cpu(davinci_cpuidle_device, smp_processor_id());
> +
> +     if (!pdata) {
> +             dev_err(&pdev->dev, "cannot get platform data\n");
> +             return -ENOENT;
> +     }
> +
> +     ddr2_pdown = *pdata;

This isn't so readable or scalable.  Just create a struct and have
the ddr2_pdown as the only field.  Then it's easy to extend later.

> +     ddr2_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     if (!ddr2_regs) {
> +             dev_err(&pdev->dev, "cannot get DDR2 controller register base");
> +             return -ENODEV;
> +     }
> +
> +     len = resource_size(ddr2_regs);
> +
> +     ddr2_regs = request_mem_region(ddr2_regs->start, len, ddr2_regs->name);
> +     if (!ddr2_regs)
> +             return -EBUSY;
> +
> +     ddr2_reg_base = ioremap(ddr2_regs->start, len);
> +     if (!ddr2_reg_base) {
> +             ret = -ENOMEM;
> +             goto ioremap_fail;
> +     }
>
> +     ret = cpuidle_register_driver(&davinci_idle_driver);
> +     if (ret) {
> +             dev_err(&pdev->dev, "failed to register driver\n");
> +             goto driver_register_fail;
> +     }
> +
> +     /* Wait for interrupt state */
> +     device->states[0].enter = davinci_enter_idle;
> +     device->states[0].exit_latency = 1;
> +     device->states[0].target_residency = 10000;
> +     device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> +     strcpy(device->states[0].name, "WFI");
> +     strcpy(device->states[0].desc, "Wait for interrupt");
> +
> +     /* Wait for interrupt and DDR self refresh state */
> +     device->states[1].enter = davinci_enter_idle;
> +     device->states[1].exit_latency = 10;
> +     device->states[1].target_residency = 10000;
> +     device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> +     strcpy(device->states[1].name, "DDR SR");
> +     strcpy(device->states[1].desc, "WFI and DDR Self Refresh");
> +
> +     device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
> +
> +     ret = cpuidle_register_device(device);
> +     if (ret) {
> +             dev_err(&pdev->dev, "failed to register device\n");
> +             goto device_register_fail;
> +     }
> +
> +     return 0;
> +
> +device_register_fail:
> +     cpuidle_unregister_driver(&davinci_idle_driver);
> +driver_register_fail:
> +     iounmap(ddr2_reg_base);
> +ioremap_fail:
> +     release_mem_region(ddr2_regs->start, len);
> +     return ret;
> +}
> +
> +static struct platform_driver davinci_cpuidle_driver = {
> +     .driver = {
> +             .name   = "cpuidle-davinci",
> +             .owner  = THIS_MODULE,
> +     },
> +};
> +
> +static int __init davinci_cpuidle_init(void)
> +{
> +     return platform_driver_probe(&davinci_cpuidle_driver,
> +                                                     davinci_cpuidle_probe);

extra indentation

> +}
> +device_initcall(davinci_cpuidle_init);
> +

Kevin

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to