Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To
    ranma
  • Operating System Sansa c200
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by ranma - 2010-03-19
Last edited by ranma - 2010-03-23

FS#11130 - Rewrite ascodec_as3514.c to use interrupts

This patch rewrites ascodec_as3514.c to use interrupts.
It gets rid of the ascodec mutex and instead introduces a mutex in adc-as3514.c.
Requests are handled using a simple request queue, and allows asynchronous request to be started from interrupt context.
The (20+sizeof(struct wakeup)) bytes request struct is either allocated on the stack (synchronous read/write) or has to be allocated statically by the caller (asynchronous read).

The asynchronous write is used in system-as3525.c (which is run with interrupts disabled).
asynchronous reads are used by another pending patch of mine implementing AUDIO_IRQ (int 9) handling for usb_plug detection without needing a thread.

   i2c_irq.patch (13.5 KiB)
Closed by  ranma
2010-03-23 09:06
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Committed

funman commented on 2010-03-19 19:08

Good work!

You changed ascodec_lock() into a adc specific lock in adc-as3514.c, but this file is also used by e200v1/c200v1 : doesn’t it cause a problem for those devices?

ranma commented on 2010-03-19 20:23

Good question. I think you’re right. Updated patch attached.

   ascodec_irq.patch (12.8 KiB)
funman commented on 2010-03-20 15:41

Tested on Clip+ : OK

mc2739 commented on 2010-03-20 22:14

Tested on e260v2 with r25262:

When I attach the USB cable while in the Rockbox menu I intermittently get a black screen lockup that requires a long poweroff (10 secs) to recover.

All other functions appear normal.

ranma commented on 2010-03-21 05:22
Michael Chicoine (mc2739)
Only with this patch? Or with 11130+11131 applied?
Since 11130 doesn’t change the usb detection I wonder why it should cause a lockup on usb plugin…

Hmm, backlight is controlled using as3514 on e200 though and system_reboot() calls _backlight_off() (why? If it reboots the backlight will be turned back on very soon anyway…). So if system_reboot() is called with interrupts disabled it would hang. Though it’s should be called from the usb thread, which means interrupts should be enabled and I don’t see why suddenly at that point the interrupt wouldn’t fire if it has been working fine that far…

ranma commented on 2010-03-21 07:36
Michael Chicoine (mc2739)
Can you try this?
It will fall back to polling if irqs are disabled.
   ascodec_irq.patch (13.2 KiB)
mc2739 commented on 2010-03-21 14:19

I tested this patch by itself first, and then tested 11131 along with this patch. The lockup occurred in both cases.

So far, with minimal testing, the new patch looks good. I have not had any lockups, yet. I will continue testing throughout the day. I will also test it with 11131 also applied.

mc2739 commented on 2010-03-22 11:31

After further testing, I have not had any lockups with the latest patch. No other problems noted.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing