linux-kernel - Re: platform/i2c busses: pm runtime and system sleep

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]

Message-Id: <201012170109.43137.rjw@sisk.pl>
Date:	Fri, 17 Dec 2010 01:09:42 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Rabin Vincent <rabin@....in>
Cc:	stern@...land.harvard.edu, linux-pm@...ts.linux-foundation.org,
	linux-i2c@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: platform/i2c busses: pm runtime and system sleep

On Thursday, December 16, 2010, Rabin Vincent wrote:
> Hello,
> 
> There seem to be some differences between the generic ops and the i2c
> and platform busses' implementations of the interaction between runtime
> PM and system sleep:
> 
>   (1) The platform bus does not implement the
>       don't-call-pm->suspend()-if pm_runtime_suspended()-returns-true
>       functionality implemented by the generic ops and i2c.
> 
>   (2) Both I2C and platform do not set the device as active when a
>       pm->resume callback exists and it succeeds.
> 
>       This seems to have been done in i2c until recently, but has been
>       removed by 753419f59e ("i2c: Fix for suspend/resume issue").  It
>       seems to me that this removal is incorrect, and instead the real
>       problem with the implementation was that it set the device as
>       active even if no resume callback existed, whereas it should only
>       do so when it exists and returns zero, like the generic ops.
> 
> Are these divergences from the generic ops to be considered as bugs?

I think so.  I'm not sure about (1), because someone may already depend on
that behavior, but (2) looks like a bug to me.

> Atleast (2) will cause devices which use UNIVERSAL_DEV_PM_OPS to have
> incorrect runtime pm state after a resume from system sleep.
> 
> If so, before I send patches to fix them: can it be assumed that only
> drivers using dev_pm_ops (and not the legacy ops of these busses) will
> need the interactions between runtime PM and system sleep as done in the
> generic ops?

Yes, you can make this assumption safely.  The drivers that don't use
dev_pm_ops can't support runtime PM at all.

> This would mean that simple busses could simply use the
> generic ops like below instead of duplicating their behaviour:
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 6b4cc56..46117e0 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -212,7 +212,7 @@ static int i2c_device_pm_resume(struct device *dev)
>  	int ret;
> 
>  	if (pm)
> -		ret = pm->resume ? pm->resume(dev) : 0;
> +		ret = pm_generic_resume(dev);
>  	else
>  		ret = i2c_legacy_resume(dev);

Thanks,
Rafael
--
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/