lis3lv02d i2c access on shutdown

lis3lv02d i2c access on shutdown

[James Maki]

Hi,

I have an TI am335x that is using a i2c PMIC with pm_power_off  hook 
which locks up on shutdown. The pm_power_off hook gets stuck obtaining 
i2c_lock_adapter(). The holder of the lock is the lis3lv02d 
accelerometer which never releases it.

I do not think there is an issue in lis3lv02d. I think this is a general 
issue with i2c since other drivers have the same design. What I don't 
have is clear understanding of if/when/how i2c_transfer() can be called 
post kernel_power_off(). Is it possible to call i2c_transfer() at this 
point. I have tried calling schedule() and msleep() inside of 
kernel_power_off() after syscore_shutdown(). It looks like interrupts 
are enabled and I can still schedule at this point. So how is it 
possible that lis3lv02d is never able to exit the i2c_transfer()? Is 
there some other lock inside i2c_transfer that it is blocking on?

Is it possible that the process which had the accelerometer open has 
exited while holding the i2c lock. Is it even possible for a device to 
shutdown or file to close while holding a lock in the kernel?

I looked at adding a lis3lv02d_i2c_shutdown, but that was too late in 
the process already and the lis3lv02d was already stuck. Is there a very 
early place during shutdown that I can kick everyone off the i2c bus? 
Obviously this might not be the best general solution, but my concern 
right now is fixing the problem on this particular system first.

-James

lis3lv02d i2c access on shutdown

[James Maki]

Hi,

I have an TI am335x that is using a i2c PMIC with pm_power_off  hook 
which locks up on shutdown. The pm_power_off hook gets stuck obtaining 
i2c_lock_adapter(). The holder of the lock is the lis3lv02d 
accelerometer which never releases it.

I do not think there is an issue in lis3lv02d. I think this is a general 
issue with i2c since other drivers have the same design. What I don't 
have is clear understanding of if/when/how i2c_transfer() can be called 
post kernel_power_off(). Is it possible to call i2c_transfer() at this 
point. I have tried calling schedule() and msleep() inside of 
kernel_power_off() after syscore_shutdown(). It looks like interrupts 
are enabled and I can still schedule at this point. So how is it 
possible that lis3lv02d is never able to exit the i2c_transfer()? Is 
there some other lock inside i2c_transfer that it is blocking on?

Is it possible that the process which had the accelerometer open has 
exited while holding the i2c lock. Is it even possible for a device to 
shutdown or file to close while holding a lock in the kernel?

I looked at adding a lis3lv02d_i2c_shutdown, but that was too late in 
the process already and the lis3lv02d was already stuck. Is there a very 
early place during shutdown that I can kick everyone off the i2c bus? 
Obviously this might not be the best general solution, but my concern 
right now is fixing the problem on this particular system first.

-James

Re: lis3lv02d i2c access on shutdown

[James Maki]

I understand the issue now. After kernel_shutdown_prepare() is called 
from kernel_power_off() you are no longer allowed to sleep. I couldn't 
find this documented anywhere but it appears to be assumed and probably 
obvious to anyone working in this part of the code.

What this also means you are no longer allowed to grab a mutex. This 
means if another task has stopped while holding the i2c mutex then an 
i2c PMIC will never be able to shutdown the device. There are a lot of 
devices out there depending on an i2c PMIC for shutdown, so kind of 
surprised me that this issue exists. The easy/ugly fix is just to force 
everyone off the i2c bus before calling kernel_shutdown_prepare(). This 
solved my problem, but not a great general solution.

On 02/18/2017 12:02 PM, James Maki wrote:
> Hi,
>
> I have an TI am335x that is using a i2c PMIC with pm_power_off hook 
> which locks up on shutdown. The pm_power_off hook gets stuck obtaining 
> i2c_lock_adapter(). The holder of the lock is the lis3lv02d 
> accelerometer which never releases it.
>
> I do not think there is an issue in lis3lv02d. I think this is a 
> general issue with i2c since other drivers have the same design. What 
> I don't have is clear understanding of if/when/how i2c_transfer() can 
> be called post kernel_power_off(). Is it possible to call 
> i2c_transfer() at this point. I have tried calling schedule() and 
> msleep() inside of kernel_power_off() after syscore_shutdown(). It 
> looks like interrupts are enabled and I can still schedule at this 
> point. So how is it possible that lis3lv02d is never able to exit the 
> i2c_transfer()? Is there some other lock inside i2c_transfer that it 
> is blocking on?
>
> Is it possible that the process which had the accelerometer open has 
> exited while holding the i2c lock. Is it even possible for a device to 
> shutdown or file to close while holding a lock in the kernel?
>
> I looked at adding a lis3lv02d_i2c_shutdown, but that was too late in 
> the process already and the lis3lv02d was already stuck. Is there a 
> very early place during shutdown that I can kick everyone off the i2c 
> bus? Obviously this might not be the best general solution, but my 
> concern right now is fixing the problem on this particular system first.
>
> -James

Re: lis3lv02d i2c access on shutdown

[Éric Piel]

On 18-02-17 19:09, James Maki wrote:
> Hi Eric,
>
> I have an TI am335x that is using a i2c PMIC with pm_power_off  hook
> which locks up on shutdown. The pm_power_off hook gets stuck obtaining
> i2c_lock_adapter(). The holder of the lock is the lis3lv02d
> accelerometer which never releases it.
>
> I do not think there is an issue in lis3lv02d. I think this is a general
> issue with i2c since other drivers have the same design. What I don't
> have is clear understanding of if/when/how i2c_transfer() can be called
> post kernel_power_off(). Is it possible to call i2c_transfer() at this
> point. I have tried calling schedule() and msleep() inside of
> kernel_power_off() after syscore_shutdown(). It looks like interrupts
> are enabled and I can still schedule at this point. So how is it
> possible that lis3lv02d is never able to exit the i2c_transfer()? Is
> there some other lock inside i2c_transfer that it is blocking on?
>
> Is it possible that the process which had the accelerometer open has
> exited while holding the i2c lock. Is it even possible for a device to
> shutdown or file to close while holding a lock in the kernel?
>
> I looked at adding a lis3lv02d_i2c_shutdown, but that was too late in
> the process already and the lis3lv02d was already stuck. Is there a very
> early place during shutdown that I can kick everyone off the i2c bus?
> Obviously this might not be the best general solution, but my concern
> right now is fixing the problem on this particular system first.
>

Dear James,

Unfortunately, I don't know enough about I2C to be able to help you. 
Honestly, I know pretty much nothing about this subsystem, and all the 
code dealing with it in the lis3lv02d driver has been contributed by 
other people.

The only thing I can say is that indeed it does sound more like an issue 
happening at the I2C level than in this specific driver. Concerning the 
lock, if I understand correctly, it shouldn't possible for a process 
talking to the accelerometer to suddenly be killed, while a kernel lock 
is held, preventing it to ever be release. That said, in practice, there 
are sufficiently enough bugs in the kernel for this to be possible ;-)

So I'd suggest you to contact people who understand well the I2C 
subsystem. I've CC'd Wolfram, maintainer of the subsystem, and the 
dedicated mailing-list.

Best,
Éric

Re: lis3lv02d i2c access on shutdown

[Wolfram Sang]

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


> >issue with i2c since other drivers have the same design. What I don't
> >have is clear understanding of if/when/how i2c_transfer() can be called
> >post kernel_power_off().

Not sure if it is exactly the same issue, but last year we concluded
that we probably need an optional i2c_transfer_noirq callback in
i2c_adapter to talk to PMICs very late or very early. No work has been
done so far, though. I hope I will find time for it this year.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: lis3lv02d i2c access on shutdown

[James Maki]

I think it is the same issue. How would i2c_transfer_noirq work? In my 
case another task was in the middle of a transaction when it was 
"stopped". This means not only do you need i2c_transfer_noirq, but also 
need to flush the bus before calling kernel_shutdown_prepare().


On 03/04/2017 04:32 PM, Wolfram Sang wrote:
>>> issue with i2c since other drivers have the same design. What I don't
>>> have is clear understanding of if/when/how i2c_transfer() can be called
>>> post kernel_power_off().
> Not sure if it is exactly the same issue, but last year we concluded
> that we probably need an optional i2c_transfer_noirq callback in
> i2c_adapter to talk to PMICs very late or very early. No work has been
> done so far, though. I hope I will find time for it this year.
>

Re: lis3lv02d i2c access on shutdown

[Wolfram Sang]

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

On Mon, Mar 06, 2017 at 09:10:52AM -0600, James Maki wrote:
> I think it is the same issue. How would i2c_transfer_noirq work? In my case
> another task was in the middle of a transaction when it was "stopped". This
> means not only do you need i2c_transfer_noirq, but also need to flush the
> bus before calling kernel_shutdown_prepare().

I don't know yet. I have a rough idea, but not even a design. The above
issue is important to know when starting to design the interface,
though. So, thanks for letting us know.

That all being said, if somebody is faster than me in doing this
interface, I am all for it :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]