[PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[Abhilash Kesavan]

The I2C driver makes a gpio_request during initialization. This request
happens again on resume and fails due to the earlier successful request.
Modify the suspend code to free the earlier requested gpios.

Errors on resume without this:
[   16.020000] s3c-i2c s3c2440-i2c.0: gpio [42] request failed
[   16.020000] s3c-i2c s3c2440-i2c.1: gpio [44] request failed
[   16.020000] s3c-i2c s3c2440-i2c.2: gpio [6] request failed

Signed-off-by: Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 3e0335f..9bec49e 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -806,6 +806,7 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 			dev_err(i2c->dev, "invalid gpio[%d]: %d\n", idx, gpio);
 			goto free_gpio;
 		}
+		i2c->gpios[idx] = gpio;
 
 		ret = gpio_request(gpio, "i2c-bus");
 		if (ret) {
@@ -1125,6 +1126,7 @@ static int s3c24xx_i2c_suspend_noirq(struct device *dev)
 	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
 
 	i2c->suspended = 1;
+	s3c24xx_i2c_dt_gpio_free(i2c);
 
 	return 0;
 }
-- 
1.6.6.1

Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[Shubhrajyoti Datta]

On Tue, Nov 6, 2012 at 11:40 AM, Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> The I2C driver makes a gpio_request during initialization. This request
> happens again on resume

Do you know why request and free is needed across the suspend and resume?

> and fails due to the earlier successful request.
> Modify the suspend code to free the earlier requested gpios.
>
> Errors on resume without this:
> [   16.020000] s3c-i2c s3c2440-i2c.0: gpio [42] request failed
> [   16.020000] s3c-i2c s3c2440-i2c.1: gpio [44] request failed
> [   16.020000] s3c-i2c s3c2440-i2c.2: gpio [6] request failed
>
> Signed-off-by: Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 3e0335f..9bec49e 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -806,6 +806,7 @@ static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
>                         dev_err(i2c->dev, "invalid gpio[%d]: %d\n", idx, gpio);
>                         goto free_gpio;
>                 }
> +               i2c->gpios[idx] = gpio;
>
>                 ret = gpio_request(gpio, "i2c-bus");
>                 if (ret) {
> @@ -1125,6 +1126,7 @@ static int s3c24xx_i2c_suspend_noirq(struct device *dev)
>         struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
>
>         i2c->suspended = 1;
> +       s3c24xx_i2c_dt_gpio_free(i2c);
>
>         return 0;
>  }
> --
> 1.6.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[Abhilash Kesavan]

Hi Shubhrajyoti,

On Tue, Nov 6, 2012 at 11:42 AM, Shubhrajyoti Datta
<omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Nov 6, 2012 at 11:40 AM, Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> The I2C driver makes a gpio_request during initialization. This request
>> happens again on resume
>
> Do you know why request and free is needed across the suspend and resume?

Other than the way the code is structured where the same
initialization code is called at
probe and resume, there isn't a need for the request/free.

Abhilash

Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[Abhilash Kesavan]

Hi,

Any comments on this fix ?

Abhilash
On Tue, Nov 6, 2012 at 1:08 PM, Abhilash Kesavan
<kesavan.abhilash-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Shubhrajyoti,
>
> On Tue, Nov 6, 2012 at 11:42 AM, Shubhrajyoti Datta
> <omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Tue, Nov 6, 2012 at 11:40 AM, Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>> The I2C driver makes a gpio_request during initialization. This request
>>> happens again on resume
>>
>> Do you know why request and free is needed across the suspend and resume?
>
> Other than the way the code is structured where the same
> initialization code is called at
> probe and resume, there isn't a need for the request/free.
>
> Abhilash

Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

[Wolfram Sang]

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

On Wed, Nov 14, 2012 at 09:10:33AM +0530, Abhilash Kesavan wrote:
> Hi,
> 
> Any comments on this fix ?

Yes, I agree with Shubhrajyoti here and the code should be refactored so
that gpio initialization is only done once at probe-time unless there is
a real reason to do at suspend/resume.

Thanks,

   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: s3c2410: Add fix for i2c suspend/resume

[Wolfram Sang]

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

On Fri, Nov 16, 2012 at 12:46:17PM +0100, Wolfram Sang wrote:
> On Wed, Nov 14, 2012 at 09:10:33AM +0530, Abhilash Kesavan wrote:
> > Hi,
> > 
> > Any comments on this fix ?
> 
> Yes, I agree with Shubhrajyoti here and the code should be refactored so
> that gpio initialization is only done once at probe-time unless there is
> a real reason to do at suspend/resume.

Note that I just applied a few patches to s3c2410 also touching this
area. Would be nice if you could wait a bit until I pushed out my
for-next branch later today and base on that. Or wait for 3.8-rc1.

Thanks,

   Wolfram

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

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