linuxsh-dev

APM issues

[Kristoffer E. <kri...@gm...>]

Bugtracking the hp6xx_apm functions and ran into some question marks. This
happend after the apm merger so its not a big suprise. Doesn't seem to be
many systems using apm in arch/sh though.

Problems:
1. Once powerbutton is hit IRQ 32 is generated and never stops
    Best approach? Clear request bit? But will that stop anything since a
new request will be created before the power button can be released.
2. mutex_lock(&state_lock); from apm-emulation.c seems to freeze the
machine. adding a ifndef config_cpu_subsystem..
   makes it run abit longer. I noticed that the mutex lock wasn't in the
"old" version. Suggestions?
3. Getting apm: apm queue event overflowed before it freezes (freezes
probably due to some mutex locks I've missed),
    this would indicate that more than one request is sent by the apm
interrupt handler?
4. PM_DISK_FIRMWARE is removed, but im not sure if that will be an issue
since the pm is handled by board specific code which doesn't require
anything to be saved. It simply turns off all clocks besides tmu/rtc and
drops power to lcd/pcmcia. Ram shouldn't be cleared.

In short I need a good way for the interrupt handler to make sure only one
request is sent and someway to get rid off / fix the mutex_lock. It would
get ugly with alot of ifdefs but dont see any other solution currently.

Re: APM issues

[Paul M. <le...@li...>]

On Mon, May 14, 2007 at 03:58:44PM -0700, Kristoffer Ericson wrote:
> Bugtracking the hp6xx_apm functions and ran into some question marks. This
> happend after the apm merger so its not a big suprise. Doesn't seem to be
> many systems using apm in arch/sh though.
> 
Yes, we've started on the SH7722 work tying in to the clock framework,
but not quite there yet. hp6xx is the only in-tree user of the code at
the moment, so you get to run in to all of these problems before anyone
else :-)

> Problems:
> 1. Once powerbutton is hit IRQ 32 is generated and never stops
>    Best approach? Clear request bit? But will that stop anything since a
> new request will be created before the power button can be released.

Sounds like it's not acked and masked properly, start debugging the
irq_chip handler that's got this IRQ.

> 2. mutex_lock(&state_lock); from apm-emulation.c seems to freeze the
> machine. adding a ifndef config_cpu_subsystem..
>   makes it run abit longer. I noticed that the mutex lock wasn't in the
> "old" version. Suggestions?

If it's freezing on a mutex_lock() it means it's blocked while trying to
acquire the mutex, meaning someone got it beforehand and is never
releasing it. Some of this could be related to your call path, I'd
suggest turning on mutex debugging and lockdep and see if anything
tumbles out.

> 3. Getting apm: apm queue event overflowed before it freezes (freezes
> probably due to some mutex locks I've missed),
>    this would indicate that more than one request is sent by the apm
> interrupt handler?

That seems likely, and would also explain the mutex_lock() re-try before
unlock. Debugging the interrupt handler first makes the most sense.

> 4. PM_DISK_FIRMWARE is removed, but im not sure if that will be an issue
> since the pm is handled by board specific code which doesn't require
> anything to be saved. It simply turns off all clocks besides tmu/rtc and
> drops power to lcd/pcmcia. Ram shouldn't be cleared.
> 
This should not impact you at all, we were basically just abusing the
definition of PM_DISK_FIRMWARE anyways. The current scheme should be more
fundamentally correct.

> In short I need a good way for the interrupt handler to make sure only one
> request is sent and someway to get rid off / fix the mutex_lock. It would
> get ugly with alot of ifdefs but dont see any other solution currently.

If the IRQ is not being masked and acked, that's a serious bug, and one
that needs to be corrected. It's not worth wasting any time trying to
limit the number of requests it sends if it's constantly firing anyways
(ie, you're never going to go to sleep with IRQs running wild).

Re: APM issues

[Kristoffer E. <kri...@gm...>]

If the irq.chip handler is at fault then that would explain my pcmcia issues
also. I've added the hd64461 pcmcia code, but get strange unexpected irq's
even though they should be registerd.

Thanks for info, know where to start looking.


2007/5/15, Paul Mundt <le...@li...>:
>
> On Mon, May 14, 2007 at 03:58:44PM -0700, Kristoffer Ericson wrote:
> > Bugtracking the hp6xx_apm functions and ran into some question marks.
> This
> > happend after the apm merger so its not a big suprise. Doesn't seem to
> be
> > many systems using apm in arch/sh though.
> >
> Yes, we've started on the SH7722 work tying in to the clock framework,
> but not quite there yet. hp6xx is the only in-tree user of the code at
> the moment, so you get to run in to all of these problems before anyone
> else :-)
>
> > Problems:
> > 1. Once powerbutton is hit IRQ 32 is generated and never stops
> >    Best approach? Clear request bit? But will that stop anything since a
> > new request will be created before the power button can be released.
>
> Sounds like it's not acked and masked properly, start debugging the
> irq_chip handler that's got this IRQ.
>
> > 2. mutex_lock(&state_lock); from apm-emulation.c seems to freeze the
> > machine. adding a ifndef config_cpu_subsystem..
> >   makes it run abit longer. I noticed that the mutex lock wasn't in the
> > "old" version. Suggestions?
>
> If it's freezing on a mutex_lock() it means it's blocked while trying to
> acquire the mutex, meaning someone got it beforehand and is never
> releasing it. Some of this could be related to your call path, I'd
> suggest turning on mutex debugging and lockdep and see if anything
> tumbles out.
>
> > 3. Getting apm: apm queue event overflowed before it freezes (freezes
> > probably due to some mutex locks I've missed),
> >    this would indicate that more than one request is sent by the apm
> > interrupt handler?
>
> That seems likely, and would also explain the mutex_lock() re-try before
> unlock. Debugging the interrupt handler first makes the most sense.
>
> > 4. PM_DISK_FIRMWARE is removed, but im not sure if that will be an issue
> > since the pm is handled by board specific code which doesn't require
> > anything to be saved. It simply turns off all clocks besides tmu/rtc and
> > drops power to lcd/pcmcia. Ram shouldn't be cleared.
> >
> This should not impact you at all, we were basically just abusing the
> definition of PM_DISK_FIRMWARE anyways. The current scheme should be more
> fundamentally correct.
>
> > In short I need a good way for the interrupt handler to make sure only
> one
> > request is sent and someway to get rid off / fix the mutex_lock. It
> would
> > get ugly with alot of ifdefs but dont see any other solution currently.
>
> If the IRQ is not being masked and acked, that's a serious bug, and one
> that needs to be corrected. It's not worth wasting any time trying to
> limit the number of requests it sends if it's constantly firing anyways
> (ie, you're never going to go to sleep with IRQs running wild).
>

Re: APM issues

[Kristoffer E. <kri...@gm...>]

On Tue, 15 May 2007 08:26:41 +0900
Paul Mundt <le...@li...> wrote:

> On Mon, May 14, 2007 at 03:58:44PM -0700, Kristoffer Ericson wrote:
> > Bugtracking the hp6xx_apm functions and ran into some question marks. This
> > happend after the apm merger so its not a big suprise. Doesn't seem to be
> > many systems using apm in arch/sh though.
> > 
> Yes, we've started on the SH7722 work tying in to the clock framework,
> but not quite there yet. hp6xx is the only in-tree user of the code at
> the moment, so you get to run in to all of these problems before anyone
> else :-)
> 
> > Problems:
> > 1. Once powerbutton is hit IRQ 32 is generated and never stops
> >    Best approach? Clear request bit? But will that stop anything since a
> > new request will be created before the power button can be released.
> 
> Sounds like it's not acked and masked properly, start debugging the
> irq_chip handler that's got this IRQ.

I've read through the manual and perhaps its due to IRQ0->IRQ3 being set as edge instead of
level. It says in the manual that to clear edge interrupts one needs to  read corresponding bit from IRR0 (1) and then write 0. *shrug* best idea yet. It would explain why a single press would send out interrupts.

They should be set as edge by default, I've done some tests trying to set IRQ0 as level, but no boot. 


> 
> > 2. mutex_lock(&state_lock); from apm-emulation.c seems to freeze the
> > machine. adding a ifndef config_cpu_subsystem..
> >   makes it run abit longer. I noticed that the mutex lock wasn't in the
> > "old" version. Suggestions?
> 
> If it's freezing on a mutex_lock() it means it's blocked while trying to
> acquire the mutex, meaning someone got it beforehand and is never
> releasing it. Some of this could be related to your call path, I'd
> suggest turning on mutex debugging and lockdep and see if anything
> tumbles out.
> 
> > 3. Getting apm: apm queue event overflowed before it freezes (freezes
> > probably due to some mutex locks I've missed),
> >    this would indicate that more than one request is sent by the apm
> > interrupt handler?
> 
> That seems likely, and would also explain the mutex_lock() re-try before
> unlock. Debugging the interrupt handler first makes the most sense.
> 
> > 4. PM_DISK_FIRMWARE is removed, but im not sure if that will be an issue
> > since the pm is handled by board specific code which doesn't require
> > anything to be saved. It simply turns off all clocks besides tmu/rtc and
> > drops power to lcd/pcmcia. Ram shouldn't be cleared.
> > 
> This should not impact you at all, we were basically just abusing the
> definition of PM_DISK_FIRMWARE anyways. The current scheme should be more
> fundamentally correct.
> 
> > In short I need a good way for the interrupt handler to make sure only one
> > request is sent and someway to get rid off / fix the mutex_lock. It would
> > get ugly with alot of ifdefs but dont see any other solution currently.
> 
> If the IRQ is not being masked and acked, that's a serious bug, and one
> that needs to be corrected. It's not worth wasting any time trying to
> limit the number of requests it sends if it's constantly firing anyways
> (ie, you're never going to go to sleep with IRQs running wild).

Re: APM issues

[Paul M. <le...@li...>]

On Fri, May 18, 2007 at 03:11:59AM -0700, Kristoffer Ericson wrote:
> On Tue, 15 May 2007 08:26:41 +0900
> Paul Mundt <le...@li...> wrote:
> > Sounds like it's not acked and masked properly, start debugging the
> > irq_chip handler that's got this IRQ.
> > 
>   The interrupt (32) is defined by the ipr_mapping and should be handled by
>   disable_ipr_irq(irq) for mask_acked. The line goes : 
>   ctrl_outw(ctrl_inw(p->addr) | (p->priority << p->shift), p->addr);
>  
[snip]

> maybe it has something to do with level/edge settings.
> 
Yes, if it's just a normal IPR IRQ, then the IRQ flow is the next thing
to look at.

On Fri, May 18, 2007 at 03:58:09AM -0700, Kristoffer Ericson wrote:
> I've read through the manual and perhaps its due to IRQ0->IRQ3 being
> set as edge instead of level. It says in the manual that to clear edge
> interrupts one needs to  read corresponding bit from IRR0 (1) and then
> write 0. *shrug* best idea yet. It would explain why a single press
> would send out interrupts.
> 
IRQ sensing is configurable, we'll have to implement a bit of logic for
setting the type, and you'll have to toss in a flag at your request_irq()
time with the level/edge selection, but that's not too big of a deal.
Generally we've avoided this since we have next to no pure-edge cases to
worry about (mostly just old broken SuperIOs that people habitually
interface to a level-only pin anyways).

> They should be set as edge by default, I've done some tests trying to
> set IRQ0 as level, but no boot. 
> 
If it's an edge IRQ you should not be seeing a retriggering of the IRQ,
that's more a property of unacked level IRQs that don't deassert until
they've been handled, in the edge case you'll simply miss the IRQ.

Changing the sense selection for an IRQ will not impact your ability to
boot (especially on something as mundane as IRQ0), so I suggest you take
a look at this again. The generic hardirq layer is the way to do this,
including a ->set_type() in the IPR irq_chip(), going around it is only
asking for trouble.