* [PATCH] driver i2c-core: i2c bus should support PM entries in struct dev_pm_ops.
@ 2009-12-02 9:08 sonic zhang
[not found] ` <1259744920.25505.2.camel-x+10Id33CNwLe6dzpsQaEw@public.gmane.org>
2009-12-13 21:12 ` Jean Delvare
0 siblings, 2 replies; 4+ messages in thread
From: sonic zhang @ 2009-12-02 9:08 UTC (permalink / raw)
To: khali-PUYAD+kWke1g9hUCZPvPmw
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Linux Kernel
Struct dev_pm_ops is not configured in current i2c bus type. i2c drivers
only depends on suspend/resume entries in struct dev_pm_ops are not
informed of PM suspend and resume events by i2c framework.
Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/i2c-core.c | 39 +++++++++++++++++++++++++++++++++++++++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 2965043..9713c0d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -155,6 +155,39 @@ static void i2c_device_shutdown(struct device *dev)
driver->shutdown(client);
}
+#ifdef CONFIG_SUSPEND
+static int i2c_device_pm_suspend(struct device *dev)
+{
+ struct i2c_driver *driver;
+ struct dev_pm_ops *pm;
+
+ if (!dev->driver)
+ return 0;
+ driver = to_i2c_driver(dev->driver);
+ pm = driver->driver.pm;
+ if (!pm || !pm->suspend)
+ return 0;
+ return pm->suspend(dev);
+}
+
+static int i2c_device_pm_resume(struct device *dev)
+{
+ struct i2c_driver *driver;
+ struct dev_pm_ops *pm;
+
+ if (!dev->driver)
+ return 0;
+ driver = to_i2c_driver(dev->driver);
+ pm = driver->driver.pm;
+ if (!pm || !pm->resume)
+ return 0;
+ return pm->resume(dev);
+}
+#else
+# define i2c_device_pm_suspend NULL
+# define i2c_device_pm_resume NULL
+#endif
+
static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -219,6 +252,11 @@ static const struct attribute_group *i2c_dev_attr_groups[] = {
NULL
};
+static struct dev_pm_ops i2c_device_pm_ops = {
+ .suspend = i2c_device_pm_suspend,
+ .resume = i2c_device_pm_resume,
+};
+
struct bus_type i2c_bus_type = {
.name = "i2c",
.match = i2c_device_match,
@@ -227,6 +265,7 @@ struct bus_type i2c_bus_type = {
.shutdown = i2c_device_shutdown,
.suspend = i2c_device_suspend,
.resume = i2c_device_resume,
+ .pm = &i2c_device_pm_ops,
};
EXPORT_SYMBOL_GPL(i2c_bus_type);
--
1.6.0
^ permalink raw reply related [flat|nested] 4+ messages in thread[parent not found: <1259744920.25505.2.camel-x+10Id33CNwLe6dzpsQaEw@public.gmane.org>]
* Re: [PATCH] driver i2c-core: i2c bus should support PM entries in struct dev_pm_ops.
[not found] ` <1259744920.25505.2.camel-x+10Id33CNwLe6dzpsQaEw@public.gmane.org>
@ 2009-12-04 9:24 ` Sonic Zhang
0 siblings, 0 replies; 4+ messages in thread
From: Sonic Zhang @ 2009-12-04 9:24 UTC (permalink / raw)
To: khali-PUYAD+kWke1g9hUCZPvPmw
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Linux Kernel
Any comments?
Thanks.
Sonic
On Wed, Dec 2, 2009 at 5:08 PM, sonic zhang <sonic.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Struct dev_pm_ops is not configured in current i2c bus type. i2c drivers
> only depends on suspend/resume entries in struct dev_pm_ops are not
> informed of PM suspend and resume events by i2c framework.
>
> Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/i2c/i2c-core.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 2965043..9713c0d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -155,6 +155,39 @@ static void i2c_device_shutdown(struct device *dev)
> driver->shutdown(client);
> }
>
> +#ifdef CONFIG_SUSPEND
> +static int i2c_device_pm_suspend(struct device *dev)
> +{
> + struct i2c_driver *driver;
> + struct dev_pm_ops *pm;
> +
> + if (!dev->driver)
> + return 0;
> + driver = to_i2c_driver(dev->driver);
> + pm = driver->driver.pm;
> + if (!pm || !pm->suspend)
> + return 0;
> + return pm->suspend(dev);
> +}
> +
> +static int i2c_device_pm_resume(struct device *dev)
> +{
> + struct i2c_driver *driver;
> + struct dev_pm_ops *pm;
> +
> + if (!dev->driver)
> + return 0;
> + driver = to_i2c_driver(dev->driver);
> + pm = driver->driver.pm;
> + if (!pm || !pm->resume)
> + return 0;
> + return pm->resume(dev);
> +}
> +#else
> +# define i2c_device_pm_suspend NULL
> +# define i2c_device_pm_resume NULL
> +#endif
> +
> static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> @@ -219,6 +252,11 @@ static const struct attribute_group *i2c_dev_attr_groups[] = {
> NULL
> };
>
> +static struct dev_pm_ops i2c_device_pm_ops = {
> + .suspend = i2c_device_pm_suspend,
> + .resume = i2c_device_pm_resume,
> +};
> +
> struct bus_type i2c_bus_type = {
> .name = "i2c",
> .match = i2c_device_match,
> @@ -227,6 +265,7 @@ struct bus_type i2c_bus_type = {
> .shutdown = i2c_device_shutdown,
> .suspend = i2c_device_suspend,
> .resume = i2c_device_resume,
> + .pm = &i2c_device_pm_ops,
> };
> EXPORT_SYMBOL_GPL(i2c_bus_type);
>
> --
> 1.6.0
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] driver i2c-core: i2c bus should support PM entries in struct dev_pm_ops.
2009-12-02 9:08 [PATCH] driver i2c-core: i2c bus should support PM entries in struct dev_pm_ops sonic zhang
[not found] ` <1259744920.25505.2.camel-x+10Id33CNwLe6dzpsQaEw@public.gmane.org>
@ 2009-12-13 21:12 ` Jean Delvare
[not found] ` <20091213221222.724bef54-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
1 sibling, 1 reply; 4+ messages in thread
From: Jean Delvare @ 2009-12-13 21:12 UTC (permalink / raw)
To: sonic zhang; +Cc: linux-i2c, Linux Kernel
Hi Sonic,
On Wed, 2 Dec 2009 17:08:40 +0800, sonic zhang wrote:
> Struct dev_pm_ops is not configured in current i2c bus type. i2c drivers
> only depends on suspend/resume entries in struct dev_pm_ops are not
> informed of PM suspend and resume events by i2c framework.
Good point. I was wondering some times ago what was the status of
pm_ops. I seem to understand this is the future of power management,
and "direct" .suspend and .resume callbacks will stop being supported
at some point in the future? If so, what is the migration plan? I don't
really want to support both forever.
Your patch introduces the following warnings:
drivers/i2c/i2c-core.c: In function ‘i2c_device_pm_suspend’:
drivers/i2c/i2c-core.c:167: warning: assignment discards qualifiers from pointer target type
drivers/i2c/i2c-core.c: In function ‘i2c_device_pm_resume’:
drivers/i2c/i2c-core.c:181: warning: assignment discards qualifiers from pointer target type
You will obviously have to fix them before I can accept your patch.
Missing const...
>
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
> ---
> drivers/i2c/i2c-core.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 2965043..9713c0d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -155,6 +155,39 @@ static void i2c_device_shutdown(struct device *dev)
> driver->shutdown(client);
> }
>
> +#ifdef CONFIG_SUSPEND
> +static int i2c_device_pm_suspend(struct device *dev)
> +{
> + struct i2c_driver *driver;
> + struct dev_pm_ops *pm;
> +
> + if (!dev->driver)
> + return 0;
> + driver = to_i2c_driver(dev->driver);
> + pm = driver->driver.pm;
This makes little sense, sorry. Why go from device_driver to i2c_driver
and then again to device_driver? The following is equivalent and more
efficient:
pm = dev->driver->pm;
> + if (!pm || !pm->suspend)
> + return 0;
> + return pm->suspend(dev);
> +}
> +
> +static int i2c_device_pm_resume(struct device *dev)
> +{
> + struct i2c_driver *driver;
> + struct dev_pm_ops *pm;
> +
> + if (!dev->driver)
> + return 0;
> + driver = to_i2c_driver(dev->driver);
> + pm = driver->driver.pm;
Same here.
> + if (!pm || !pm->resume)
> + return 0;
> + return pm->resume(dev);
> +}
> +#else
> +# define i2c_device_pm_suspend NULL
> +# define i2c_device_pm_resume NULL
No space between "#" and "define", please.
> +#endif
> +
> static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> @@ -219,6 +252,11 @@ static const struct attribute_group *i2c_dev_attr_groups[] = {
> NULL
> };
>
> +static struct dev_pm_ops i2c_device_pm_ops = {
Could be const.
> + .suspend = i2c_device_pm_suspend,
> + .resume = i2c_device_pm_resume,
> +};
> +
> struct bus_type i2c_bus_type = {
> .name = "i2c",
> .match = i2c_device_match,
> @@ -227,6 +265,7 @@ struct bus_type i2c_bus_type = {
> .shutdown = i2c_device_shutdown,
> .suspend = i2c_device_suspend,
> .resume = i2c_device_resume,
> + .pm = &i2c_device_pm_ops,
> };
> EXPORT_SYMBOL_GPL(i2c_bus_type);
>
I've made all the changes above myself, it build OK but I can't test.
Modified patch is here:
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-core-i2c-bus-should-support-pm-entries-in-struct-dev_pm_ops.patch
Please test and report if anything's wrong.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-12-15 5:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-02 9:08 [PATCH] driver i2c-core: i2c bus should support PM entries in struct dev_pm_ops sonic zhang
[not found] ` <1259744920.25505.2.camel-x+10Id33CNwLe6dzpsQaEw@public.gmane.org>
2009-12-04 9:24 ` Sonic Zhang
2009-12-13 21:12 ` Jean Delvare
[not found] ` <20091213221222.724bef54-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2009-12-15 5:38 ` Sonic Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).