Making kbd.c code generic

reyalp

13880

Re: Making kbd.c code generic

« Reply #10 on: 23 / February / 2015, 23:27:19 »

Advertisements

Quote from: srsa_4c on 23 / February / 2015, 19:28:26

This wasn't my invention, it's just how the ixus40 port was done. The firmware code uses system timers, and is quite different from anything that came later, so it's impossible to make it look/work like the usual keyboard code.

Yes, I think ixus30 and ixus40 are going to need pretty much stay this way and not use the common code, except maybe the press/release etc.

Quote

Quote
a470 has a weird hack involving the power button
+ same hack in ixus110, I'm the one who's responsible... Needed the extra button for CHDK.
Perhaps it can be replaced by something generic? Note that this is using an inverted button on the a470.

Thanks for pointing out the ixus110.

The trend seems to be toward less buttons so it might be nice to have generic, but in the meantime these can with KBD_CUSTOM_UPDATE_KEY_STATE

Having a handful of unique kbd.c files is still going to be a lot better than ~130.

Logged

Don't forget what the H stands for.

reyalp

13880

Re: Making kbd.c code generic

« Reply #11 on: 28 / February / 2015, 19:49:45 »

I think this could start to be integrated in the trunk. If anyone thinks this isn't a good idea, or that major changes in the approach are needed, please let me know.

My approach would be:
For the initial checkin, all unconverted cameras would be set to use KBD_CUSTOM_ALL i.e. use their own kbd.c and ignore the current generic code. This would be temporary except for the ones (like ixus30) that need radically different kbd.c code.

Start converting the don't have any special weirdness blind, as well as cameras that have testers available. A list of which cameras haven't been tested should be kept.

Additional generic code for e.g. inverted key logic, the extra button hack, can be added later.

some other random notes:
* GetKbdState is found by the sig finder on many cams now, and can be used instead of reading MMIOs directly.
* KEY_DUMMY will be removed
* MALLOCD_STACK for vxworks kbd task was never used and can be removed
* Using C code in the vxworks naked mykbd_task is suspect
* DryOS on cams that start kbd_task directly can merge mykbd_task and mykbd_task_proceed (see D10 etc in patch)
* alt_mode_key_mask can probably be removed at some point
* Many kbd.c assume USB_IDX == SD_READONLY_IDX

Logged

Don't forget what the H stands for.

nafraf

1308

Re: Making kbd.c code generic

« Reply #12 on: 28 / February / 2015, 21:33:50 »

kbd-common-work-4.patch tested on A495. Keys tested with ubtest.bas, keys works without problems.

Logged

philmoz

3445

Re: Making kbd.c code generic

« Reply #13 on: 02 / March / 2015, 16:12:32 »

Quick test on the G12, no issues.

When I get a few minutes I'll do the IXUS310 touchscreen version.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

reyalp

13880

Re: Making kbd.c code generic

« Reply #14 on: 02 / March / 2015, 16:50:06 »

Quote from: philmoz on 02 / March / 2015, 16:12:32

Quick test on the G12, no issues.

When I get a few minutes I'll do the IXUS310 touchscreen version.

Thanks.
Eventually, I'd like make more of the touch stuff common, but it doesn't have to happen in the first pass.

Quote from: reyalp on 22 / February / 2015, 20:22:10

There are two common styles of using _GetKbdState (aka kbd_fetch_data) and _kbd_read_keys_r2.

On cameras like d10:
kbd_fetch_data into new_state
run kbd_process (all chdk kbd task code)
copy into physw_status, fiddling key bits as required
_kbd_read_keys_r2, into physw_status
fiddle USB and SD read-only bits.

On cameras like sx160:
kbd_fetch_data into new_state
_kbd_read_keys_r2, into new_state
run kbd_process (all chdk kbd task code)
copy into physw_status, fiddling key bits as required
fiddle USB and SD read-only bits.

FWIW, I converted ixus140 and d10 to the same style as sx160, using the sigfinder _GetKbdState and saw no issues or change in behavior.

I see a comment in g12 which probably explains the origin of this variant.

Code: [Select]

//_kbd_read_keys_r2(physw_status); // re-reads physw_status[0] from 0x2DE4 at start (so above doesn't work properly) !!!!!

Logged

Don't forget what the H stands for.

philmoz

3445

Re: Making kbd.c code generic

« Reply #15 on: 03 / March / 2015, 03:34:47 »

Updated patch with G12 and IXUS310 added.

I have added an extra short to KeyMap for the IXUS310 that I use to set the font scale factor - I use the symbols from the font for up/down/left/right instead of text; but scaled up to fill the button better.

I got this from waterwingz original N port; but this seems to have been removed in that version now.

Phil.

kbd-common-work-5.patch (69.04 kB - downloaded 27 times.)

Logged

srsa_4c

4451

Re: Making kbd.c code generic

« Reply #16 on: 03 / March / 2015, 19:37:41 »

Quote from: reyalp on 23 / February / 2015, 23:27:19

Thanks for pointing out the ixus110.

The trend seems to be toward less buttons so it might be nice to have generic, but in the meantime these can with KBD_CUSTOM_UPDATE_KEY_STATE

I managed to convert this one without KBD_CUSTOM_UPDATE_KEY_STATE (attached, I've been using the v4 patch). The "fake key" functionality is still working.

A side note: no matter what I try, the toolchain just doesn't want to emit correct code for mykbd_task(). Adding/removing the naked and noinline attributes makes no difference.
I know that this task is unlikely to ever exit and even if it did, ExitTask would probably not return, but it just isn't right.
edit: disregard the above, just checked, _ExitTask is declared as noreturn in lolevel.h

Code: [Select]

03b9b7cc <mykbd_task>:
 3b9b7cc: e59f3028  ldr r3, [pc, #40] ; 3b9b7fc <mykbd_task+0x30>
 3b9b7d0: e5933000  ldr r3, [r3]
 3b9b7d4: e3530000  cmp r3, #0
 3b9b7d8: 0a000006  beq 3b9b7f8 <mykbd_task+0x2c>
 3b9b7dc: e3a0000a  mov r0, #10
 3b9b7e0: ebffb1a6  bl 3b87e80 <_SleepTask>
 3b9b7e4: ebfffff3  bl 3b9b7b8 <wrap_kbd_p1_f>
 3b9b7e8: e3500001  cmp r0, #1
 3b9b7ec: 1afffff6  bne 3b9b7cc <mykbd_task>
 3b9b7f0: ebffb1be  bl 3b87ef0 <_kbd_p2_f>
 3b9b7f4: eafffff4  b 3b9b7cc <mykbd_task>
 3b9b7f8: ebffb161  bl 3b87d84 <_ExitTask>
 3b9b7fc: 00001c34  .word 0x00001c34

ixus110_kbdcommonv4.diff (16.35 kB - downloaded 22 times.)

« Last Edit: 03 / March / 2015, 19:53:35 by srsa_4c »

Logged

reyalp

13880

Re: Making kbd.c code generic

« Reply #17 on: 07 / March / 2015, 18:10:25 »

Thanks for both of those. I've attached an updated combined patch.

Regarding CAM_TOUCHSCREEN_SYMBOLS, all the touch cameras should probably be converted to use symbols, but this doesn't need to be done right away.

Regarding the ixus110, I have reservations about having a variable in the mask define, but again this can be changed later if needed.

Unless there are objections, I will start putting this in the trunk over the weekend.

kbd-common-work-6.patch (88.82 kB - downloaded 26 times.)

Logged

Don't forget what the H stands for.

reyalp

13880

Re: Making kbd.c code generic

« Reply #18 on: 08 / March / 2015, 17:42:29 »

I've checked in the initial code in trunk 4046

I have also started a wiki page for requests to test blind changes: http://chdk.wikia.com/wiki/Testing_Needed
I'll add a section for kbd.c once I start converting more cameras.

Logged

Don't forget what the H stands for.

waterwingz

12132

Re: Making kbd.c code generic

« Reply #19 on: 11 / March / 2015, 20:37:00 »

A560 tested successfully. Wiki page updated.

Logged

Ported : A1200 SD940 G10 Powershot N G16