[PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Zhao Chenhui]

When entering deep sleep, the value in the registers I2CFDR and
I2CDFSRR are lost. This causes I2C access to fail after resuming.

Add suspend/resume routines to save/restore the registers
I2CFDR and I2CDFSRR.

Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/i2c/busses/i2c-mpc.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a8ebb84..f95a647 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -1,7 +1,9 @@
 /*
  * (C) Copyright 2003-2004
  * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
-
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
  * This is a combined i2c adapter and algorithm driver for the
  * MPC107/Tsi107 PowerPC northbridge and processors that include
  * the same I2C unit (8240, 8245, 85xx).
@@ -64,6 +66,9 @@ struct mpc_i2c {
 	struct i2c_adapter adap;
 	int irq;
 	u32 real_clk;
+#ifdef CONFIG_PM
+	u8 fdr, dfsrr;
+#endif
 };
 
 struct mpc_i2c_divider {
@@ -668,6 +673,30 @@ static int __devexit fsl_i2c_remove(struct platform_device *op)
 	return 0;
 };
 
+#ifdef CONFIG_PM
+static int mpc_i2c_suspend(struct device *dev)
+{
+	struct mpc_i2c *i2c = dev_get_drvdata(dev);
+
+	i2c->fdr = readb(i2c->base + MPC_I2C_FDR);
+	i2c->dfsrr = readb(i2c->base + MPC_I2C_DFSRR);
+
+	return 0;
+}
+
+static int mpc_i2c_resume(struct device *dev)
+{
+	struct mpc_i2c *i2c = dev_get_drvdata(dev);
+
+	writeb(i2c->fdr, i2c->base + MPC_I2C_FDR);
+	writeb(i2c->dfsrr, i2c->base + MPC_I2C_DFSRR);
+
+	return 0;
+}
+
+SIMPLE_DEV_PM_OPS(mpc_i2c_pm_ops, mpc_i2c_suspend, mpc_i2c_resume);
+#endif
+
 static struct mpc_i2c_data mpc_i2c_data_512x __devinitdata = {
 	.setup = mpc_i2c_setup_512x,
 };
@@ -712,6 +741,9 @@ static struct platform_driver mpc_i2c_driver = {
 		.owner = THIS_MODULE,
 		.name = DRV_NAME,
 		.of_match_table = mpc_i2c_of_match,
+#ifdef CONFIG_PM
+		.pm = &mpc_i2c_pm_ops,
+#endif
 	},
 };
 
-- 
1.6.4.1

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Shubhrajyoti Datta]

Hi Zhao,

On Wed, Feb 29, 2012 at 4:09 PM, Zhao Chenhui
<chenhui.zhao@freescale.com> wrote:
> When entering deep sleep, the value in the registers I2CFDR and
> I2CDFSRR are lost. This causes I2C access to fail after resuming.
>
> Add suspend/resume routines to save/restore the registers
> I2CFDR and I2CDFSRR.
>
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/i2c/busses/i2c-mpc.c |   34 +++++++++++++++++++++++++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index a8ebb84..f95a647 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -1,7 +1,9 @@
>  /*
>  * (C) Copyright 2003-2004
>  * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
> -
> + *
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *
>  * This is a combined i2c adapter and algorithm driver for the
>  * MPC107/Tsi107 PowerPC northbridge and processors that include
>  * the same I2C unit (8240, 8245, 85xx).
> @@ -64,6 +66,9 @@ struct mpc_i2c {
>        struct i2c_adapter adap;
>        int irq;
>        u32 real_clk;
> +#ifdef CONFIG_PM
> +       u8 fdr, dfsrr;
How about having  a structure here ?
> +#endif
>  };
>
>  struct mpc_i2c_divider {
> @@ -668,6 +673,30 @@ static int __devexit fsl_i2c_remove(struct platform_device *op)
>        return 0;
>  };
>
> +#ifdef CONFIG_PM
> +static int mpc_i2c_suspend(struct device *dev)
> +{
> +       struct mpc_i2c *i2c = dev_get_drvdata(dev);
> +
> +       i2c->fdr = readb(i2c->base + MPC_I2C_FDR);
> +       i2c->dfsrr = readb(i2c->base + MPC_I2C_DFSRR);
> +
> +       return 0;
> +}
> +
> +static int mpc_i2c_resume(struct device *dev)
> +{
> +       struct mpc_i2c *i2c = dev_get_drvdata(dev);
> +
> +       writeb(i2c->fdr, i2c->base + MPC_I2C_FDR);
> +       writeb(i2c->dfsrr, i2c->base + MPC_I2C_DFSRR);
> +
> +       return 0;
> +}
> +
> +SIMPLE_DEV_PM_OPS(mpc_i2c_pm_ops, mpc_i2c_suspend, mpc_i2c_resume);
> +#endif
> +
>  static struct mpc_i2c_data mpc_i2c_data_512x __devinitdata = {
>        .setup = mpc_i2c_setup_512x,
>  };
> @@ -712,6 +741,9 @@ static struct platform_driver mpc_i2c_driver = {
>                .owner = THIS_MODULE,
>                .name = DRV_NAME,
>                .of_match_table = mpc_i2c_of_match,
> +#ifdef CONFIG_PM
> +               .pm = &mpc_i2c_pm_ops,
> +#endif
>        },
>  };
>
> --
> 1.6.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Tabi Timur-B04825]

On Wed, Feb 29, 2012 at 7:02 AM, Shubhrajyoti Datta
<omaplinuxkernel@gmail.com> wrote:
>
>> +#ifdef CONFIG_PM
>> +       u8 fdr, dfsrr;

> How about having  a structure here ?

Why? He's just adding two variables in an existing structure.  Adding
a nested structure would unnecessarily complicate things.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

Hi,

On Wed, Feb 29, 2012 at 06:39:21PM +0800, Zhao Chenhui wrote:
> When entering deep sleep, the value in the registers I2CFDR and
> I2CDFSRR are lost. This causes I2C access to fail after resuming.
> 
> Add suspend/resume routines to save/restore the registers
> I2CFDR and I2CDFSRR.
> 
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/i2c/busses/i2c-mpc.c |   34 +++++++++++++++++++++++++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index a8ebb84..f95a647 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -1,7 +1,9 @@
>  /*
>   * (C) Copyright 2003-2004
>   * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
> -
> + *
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *

I'd think the change is too trivial to claim copyright on the file.

Thanks for the contribution,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Ben Dooks]

On Wed, Feb 29, 2012 at 07:45:23PM +0100, Wolfram Sang wrote:
> Hi,
> 
> On Wed, Feb 29, 2012 at 06:39:21PM +0800, Zhao Chenhui wrote:
> > When entering deep sleep, the value in the registers I2CFDR and
> > I2CDFSRR are lost. This causes I2C access to fail after resuming.
> > 
> > Add suspend/resume routines to save/restore the registers
> > I2CFDR and I2CDFSRR.
> > 
> > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> >  drivers/i2c/busses/i2c-mpc.c |   34 +++++++++++++++++++++++++++++++++-
> >  1 files changed, 33 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> > index a8ebb84..f95a647 100644
> > --- a/drivers/i2c/busses/i2c-mpc.c
> > +++ b/drivers/i2c/busses/i2c-mpc.c
> > @@ -1,7 +1,9 @@
> >  /*
> >   * (C) Copyright 2003-2004
> >   * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
> > -
> > + *
> > + * Copyright 2012 Freescale Semiconductor, Inc.
> > + *
> 
> I'd think the change is too trivial to claim copyright on the file.

I agree with that.

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

On Wed, Feb 29, 2012 at 07:45:23PM +0100, Wolfram Sang wrote:
> Hi,
> 
> On Wed, Feb 29, 2012 at 06:39:21PM +0800, Zhao Chenhui wrote:
> > When entering deep sleep, the value in the registers I2CFDR and
> > I2CDFSRR are lost. This causes I2C access to fail after resuming.
> > 
> > Add suspend/resume routines to save/restore the registers
> > I2CFDR and I2CDFSRR.
> > 
> > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> >  drivers/i2c/busses/i2c-mpc.c |   34 +++++++++++++++++++++++++++++++++-
> >  1 files changed, 33 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> > index a8ebb84..f95a647 100644
> > --- a/drivers/i2c/busses/i2c-mpc.c
> > +++ b/drivers/i2c/busses/i2c-mpc.c
> > @@ -1,7 +1,9 @@
> >  /*
> >   * (C) Copyright 2003-2004
> >   * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
> > -
> > + *
> > + * Copyright 2012 Freescale Semiconductor, Inc.
> > + *
> 
> I'd think the change is too trivial to claim copyright on the file.

Do you want to resend with the comments addressed?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Li Yang-R58472]

> On Wed, Feb 29, 2012 at 07:45:23PM +0100, Wolfram Sang wrote:
> > Hi,
> >
> > On Wed, Feb 29, 2012 at 06:39:21PM +0800, Zhao Chenhui wrote:
> > > When entering deep sleep, the value in the registers I2CFDR and
> > > I2CDFSRR are lost. This causes I2C access to fail after resuming.
> > >
> > > Add suspend/resume routines to save/restore the registers I2CFDR and
> > > I2CDFSRR.
> > >
> > > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > ---
> > >  drivers/i2c/busses/i2c-mpc.c |   34
> +++++++++++++++++++++++++++++++++-
> > >  1 files changed, 33 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-mpc.c
> > > b/drivers/i2c/busses/i2c-mpc.c index a8ebb84..f95a647 100644
> > > --- a/drivers/i2c/busses/i2c-mpc.c
> > > +++ b/drivers/i2c/busses/i2c-mpc.c
> > > @@ -1,7 +1,9 @@
> > >  /*
> > >   * (C) Copyright 2003-2004
> > >   * Humboldt Solutions Ltd, adrian@humboldt.co.uk.
> > > -
> > > + *
> > > + * Copyright 2012 Freescale Semiconductor, Inc.
> > > + *
> >
> > I'd think the change is too trivial to claim copyright on the file.
> 
> Do you want to resend with the comments addressed?

We have a company policy for adding copyright claims.  And this patch has met our criteria that requires adding copyright.

We certainly don't want the policy to be against the common practice of the community.  If it is we can consider changing our policy.  However I can't find a clear message stating the consensus of what the Linux kernel community think of adding copyright claims.  I even found some message that encourage adding copyright to maintain the GPL license.

- Leo

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi,

> We certainly don't want the policy to be against the common practice
> of the community.  If it is we can consider changing our policy.

Nice, thanks for being open to discussion!

> However I can't find a clear message stating the consensus of what the
> Linux kernel community think of adding copyright claims.  I even found

Well, what I know, the consensus is "common sense". What is obviously
not "clear" in a way you could write it down. At least, Ben agrees.

> some message that encourage adding copyright to maintain the GPL
> license.

I am interested. Do you have a link?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Li Yang]

On Wed, Apr 18, 2012 at 6:33 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Hi,
>
>> We certainly don't want the policy to be against the common practice
>> of the community.  If it is we can consider changing our policy.
>
> Nice, thanks for being open to discussion!
>
>> However I can't find a clear message stating the consensus of what the
>> Linux kernel community think of adding copyright claims.  I even found
>
> Well, what I know, the consensus is "common sense". What is obviously
> not "clear" in a way you could write it down. At least, Ben agrees.
>

We do honor your point and can be flexible on this.  I'm just not sure
if our policy need to be changed to align with the common sense of the
community.

>> some message that encourage adding copyright to maintain the GPL
>> license.
>
> I am interested. Do you have a link?

Well.  I can't find the exact message.  I remember it is from an email
discussion in which someone said by encouraging contributors to add
copyright claims to a file it would be clear that no one can change
the license of the file away from GPL.

- Leo

Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep

[Tabi Timur-B04825]

On Wed, Apr 18, 2012 at 9:44 AM, Li Yang <leoli@freescale.com> wrote:

> We do honor your point and can be flexible on this.  I'm just not sure
> if our policy need to be changed to align with the common sense of the
> community.

It does need to be changed, and we are currently working on changing it.

The patch should be resubmitted without the Copyright statement.

-- 
Timur Tabi
Linux kernel developer at Freescale