| Summary: | New driver for AuthenTech AES3500 | ||
|---|---|---|---|
| Product: | libfprint | Reporter: | Juvenn Woo <Machese> |
| Component: | libfprint | Assignee: | Juvenn Woo <Machese> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | medium | CC: | anarsoul, Machese |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
git formatted diff against commit 7eafca7
lsusb output Add standalone aes3500 Extract common routines AES3K from AES3500 and AES4000 Extract common routines from aes4000 Add aes3500 support Cleanup macros and comments for aes3k drivers 0001 aes3k: extract common routines aes3k from aes4000 0002 aes3k: add aes3500 driver support |
||
|
Description
Juvenn Woo
2013-05-08 08:45:06 UTC
Created attachment 79018 [details]
lsusb output
There're a few differences between aes4000 and aes3500, could you extend aes4000 to support aes3500 devices? Btw, please use git format-patch next time (In reply to comment #2) > There're a few differences between aes4000 and aes3500, could you extend > aes4000 to support aes3500 devices? A driver to support both devices? I'm not sure, but I may not be experienced enough to do it. Will look into it, anyway. > > Btw, please use git format-patch next time Will do so, thanks. Take a look at aesx660.c, aes1660.c and aes2660.c aesx660.c is common routines for aes1660 and aes2660 devices, and device-specific code is in aes1660.c and aes2660.c (In reply to comment #4) > Take a look at aesx660.c, aes1660.c and aes2660.c > > aesx660.c is common routines for aes1660 and aes2660 devices, and > device-specific code is in aes1660.c and aes2660.c Thanks Vasily, for pointing me out the way. I'll figure out some time to do it. Created attachment 79392 [details] [review] Add standalone aes3500 Created attachment 79393 [details] [review] Extract common routines AES3K from AES3500 and AES4000 (In reply to comment #4) > Take a look at aesx660.c, aes1660.c and aes2660.c > > aesx660.c is common routines for aes1660 and aes2660 devices, and > device-specific code is in aes1660.c and aes2660.c Hi Vasily, Following your approach, I've extracted out common routines as aes3k, and with differences left in aes3500 and aes4000, respctively. Please have a look, and tell me if anything's not good yet. Thanks! (In reply to comment #8) > Hi Vasily, > > Following your approach, I've extracted out common routines as aes3k, and > with differences left in aes3500 and aes4000, respctively. > > Please have a look, and tell me if anything's not good yet. Thanks! Hi Juvenn, please create 2 patches: 1st to extract common routines from aes4k, 2nd to add aes3k device support I know that it's additional work, but it does not look clean to add code in 1st patch that will be removed in 2nd, it's redundant. Thanks! Created attachment 79398 [details] [review] Extract common routines from aes4000 Created attachment 79399 [details] [review] Add aes3500 support Hi Vasily, Just prepared new patches as you've outlined. Your feedback are welcome. Thanks! Comment on attachment 79398 [details] [review] Extract common routines from aes4000 Review of attachment 79398 [details] [review]: ----------------------------------------------------------------- ::: libfprint/drivers/aes4000.c @@ +42,2 @@ > > +// FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Substitute it as a macro value, or remove this comment. And don't use c99 comments, use multiline Comment on attachment 79399 [details] [review] Add aes3500 support Review of attachment 79399 [details] [review]: ----------------------------------------------------------------- Looks good Created attachment 79452 [details] [review] Cleanup macros and comments for aes3k drivers Hi Vasily, Added this new patch to cleanup the macros and comments. Please review. Thanks! Hi Vasily, Were you notified the new patch to cleanup the macros and comments? Let me know what you think. Thanks! Comment on attachment 79452 [details] [review] Cleanup macros and comments for aes3k drivers Review of attachment 79452 [details] [review]: ----------------------------------------------------------------- Please fixup 1st and 2nd patches, don't create another one. You can use git interactive rebase (git rebase -i) to apply fixup against earlier commits. See also some comments: ::: libfprint/drivers/aes3500.c @@ +46,2 @@ > #define FRAME_WIDTH 128 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Please use brackets around macro value ::: libfprint/drivers/aes4000.c @@ +41,4 @@ > > +/* image size = FRAME_WIDTH x FRAME_WIDTH */ > +#define FRAME_WIDTH 96 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Same as above @@ +41,5 @@ > > +/* image size = FRAME_WIDTH x FRAME_WIDTH */ > +#define FRAME_WIDTH 96 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 > +#define FRAME_NUMBER FRAME_WIDTH / AES3K_FRAME_HEIGHT Same as above Created attachment 79604 [details] [review] 0001 aes3k: extract common routines aes3k from aes4000 Created attachment 79605 [details] [review] 0002 aes3k: add aes3500 driver support Hi Vasily, Newly built 2 patches attached, please have a review. And thanks for guiding me through to produce good code, this is my first C project. Anything that's not good enough, please let me konw. I'd be glad to fix them. Best, Comment on attachment 79604 [details] [review] 0001 aes3k: extract common routines aes3k from aes4000 Review of attachment 79604 [details] [review]: ----------------------------------------------------------------- Looks OK Comment on attachment 79605 [details] [review] 0002 aes3k: add aes3500 driver support Review of attachment 79605 [details] [review]: ----------------------------------------------------------------- Looks OK Merged into libfprint master |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.