Support and General Use > Hardware
ipod mini 2nd gen/scroll wheel unresponsive after hold on/off
chris_s:
By the way, a udelay of 1000 instead of 2000, i.e. a 1 millisecond delay, seems to suffice in my experience to get rid of the bug (only tested on iPod 4Gs).
I know Torne Wuff mentions sleeping in an interrupt handler as a bad idea in the comments to FS#5230, but as far as I can tell, udelay isn't technically like sleeping but instead busy waits and so is appropriate in this context? It is also already used in the same file, albeit with a shorter time period... (in the comments it says that in the ipodlinux source it used to delay for 250 microseconds instead of the 50 that are used now). Wouldn't the same objection apply to that line then, or am I missing something?
wodz:
By sleeping in interrupt handler he meant busylooping. udelay(2000) is 2 ms of not doing any useful stuff. I can easily understand the objections for this patch. From the other hand https://www.rockbox.org/tracker/task/5230#comment30395 says udelay is not in hot codepath. ifdefing is an option but ugly one.
chris_s:
--- Quote from: wodz on January 04, 2019, 04:38:40 PM ---By sleeping in interrupt handler he meant busylooping. udelay(2000) is 2 ms of not doing any useful stuff. I can easily understand the objections for this patch. From the other hand https://www.rockbox.org/tracker/task/5230#comment30395 says udelay is not in hot codepath. ifdefing is an option but ugly one.
--- End quote ---
I would understand the objection if this didn't fix a long-standing, annoying and obvious bug (admittedly on devices used by fewer and fewer people, probably). By using ifdef to target only the actually affected devices, it could be assured that there's no chance of this (extremely small and localized) patch making anything worse on devices other than the ones suffering from the problem – although it seems like the patch may never be in the codepath for unaffected devices anyway. On the affected ones, it seems to me the tradeoff would be very much worth it to get rid of the problem of disabling HOLD and then (more often than not) having to repeatedly press a button (or scroll) before any input action is recognized. Since the patch only affects the scenario in which the device already behaves in an extremely unsatisfactory way, I'm not sure how the unnoticeable delay is making anything perceptibly worse, unless there is some deeper underlying (side-)effect I'm not aware of.
From personal experience, I'd say the experience between the official build and the patched one is night and day, and I, myself, would never want to use an unpatched version on my iPod 4G ever again (except for testing purposes).
To me, the question is whether the patch is bad enough that it's preferable to leave the existing bug in place instead? From my perspective as a user, this is easily answered. It's the choice between a system that (viewed as a black box) seems to work exactly as intended and one that seems partly broken.
saratoga:
--- Quote from: wodz on January 04, 2019, 04:38:40 PM ---By sleeping in interrupt handler he meant busylooping. udelay(2000) is 2 ms of not doing any useful stuff. I can easily understand the objections for this patch. From the other hand https://www.rockbox.org/tracker/task/5230#comment30395 says udelay is not in hot codepath. ifdefing is an option but ugly one.
--- End quote ---
Not familiar with the iPods, but according to the wiki, the lowest byte on the clickwheel is 0x1a when unlocked, and assuming interrupts aren't getting generated when the device is locked (i hope??), it should be pretty safe. You'd only trigger the wait when you're in some weird state not documented on the wiki, which I guess is whatever the delay is supposed to correct.
https://www.rockbox.org/wiki/PortalPlayer502x#I2C_Controller
traveltrousers:
I also still have this problem on my ipod 5.5 running RB 3.14.
This is a 13 year old bug!! :p
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version