Tarun Kanti DebBarma <tarun.kanti@xxxxxx> writes:
> From: Charulatha V <charu@xxxxxx>
>
> Call runtime pm APIs pm_runtime_get_sync() and pm_runtime_put_sync()
> for enabling/disabling clocks appropriately. Remove syscore_ops and
> instead use dev_pm_ops now.
>
> Signed-off-by: Charulatha V <charu@xxxxxx>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@xxxxxx>
Approach is fine. some minor comments below..
> ---
> drivers/gpio/gpio-omap.c | 100 ++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 80 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index ea984fd..f5bb469 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -80,6 +80,8 @@ struct gpio_bank {
> struct omap_gpio_reg_offs *regs;
> };
>
> +static void omap_gpio_mod_init(struct gpio_bank *bank);
> +
> #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
> #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
> #define GPIO_MOD_CTRL_BIT BIT(0)
> @@ -492,6 +494,22 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
> struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
> unsigned long flags;
>
> + /*
> + * If this is the first gpio_request for the bank,
> + * enable the bank module.
> + */
> + if (!bank->mod_usage) {
> + if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0)) {
> + dev_err(bank->dev, "%s: GPIO bank %d "
> + "pm_runtime_get_sync failed\n",
> + __func__, bank->id);
> + return -EINVAL;
> + }
> +
> + /* Initialize the gpio bank registers to init time value */
> + omap_gpio_mod_init(bank);
> + }
> +
> spin_lock_irqsave(&bank->lock, flags);
>
> /* Set trigger to none. You need to enable the desired trigger with
> @@ -548,6 +566,18 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
>
> _reset_gpio(bank, bank->chip.base + offset);
> spin_unlock_irqrestore(&bank->lock, flags);
> +
> + /*
> + * If this is the last gpio to be freed in the bank,
> + * disable the bank module.
> + */
> + if (!bank->mod_usage) {
> + if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0)) {
> + dev_err(bank->dev, "%s: GPIO bank %d "
> + "pm_runtime_put_sync failed\n",
> + __func__, bank->id);
> + }
> + }
> }
>
> /*
> @@ -572,6 +602,9 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> chained_irq_enter(chip, desc);
>
> bank = irq_get_handler_data(irq);
> +
> + pm_runtime_get_sync(bank->dev);
> +
> isr_reg = bank->base + bank->regs->irqstatus;
>
> if (WARN_ON(!isr_reg))
> @@ -640,6 +673,8 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> exit:
> if (!unmasked)
> chained_irq_exit(chip, desc);
> +
> + pm_runtime_put_sync(bank->dev);
> }
>
> static void gpio_irq_shutdown(struct irq_data *d)
> @@ -1120,12 +1155,25 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
> }
>
> pm_runtime_enable(bank->dev);
> - pm_runtime_get_sync(bank->dev);
> + pm_runtime_irq_safe(bank->dev);
> + if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0)) {
> + dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_get_sync "
> + "failed\n", __func__, bank->id);
> + iounmap(bank->base);
> + return -EINVAL;
> + }
>
> omap_gpio_mod_init(bank);
> omap_gpio_chip_init(bank);
> omap_gpio_show_rev(bank);
>
> + if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0)) {
> + dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_put_sync "
> + "failed\n", __func__, bank->id);
> + iounmap(bank->base);
> + return -EINVAL;
> + }
> +
> list_add_tail(&bank->node, &omap_gpio_list);
>
> return ret;
> @@ -1136,7 +1184,7 @@ err_exit:
> return ret;
> }
>
> -static int omap_gpio_suspend(void)
> +static int omap_gpio_suspend(struct device *dev)
> {
> struct gpio_bank *bank;
>
> @@ -1163,7 +1211,7 @@ static int omap_gpio_suspend(void)
> return 0;
> }
>
> -static void omap_gpio_resume(void)
> +static int omap_gpio_resume(struct device *dev)
> {
> struct gpio_bank *bank;
>
> @@ -1173,7 +1221,7 @@ static void omap_gpio_resume(void)
> unsigned long flags;
>
> if (!bank->suspend_support)
> - return;
> + return 0;
>
> wake_clear = bank->base + bank->regs->wkup_clear;
> wake_set = bank->base + bank->regs->wkup_set;
> @@ -1183,12 +1231,18 @@ static void omap_gpio_resume(void)
> __raw_writel(bank->saved_wakeup, wake_set);
> spin_unlock_irqrestore(&bank->lock, flags);
> }
> + return 0;
> }
>
> -static struct syscore_ops omap_gpio_syscore_ops = {
> - .suspend = omap_gpio_suspend,
> - .resume = omap_gpio_resume,
> -};
> +static int omap_gpio_pm_runtime_suspend(struct device *dev)
Please drop the 'pm_' in these function names.
> +{
> + return 0;
> +}
> +
> +static int omap_gpio_pm_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
Actually, empty functions should not even be added. Just add them in
the patch that uses them.
Kevin
--
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
