linux-kernel - Re: [PATCH] pinctrl: tegra: add suspend-resume support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 28 Mar 2013 11:48:43 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Bibek Basu <bbasu@...dia.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel@...r.kernel.org,
	Pritesh Raithatha <praithatha@...dia.com>
Subject: Re: [PATCH] pinctrl: tegra: add suspend-resume support

On 03/28/2013 11:11 AM, Bibek Basu wrote:
> From: Pritesh Raithatha <praithatha@...dia.com>
> 
> This patch adds suspend and resume callbacks to the pinctrl-tegra driver.
> 
> Based on work by:
> Pritesh Raithatha <praithatha@...dia.com>

Those two lines are somewhat implied by the fact the commit's git author
field is Pritesh, and his s-o-b line is included below.

> Signed-off-by: Pritesh Raithatha <praithatha@...dia.com>
> Signed-off-by: Bibek Basu <bbasu@...dia.com>

> diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c

> +static struct tegra_pmx *pmx;

Isn't there any way to pass data into the suspend/resume functions so
that this global isn't needed?

Why can't we just use the device suspend/resume functions rather than
global (syscore) suspend/resume functions? Presumably this is to ensure
that all other drivers suspend first, then the pinctrl driver does, and
the reverse for resume. Can't we rely on deferred probe to ensure that
instead?

To make that work, we might need every affected driver to define a dummy
pinmux state in DT that references the pinctrl driver, to make sure they
all get probed after the pinctrl driver.

> +#ifdef CONFIG_PM_SLEEP

Perhaps we can remove the ifdefs, and always compile this code. The
compiler should remove any unused functions from the final linked image.

>  int tegra_pinctrl_probe(struct platform_device *pdev,
>  			const struct tegra_pinctrl_soc_data *soc_data)
> -{
> -	struct tegra_pmx *pmx;
> +	{

The indentation of the { is wrong there.

> +#ifdef CONFIG_PM_SLEEP

I think you can use if (IS_ENABLED(CONFIG_PM_SLEEP)) here instead of an
ifdef. This will allow the code to be compiled in all cases (to allow
better compile coverage), but it will be optimized out if
CONFIG_PM_SLEEP isn't enabled. Can you please make that change, and
verify that it works?

The same comment applies to the other ifdefs below.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/