[PATCH 1/1] Blackfin I2C Driver: Functional power management support

[PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Bryan Wu]

From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

PM_SUSPEND_MEM: Blackfin does not maintain register state through
Hibernate. Save and restore peripheral base initialization during
PM transitions.

Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |   35 ++++++++++++++++++++++++-----------
 1 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 48d084b..3c855ff 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -49,6 +49,8 @@ struct bfin_twi_iface {
 	struct i2c_msg 		*pmsg;
 	int			msg_num;
 	int			cur_msg;
+	u16			saved_clkdiv;
+	u16			saved_control;
 	void __iomem		*regs_base;
 };
 
@@ -565,32 +567,43 @@ static u32 bfin_twi_functionality(struct i2c_adapter *adap)
 	       I2C_FUNC_I2C;
 }
 
-
 static struct i2c_algorithm bfin_twi_algorithm = {
 	.master_xfer   = bfin_twi_master_xfer,
 	.smbus_xfer    = bfin_twi_smbus_xfer,
 	.functionality = bfin_twi_functionality,
 };
 
-
-static int i2c_bfin_twi_suspend(struct platform_device *dev, pm_message_t state)
+static int i2c_bfin_twi_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct bfin_twi_iface *iface = platform_get_drvdata(dev);
+	struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
+
+	iface->saved_clkdiv = read_CLKDIV(iface);
+	iface->saved_control = read_CONTROL(iface);
+
+	free_irq(iface->irq, iface);
 
 	/* Disable TWI */
-	write_CONTROL(iface, read_CONTROL(iface) & ~TWI_ENA);
-	SSYNC();
+	write_CONTROL(iface, iface->saved_control & ~TWI_ENA);
 
 	return 0;
 }
 
-static int i2c_bfin_twi_resume(struct platform_device *dev)
+static int i2c_bfin_twi_resume(struct platform_device *pdev)
 {
-	struct bfin_twi_iface *iface = platform_get_drvdata(dev);
+	struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
 
-	/* Enable TWI */
-	write_CONTROL(iface, read_CONTROL(iface) | TWI_ENA);
-	SSYNC();
+	int rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
+		IRQF_DISABLED, pdev->name, iface);
+	if (rc) {
+		dev_err(&pdev->dev, "Can't get IRQ %d !\n", iface->irq);
+		return -ENODEV;
+	}
+
+	/* Resume TWI interface clock as specified */
+	write_CLKDIV(iface, iface->saved_clkdiv);
+
+	/* Resume TWI */
+	write_CONTROL(iface, iface->saved_control);
 
 	return 0;
 }
-- 
1.5.6


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Ben Dooks]

On Sun, Jul 27, 2008 at 02:41:54PM +0800, Bryan Wu wrote:
> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> 
> PM_SUSPEND_MEM: Blackfin does not maintain register state through
> Hibernate. Save and restore peripheral base initialization during
> PM transitions.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-bfin-twi.c |   35 ++++++++++++++++++++++++-----------
>  1 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
> index 48d084b..3c855ff 100644
> --- a/drivers/i2c/busses/i2c-bfin-twi.c
> +++ b/drivers/i2c/busses/i2c-bfin-twi.c
> @@ -49,6 +49,8 @@ struct bfin_twi_iface {
>  	struct i2c_msg 		*pmsg;
>  	int			msg_num;
>  	int			cur_msg;
> +	u16			saved_clkdiv;
> +	u16			saved_control;
>  	void __iomem		*regs_base;
>  };
>  
> @@ -565,32 +567,43 @@ static u32 bfin_twi_functionality(struct i2c_adapter *adap)
>  	       I2C_FUNC_I2C;
>  }
>  
> -
>  static struct i2c_algorithm bfin_twi_algorithm = {
>  	.master_xfer   = bfin_twi_master_xfer,
>  	.smbus_xfer    = bfin_twi_smbus_xfer,
>  	.functionality = bfin_twi_functionality,
>  };
>  
> -
> -static int i2c_bfin_twi_suspend(struct platform_device *dev, pm_message_t state)
> +static int i2c_bfin_twi_suspend(struct platform_device *pdev, pm_message_t state)
>  {
> -	struct bfin_twi_iface *iface = platform_get_drvdata(dev);
> +	struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
> +
> +	iface->saved_clkdiv = read_CLKDIV(iface);
> +	iface->saved_control = read_CONTROL(iface);
> +
> +	free_irq(iface->irq, iface);
>  
>  	/* Disable TWI */
> -	write_CONTROL(iface, read_CONTROL(iface) & ~TWI_ENA);
> -	SSYNC();
> +	write_CONTROL(iface, iface->saved_control & ~TWI_ENA);

I assume removing the SSYNC() call is valid?
  
>  	return 0;
>  }
>  
> -static int i2c_bfin_twi_resume(struct platform_device *dev)
> +static int i2c_bfin_twi_resume(struct platform_device *pdev)
>  {
> -	struct bfin_twi_iface *iface = platform_get_drvdata(dev);
> +	struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
>  
> -	/* Enable TWI */
> -	write_CONTROL(iface, read_CONTROL(iface) | TWI_ENA);
> -	SSYNC();
> +	int rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
> +		IRQF_DISABLED, pdev->name, iface);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Can't get IRQ %d !\n", iface->irq);
> +		return -ENODEV;
> +	}
> +
> +	/* Resume TWI interface clock as specified */
> +	write_CLKDIV(iface, iface->saved_clkdiv);
> +
> +	/* Resume TWI */
> +	write_CONTROL(iface, iface->saved_control);
>  
>  	return 0;
>  }
> -- 
> 1.5.6

if removing the SSYNC() call is ok, then I'll queue this into
my tree which I will be requesting a pull asap.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Bryan Wu]

On Mon, Jul 28, 2008 at 5:23 PM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> On Sun, Jul 27, 2008 at 02:41:54PM +0800, Bryan Wu wrote:
>> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>>
>> PM_SUSPEND_MEM: Blackfin does not maintain register state through
>> Hibernate. Save and restore peripheral base initialization during
>> PM transitions.
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/i2c/busses/i2c-bfin-twi.c |   35 ++++++++++++++++++++++++-----------
>>  1 files changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
>> index 48d084b..3c855ff 100644
>> --- a/drivers/i2c/busses/i2c-bfin-twi.c
>> +++ b/drivers/i2c/busses/i2c-bfin-twi.c
>> @@ -49,6 +49,8 @@ struct bfin_twi_iface {
>>       struct i2c_msg          *pmsg;
>>       int                     msg_num;
>>       int                     cur_msg;
>> +     u16                     saved_clkdiv;
>> +     u16                     saved_control;
>>       void __iomem            *regs_base;
>>  };
>>
>> @@ -565,32 +567,43 @@ static u32 bfin_twi_functionality(struct i2c_adapter *adap)
>>              I2C_FUNC_I2C;
>>  }
>>
>> -
>>  static struct i2c_algorithm bfin_twi_algorithm = {
>>       .master_xfer   = bfin_twi_master_xfer,
>>       .smbus_xfer    = bfin_twi_smbus_xfer,
>>       .functionality = bfin_twi_functionality,
>>  };
>>
>> -
>> -static int i2c_bfin_twi_suspend(struct platform_device *dev, pm_message_t state)
>> +static int i2c_bfin_twi_suspend(struct platform_device *pdev, pm_message_t state)
>>  {
>> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
>> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
>> +
>> +     iface->saved_clkdiv = read_CLKDIV(iface);
>> +     iface->saved_control = read_CONTROL(iface);
>> +
>> +     free_irq(iface->irq, iface);
>>
>>       /* Disable TWI */
>> -     write_CONTROL(iface, read_CONTROL(iface) & ~TWI_ENA);
>> -     SSYNC();
>> +     write_CONTROL(iface, iface->saved_control & ~TWI_ENA);
>
> I assume removing the SSYNC() call is valid?
>

Yes, I think it is OK for that. We tested on our platform.
Actually, keeping the SSYNC() is not a big deal for this driver.
How do you think Michael?

>>       return 0;
>>  }
>>
>> -static int i2c_bfin_twi_resume(struct platform_device *dev)
>> +static int i2c_bfin_twi_resume(struct platform_device *pdev)
>>  {
>> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
>> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
>>
>> -     /* Enable TWI */
>> -     write_CONTROL(iface, read_CONTROL(iface) | TWI_ENA);
>> -     SSYNC();
>> +     int rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
>> +             IRQF_DISABLED, pdev->name, iface);
>> +     if (rc) {
>> +             dev_err(&pdev->dev, "Can't get IRQ %d !\n", iface->irq);
>> +             return -ENODEV;
>> +     }
>> +
>> +     /* Resume TWI interface clock as specified */
>> +     write_CLKDIV(iface, iface->saved_clkdiv);
>> +
>> +     /* Resume TWI */
>> +     write_CONTROL(iface, iface->saved_control);
>>
>>       return 0;
>>  }
>> --
>> 1.5.6
>
> if removing the SSYNC() call is ok, then I'll queue this into
> my tree which I will be requesting a pull asap.
>

Thanks lot, I'd like to see that.
-Bryan

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Ben Dooks]

On Mon, Jul 28, 2008 at 06:07:28PM +0800, Bryan Wu wrote:
> On Mon, Jul 28, 2008 at 5:23 PM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> > On Sun, Jul 27, 2008 at 02:41:54PM +0800, Bryan Wu wrote:
> >> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> >>
> >> PM_SUSPEND_MEM: Blackfin does not maintain register state through
> >> Hibernate. Save and restore peripheral base initialization during
> >> PM transitions.
> >>
> >> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> >> Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> ---
> >>  drivers/i2c/busses/i2c-bfin-twi.c |   35 ++++++++++++++++++++++++-----------
> >>  1 files changed, 24 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
> >> index 48d084b..3c855ff 100644
> >> --- a/drivers/i2c/busses/i2c-bfin-twi.c
> >> +++ b/drivers/i2c/busses/i2c-bfin-twi.c
> >> @@ -49,6 +49,8 @@ struct bfin_twi_iface {
> >>       struct i2c_msg          *pmsg;
> >>       int                     msg_num;
> >>       int                     cur_msg;
> >> +     u16                     saved_clkdiv;
> >> +     u16                     saved_control;
> >>       void __iomem            *regs_base;
> >>  };
> >>
> >> @@ -565,32 +567,43 @@ static u32 bfin_twi_functionality(struct i2c_adapter *adap)
> >>              I2C_FUNC_I2C;
> >>  }
> >>
> >> -
> >>  static struct i2c_algorithm bfin_twi_algorithm = {
> >>       .master_xfer   = bfin_twi_master_xfer,
> >>       .smbus_xfer    = bfin_twi_smbus_xfer,
> >>       .functionality = bfin_twi_functionality,
> >>  };
> >>
> >> -
> >> -static int i2c_bfin_twi_suspend(struct platform_device *dev, pm_message_t state)
> >> +static int i2c_bfin_twi_suspend(struct platform_device *pdev, pm_message_t state)
> >>  {
> >> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
> >> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
> >> +
> >> +     iface->saved_clkdiv = read_CLKDIV(iface);
> >> +     iface->saved_control = read_CONTROL(iface);
> >> +
> >> +     free_irq(iface->irq, iface);
> >>
> >>       /* Disable TWI */
> >> -     write_CONTROL(iface, read_CONTROL(iface) & ~TWI_ENA);
> >> -     SSYNC();
> >> +     write_CONTROL(iface, iface->saved_control & ~TWI_ENA);
> >
> > I assume removing the SSYNC() call is valid?
> >
> 
> Yes, I think it is OK for that. We tested on our platform.
> Actually, keeping the SSYNC() is not a big deal for this driver.
> How do you think Michael?
> 
> >>       return 0;
> >>  }
> >>
> >> -static int i2c_bfin_twi_resume(struct platform_device *dev)
> >> +static int i2c_bfin_twi_resume(struct platform_device *pdev)
> >>  {
> >> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
> >> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
> >>
> >> -     /* Enable TWI */
> >> -     write_CONTROL(iface, read_CONTROL(iface) | TWI_ENA);
> >> -     SSYNC();
> >> +     int rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
> >> +             IRQF_DISABLED, pdev->name, iface);
> >> +     if (rc) {
> >> +             dev_err(&pdev->dev, "Can't get IRQ %d !\n", iface->irq);
> >> +             return -ENODEV;
> >> +     }
> >> +
> >> +     /* Resume TWI interface clock as specified */
> >> +     write_CLKDIV(iface, iface->saved_clkdiv);
> >> +
> >> +     /* Resume TWI */
> >> +     write_CONTROL(iface, iface->saved_control);
> >>
> >>       return 0;
> >>  }
> >> --
> >> 1.5.6
> >
> > if removing the SSYNC() call is ok, then I'll queue this into
> > my tree which I will be requesting a pull asap.

ok, this is in my queue, unless anyone objects then it'll be pushed
out later today.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Hennerich, Michael]

>From: Ben Dooks [mailto:ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org]
>
>On Mon, Jul 28, 2008 at 06:07:28PM +0800, Bryan Wu wrote:
>> On Mon, Jul 28, 2008 at 5:23 PM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
wrote:
>> > On Sun, Jul 27, 2008 at 02:41:54PM +0800, Bryan Wu wrote:
>> >> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>> >>
>> >> PM_SUSPEND_MEM: Blackfin does not maintain register state through
>> >> Hibernate. Save and restore peripheral base initialization during
>> >> PM transitions.
>> >>
>> >> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>> >> Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> ---
>> >>  drivers/i2c/busses/i2c-bfin-twi.c |   35
++++++++++++++++++++++++----
>-------
>> >>  1 files changed, 24 insertions(+), 11 deletions(-)
>> >>
>> >> diff --git a/drivers/i2c/busses/i2c-bfin-twi.c
>b/drivers/i2c/busses/i2c-bfin-twi.c
>> >> index 48d084b..3c855ff 100644
>> >> --- a/drivers/i2c/busses/i2c-bfin-twi.c
>> >> +++ b/drivers/i2c/busses/i2c-bfin-twi.c
>> >> @@ -49,6 +49,8 @@ struct bfin_twi_iface {
>> >>       struct i2c_msg          *pmsg;
>> >>       int                     msg_num;
>> >>       int                     cur_msg;
>> >> +     u16                     saved_clkdiv;
>> >> +     u16                     saved_control;
>> >>       void __iomem            *regs_base;
>> >>  };
>> >>
>> >> @@ -565,32 +567,43 @@ static u32 bfin_twi_functionality(struct
>i2c_adapter *adap)
>> >>              I2C_FUNC_I2C;
>> >>  }
>> >>
>> >> -
>> >>  static struct i2c_algorithm bfin_twi_algorithm = {
>> >>       .master_xfer   = bfin_twi_master_xfer,
>> >>       .smbus_xfer    = bfin_twi_smbus_xfer,
>> >>       .functionality = bfin_twi_functionality,
>> >>  };
>> >>
>> >> -
>> >> -static int i2c_bfin_twi_suspend(struct platform_device *dev,
>pm_message_t state)
>> >> +static int i2c_bfin_twi_suspend(struct platform_device *pdev,
>pm_message_t state)
>> >>  {
>> >> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
>> >> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
>> >> +
>> >> +     iface->saved_clkdiv = read_CLKDIV(iface);
>> >> +     iface->saved_control = read_CONTROL(iface);
>> >> +
>> >> +     free_irq(iface->irq, iface);
>> >>
>> >>       /* Disable TWI */
>> >> -     write_CONTROL(iface, read_CONTROL(iface) & ~TWI_ENA);
>> >> -     SSYNC();
>> >> +     write_CONTROL(iface, iface->saved_control & ~TWI_ENA);
>> >
>> > I assume removing the SSYNC() call is valid?
>> >
>>
>> Yes, I think it is OK for that. We tested on our platform.
>> Actually, keeping the SSYNC() is not a big deal for this driver.
>> How do you think Michael?
>>
>> >>       return 0;
>> >>  }
>> >>
>> >> -static int i2c_bfin_twi_resume(struct platform_device *dev)
>> >> +static int i2c_bfin_twi_resume(struct platform_device *pdev)
>> >>  {
>> >> -     struct bfin_twi_iface *iface = platform_get_drvdata(dev);
>> >> +     struct bfin_twi_iface *iface = platform_get_drvdata(pdev);
>> >>
>> >> -     /* Enable TWI */
>> >> -     write_CONTROL(iface, read_CONTROL(iface) | TWI_ENA);
>> >> -     SSYNC();
>> >> +     int rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
>> >> +             IRQF_DISABLED, pdev->name, iface);
>> >> +     if (rc) {
>> >> +             dev_err(&pdev->dev, "Can't get IRQ %d !\n",
iface->irq);
>> >> +             return -ENODEV;
>> >> +     }
>> >> +
>> >> +     /* Resume TWI interface clock as specified */
>> >> +     write_CLKDIV(iface, iface->saved_clkdiv);
>> >> +
>> >> +     /* Resume TWI */
>> >> +     write_CONTROL(iface, iface->saved_control);
>> >>
>> >>       return 0;
>> >>  }
>> >> --
>> >> 1.5.6
>> >
>> > if removing the SSYNC() call is ok, then I'll queue this into
>> > my tree which I will be requesting a pull asap.
>
>ok, this is in my queue, unless anyone objects then it'll be pushed
>out later today.

Removing this SSYNC is absolutely safe. There are potentially many other
ones that can be removed too.

Michael

>
>--
>Ben
>
>Q:      What's a light-year?
>A:      One-third less calories than a regular year.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/1] Blackfin I2C Driver: Functional power management support

[Ben Dooks]

On Mon, Jul 28, 2008 at 01:16:44PM +0100, Hennerich, Michael wrote:
> 

> >> > if removing the SSYNC() call is ok, then I'll queue this into
> >> > my tree which I will be requesting a pull asap.
> >
> >ok, this is in my queue, unless anyone objects then it'll be pushed
> >out later today.
> 
> Removing this SSYNC is absolutely safe. There are potentially many other
> ones that can be removed too.

Ok, next time a note to this effect in the description
would be useful to inform everyone that you've not just
accidentally removed something the driver might rely on
having.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c