New, simpler version of the CHDK 'loader' code.

waterwingz

12140

Re: New, simpler version of the CHDK 'loader' code.

« Reply #10 on: 02 / December / 2012, 09:19:31 »

Advertisements

Quote from: philmoz on 01 / December / 2012, 23:47:39

Although it all looks ok - I'm a little nervous about committing a lot of untested changes.

Speaking as someone who was allowed to do just that, it's not that bad.

In this case, you have what I would call plenty of different cameras that have now successfully tested the fix. And if it doesn't work for some reason on a particular camera, it should be obvious right away ? So I'd say "go for it" - at least in the 1.2.0 version.

Incidentally, it seems like a good cleanup but can you really tell any difference in boot time? Sorry if I missed something but is there any other benefit to the change?

Logged

Ported : A1200 SD940 G10 Powershot N G16

reyalp

14114

Re: New, simpler version of the CHDK 'loader' code.

« Reply #11 on: 02 / December / 2012, 15:14:41 »

Quote from: waterwingz on 02 / December / 2012, 09:19:31

Speaking as someone who was allowed to do just that, it's not that bad.

In this case, you have what I would call plenty of different cameras that have now successfully tested the fix. And if it doesn't work for some reason on a particular camera, it should be obvious right away ? So I'd say "go for it" - at least in the 1.2.0 version.

I view this a bit differently. For the loader, the risk may be low, but the benefit is also low. If there is a mistake and one camera crashes on boot, we might not know about it for a long time, at which point it may be difficult to track down. In the case of the remote code, there was really no other option. Using the new system really required that every camera be updated, and getting rid of the old mess was a substantial benefit. In contrast, I don't see any significant problem if some cameras stay on the old loader code for a long time. At worst, it will confuse people looking at the code.

Logged

Don't forget what the H stands for.

waterwingz

12140

Re: New, simpler version of the CHDK 'loader' code.

« Reply #12 on: 02 / December / 2012, 15:47:31 »

Quote from: reyalp on 02 / December / 2012, 15:14:41

I view this a bit differently. For the loader, the risk may be low, but the benefit is also low.

I do not disagree. That's kind of why I asked the questions at the end of my post.

Logged

Ported : A1200 SD940 G10 Powershot N G16

srsa_4c

4451

Re: New, simpler version of the CHDK 'loader' code.

« Reply #13 on: 09 / May / 2013, 16:00:48 »

I have a question.

Why does the typical CHDK loader code look like this ( simpler version, my_restart() ):
[phase 1] copy CHDK blob to its final place
[phase 2] execute a "restart" routine taken from the firmware

The latter seems to start with code that disables interrupts. I only have one fw update program disassembly (for the s1is), that starts also with disabling interrupts.

Is it absolutely certain that no interrupts happen during [phase 1] ? (I know that the copy phase doesn't take significant time, but that time is still greater than zero.) If an interrupt does strike, what could happen?

As usual, I could be possibly overlooking something...

Logged

reyalp

14114

Re: New, simpler version of the CHDK 'loader' code.

« Reply #14 on: 09 / May / 2013, 16:43:27 »

Quote from: srsa_4c on 09 / May / 2013, 16:00:48

Is it absolutely certain that no interrupts happen during [phase 1] ? (I know that the copy phase doesn't take significant time, but that time is still greater than zero.) If an interrupt does strike, what could happen?

Going from memory (quite likely to be wrong

) I think the canon restart code that loads diskboot.bin or ps.fi* will disable interrupts before jumping to the newly loaded binary. Otherwise, if an interrupt were to occur, it would go to the "old"interrupt handlers (assuming the restart code doesn't set thing back to the high vector at 0xFFFF0000) with possibly bad results.

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: New, simpler version of the CHDK 'loader' code.

« Reply #15 on: 10 / May / 2013, 09:54:09 »

Quote from: reyalp on 09 / May / 2013, 16:43:27

Going from memory (quite likely to be wrong ) I think the canon restart code that loads diskboot.bin or ps.fi* will disable interrupts before jumping to the newly loaded binary.

Checked the a410 code, and this seems to be the case indeed. A routine which emits "Restart called" on the UART disables interrupts before calling the bootloader.
The reason I was asking is that I can reproduce an issue on one of my a410 cameras (which is not in the best shape and has no RTC battery). When using fw update to start CHDK, the cam extends the lens, and retracts it with an unusually slow speed. Display only comes on when it's finished. The UART log shows evidence of lens retraction, but not more. However, ~~since I'm not able to reproduce this on my other a410~~, it's probably hw related.
update: same thing happens on the other a410 when the backup battery is removed. Once it happens, the camera must rest for ~10 minutes for the issue to show up again.

« Last Edit: 10 / May / 2013, 10:07:08 by srsa_4c »

Logged

nafraf

1308

Re: New, simpler version of the CHDK 'loader' code.

« Reply #16 on: 15 / December / 2014, 20:59:12 »

Patches for a420 and a710. Simpler loader code and remove autodetected entries from stubs_entry_2.S and stubs_min.S

a420_newloader_trunk3801.patch (17.51 kB - downloaded 24 times.)

a710_newloader_trunk3801.patch (13.57 kB - downloaded 22 times.)

Logged

koshy

1096

Re: New, simpler version of the CHDK 'loader' code.

« Reply #17 on: 15 / December / 2014, 21:09:11 »

Quote from: nafraf on 15 / December / 2014, 20:59:12

Patches for a420 and a710. Simpler loader code and remove autodetected entries from stubs_entry_2.S and stubs_min.S

Thank you. Both cameras (A420 and A710) power on just fine with the new loader code.

Logged

Koshy had a little ELPH which wasn't white as snow but everywhere that Koshy went the ELPH was sure to go. (actually an SD, but that detail ruins the rhyme...)

nafraf

1308

Re: New, simpler version of the CHDK 'loader' code.

« Reply #18 on: 15 / December / 2014, 21:18:07 »

Thanks for testing. Patches for a560 and s3is.

a560_newloader_trunk3801.patch (16.19 kB - downloaded 23 times.)

s3is_newloader_trunk3801.patch (16.57 kB - downloaded 21 times.)

Logged

koshy

1096

Re: New, simpler version of the CHDK 'loader' code.

« Reply #19 on: 15 / December / 2014, 21:30:50 »

Quote from: nafraf on 15 / December / 2014, 21:18:07

Thanks for testing. Patches for a560 and s3is.

Thanks, both of those power on nicely, too. (S3IS and A560)
A560 is one of the three VX-Works cams that have the play push button and only using that to power on works. How to use the Pwr button is unresolved for these three. So that remains as it was.

Logged

Koshy had a little ELPH which wasn't white as snow but everywhere that Koshy went the ELPH was sure to go. (actually an SD, but that detail ruins the rhyme...)