Rockbox

Well, your patches are interesting! :-)

I also measure less current (power consumption). From the measurements I would expect savings of about 1mA. To finalize the tests I will battery bench a patched software over night.

Any other testers are welcome -- maybe this patch can also be applied to other iPods (e.g. nano).

Hmm, even though the patch for the battery current shows a clear reduction in current, the battery bench is identical. There seems to be no difference in runtime with this patch...

So you've also seen the change in 4066_ISTAT ( FS#9728 )? I wonder what's going on.

Yes, I also had some reduction in Ibat (about -2). But no effect on battery runtime at all.

Measurements of FireWire input power show some savings. These measurements were taken with two different DMMs while at the main menu with ide power and the backlight turned off.
r19869 unaltered: 14.7 to 14.8 mA , 14.81 to 14.86 mA
r19869 + FS#9746: 14.3 to 14.4 mA , 14.41 to 14.48 mA

Hmm, I still don't understand why I see no difference in the battery benchs. A difference of 0.4mA should result in better runtime results. Boris, could you perform 2 benchs with unaltered and patched versions?

Yeah, I don't understand that either. Since 0.4mA is the savings in FireWire current at 12V, the battery current savings should be even greater, and that ought to show in battery bench results. What do you mean when you say you see no difference? Is there literally no difference or do you feel that the difference seen is meaningless if you consider measurement precision?

I've started a baseline battery bench tonight.

I did a lot of benchs to test this patch and the DMA patch against an unpatched version (unpatched in terms of, "did not include your patches"). The battery benchs of unpatched version against the version with this patch did not differ significantly (most important is 90-10 runtime). I deleted the bench files, so I cannot post them now. With 0.4mA fewer current consumption I would expect 10-15min more runtime for my testcase.

I am really curious about your results :)

My battery bench didn't turn out very good. It ends at 15%, maybe because the iPod was shut down by hardware rather than by Rockbox, perhaps because my battery is old. With the patch, logged time and final voltage are both lower. However, the descent was more steep without the patch.

I could claim that the difference in slope means that power is being saved, and the lower initial voltage and shorter runtime is due to a lower initial state of charge. However, I don't feel confident about saying this. I wonder how I could standardize the initial conditions. Maybe the state of charge differs based on whether the battery was charged from empty or topped up multiple times. I also could have started the battery_bench in a more consistent way, maybe playing, pausing, rewinding to 0:00, starting battery bench and then unpausing, instead of starting battery_bench and then browsing to the file and playing it.

Actually, if I horizontally move one of the graphs, they can match up almost exactly. The difference in slope in the .png I attached above is probably deceptive.

Here is my attempt to match the curves at the start. The curve with this patch does seem to be a bit less steep, though it ends a bit sooner.

The pins in question are initialized to OUTPUT_EN =1 and OUTPUT_VAL = 0. So, when ENABLE = 1, they should output a 0. I guess this minimizes the amount of current flowing into the IDE data and control lines, which are also effectively at 0 at the HD end when IDE power is off. It seems that this is valid for other PP502x based iPods and it may be valid for other PP502x based devices with ATA storage.

There's also DEV_INIT2 & 0x6000. Those bits must be 0 for IDE to work. They may be set to 1 at the end of ide_power_enable(false) and set to 0 at the beginning of ide_power_enable(true), but that doesn't seem to save any current.

What should I do with this patch? Alternatives include:
1) Commit it for 5G iPods
2) Commit a patch like this for all PP502x iPods which can turn off IDE
3) Commit a patch like this for all PP502x targets which can turn off IDE
4) Close this and forget about it.

Based on measurements via  FS#9728  and measurements of FireWire input current via two different DMMs, this patch reduces current draw. The battery bench may show a small savings, but it's not really conclusive. The savings are relatively insignificant, but if it's easy to save a small bit of power, why not do it?

In all cases, if I commit this I would add port initialization.

BTW I didn't realize that reducing priority (or was it severity) would change the task to a different and unusual colour, and kind of highlight it. Maybe I shouldn't do that?

As this patch reduces current draw I would like to see it in svn. But before submitting a general PP502x-patch such change should be tested and verified for the affected targets. Why not create the general PP502x-patch, submit it here and call for testers for all needed targets via IRC or the forum?

You are of course right: it needs to be tested. I will create a general PP502x patch, post it here and call for testers.

Submitted for iPod Video with r25255.