Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[Jiri Kosina]

On Fri, 22 Feb 2013, 송은봉 wrote:

> 
>  
> 
> I've been debugging the abnormal operation of i2c on octeon.
> If a process is terminated by signal in the middle of i2c operation,
> next i2c read operation which is done by another process was failed.
> So i changed to ignore signal in the middle of i2c operation.
> After that the problem was not reproduced.

This is not really material directly for trivial.git. Adding maintainers 
to CC.

> 
>  
> 
> Signed-off-by: EunBong Song <eunb.song-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> diff -up drivers/i2c/busses/i2c-octeon.c{.orig,}
> --- drivers/i2c/busses/i2c-octeon.c.orig 2013-02-21 08:09:03.168018843 -0800
> +++ drivers/i2c/busses/i2c-octeon.c 2013-02-21 08:09:38.344018898 -0800
> @@ -183,7 +183,7 @@ static irqreturn_t octeon_i2c_isr(int ir
>   struct octeon_i2c *i2c = dev_id;
>  
>   octeon_i2c_int_disable(i2c);
> - wake_up_interruptible(&i2c->queue);
> + wake_up(&i2c->queue);
>  
>   return IRQ_HANDLED;
>  }
> @@ -206,7 +206,7 @@ static int octeon_i2c_wait(struct octeon
>  
>   octeon_i2c_int_enable(i2c);
>  
> - result = wait_event_interruptible_timeout(i2c->queue,
> + result = wait_event_timeout(i2c->queue,
>          octeon_i2c_test_iflg(i2c),
>          i2c->adap.timeout);
> 
>  
> 
>  
> 
> [IMAGE]
> 
> [SeenTimeChecker?do=5c4ee24ec3c4b5ef0b1e1d188d51662fbee53716e9d11aa47790d17410439b26f5961395f090d04f94a68828d2d0a033db9fdddda33
> e82cbe4a391424e62fcf6cf878f9a26ce15a0]
> 

-- 
Jiri Kosina
SUSE Labs

Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[Wolfram Sang]

On Tue, Feb 26, 2013 at 11:02:17AM +0100, Jiri Kosina wrote:
> On Fri, 22 Feb 2013, 송은봉 wrote:
> 
> > 
> >  
> > 
> > I've been debugging the abnormal operation of i2c on octeon.
> > If a process is terminated by signal in the middle of i2c operation,
> > next i2c read operation which is done by another process was failed.
> > So i changed to ignore signal in the middle of i2c operation.
> > After that the problem was not reproduced.
> 
> This is not really material directly for trivial.git. Adding maintainers 
> to CC.

Yes, this should not go via trivial. Please resend to i2c list. Patch
looks okay from a glimpse (and fixes an issue we have seen before and
fixed the same way).

Thanks,

   Wolfram

Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[anish singh]

On Tue, Feb 26, 2013 at 4:37 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Tue, Feb 26, 2013 at 11:02:17AM +0100, Jiri Kosina wrote:
>> On Fri, 22 Feb 2013, 송은봉 wrote:
>>
>> >
>> >
>> >
>> > I've been debugging the abnormal operation of i2c on octeon.
>> > If a process is terminated by signal in the middle of i2c operation,
>> > next i2c read operation which is done by another process was failed.
>> > So i changed to ignore signal in the middle of i2c operation.
>> > After that the problem was not reproduced.
>>
>> This is not really material directly for trivial.git. Adding maintainers
>> to CC.
>
> Yes, this should not go via trivial. Please resend to i2c list. Patch
> looks okay from a glimpse (and fixes an issue we have seen before and
> fixed the same way).
Just curios to know why reinitializing the i2c controller is a bad idea?
This fix looks perfect but if we know that there is only one i2c device
currently being controlled by controller.Can we(i know it is expensive
compare to the fix provided) not re-initialize the i2c-controller?

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

I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[송은봉]

I've been debugging the abnormal operation of i2c on octeon. 
If a process is terminated by signal in the middle of i2c operation, 
next i2c read operation which is done by another process was failed.
So i changed to ignore signal in the middle of i2c operation. 
After that the problem was not reproduced.

 

Signed-off-by: EunBong Song <eunb.song@samsung.com>

diff -up drivers/i2c/busses/i2c-octeon.c{.orig,}
--- drivers/i2c/busses/i2c-octeon.c.orig 2013-02-21 08:09:03.168018843 -0800
+++ drivers/i2c/busses/i2c-octeon.c 2013-02-21 08:09:38.344018898 -0800
@@ -183,7 +183,7 @@ static irqreturn_t octeon_i2c_isr(int ir
  struct octeon_i2c *i2c = dev_id;
 
  octeon_i2c_int_disable(i2c);
- wake_up_interruptible(&i2c->queue);
+ wake_up(&i2c->queue);
 
  return IRQ_HANDLED;
 }
@@ -206,7 +206,7 @@ static int octeon_i2c_wait(struct octeon
 
  octeon_i2c_int_enable(i2c);
 
- result = wait_event_interruptible_timeout(i2c->queue,
+ result = wait_event_timeout(i2c->queue,
         octeon_i2c_test_iflg(i2c),
         i2c->adap.timeout);

Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[Wolfram Sang]

On Tue, Feb 26, 2013 at 09:08:43PM +0000, 송은봉 wrote:
> 
> I've been debugging the abnormal operation of i2c on octeon. 
> If a process is terminated by signal in the middle of i2c operation, 
> next i2c read operation which is done by another process was failed.
> So i changed to ignore signal in the middle of i2c operation. 
> After that the problem was not reproduced.
> 
>  
> 
> Signed-off-by: EunBong Song <eunb.song-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Basically OK, but you have lots of whitespace issues, so I can't apply
your patch. Please resend and make sure it applies.

Thanks,

   Wolfram

Re: I2C: Fix i2c fail problem when a process is terminated by a signal on octeon in 3.8

[David Daney]

On 02/26/2013 12:54 PM, 송은봉 wrote:
> I've been debugging the abnormal operation of i2c on octeon.
> If a process is terminated by signal in the middle of i2c operation,
> next i2c read operation which is done by another process was failed.
> So i changed to ignore signal in the middle of i2c operation.
> After that the problem was not reproduced.
> 

This is a known issue.  However I don't think the solution you have is
correct...

> Signed-off-by: EunBong Song <eunb.song-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org 
> <mailto:eunb.song-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>>
> 
> diff -up drivers/i2c/busses/i2c-octeon.c{.orig,}
> --- drivers/i2c/busses/i2c-octeon.c.orig 2013-02-21 08:09:03.168018843 -0800
> +++ drivers/i2c/busses/i2c-octeon.c 2013-02-21 08:09:38.344018898 -0800
> @@ -183,7 +183,7 @@ static irqreturn_t octeon_i2c_isr(int ir
>    struct octeon_i2c *i2c = dev_id;
> 
>    octeon_i2c_int_disable(i2c);
> - wake_up_interruptible(&i2c->queue);
> + wake_up(&i2c->queue);
> 
>    return IRQ_HANDLED;
>   }
> @@ -206,7 +206,7 @@ static int octeon_i2c_wait(struct octeon
> 
>    octeon_i2c_int_enable(i2c);
> 
> - result = wait_event_interruptible_timeout(i2c->queue,
> + result = wait_event_timeout(i2c->queue,

If the wait is not interruptible, I think you will not be able to 'kill
-9' a userspace process blocked here.

>           octeon_i2c_test_iflg(i2c),
>           i2c->adap.timeout);
> 

The real solution is to move processing of the I2C protocol to a kernel
thread and communicate between the this thread and userspace via a
command queue mechanism, much like the way it is done in the mmc/host
driver infrastructure.

David Daney