And no one waits for this ssm (and async USB reqs) to complete here. It's a race condition I've described earlier. Please move device initialization into dev_activate().
How do you ensure that _all_ transfers are completed now? fpi_imgdev_deactivate_complete should be called only after deactivation has been completed. For this driver "cancel" button in fprint_demo won't work.
Comment on attachment 82310 Add-VFS5011- driver. patch
0001-lib-
Review of attachment 82310: ------- ------- ------- ------- ------- ------- ------- ------- --
-------
::: libfprint/ drivers/ vfs5011. c
@@ +33,5 @@
> + }
> + fp_dbg(s);
> +#endif
> +}
> +
Please drop all debugging stuff from driver. Only debug logs via fp_dbg are allowed.
@@ +321,5 @@ size*sizeof( int)); filtersize* sizeof( int));
> +static void median_filter(int *data, int size, int filtersize)
> +{
> + int i;
> + int *result = (int *)malloc(
> + int *sortbuf = (int *)malloc(
Please use g_malloc or g_malloc0 here.
@@ +329,5 @@ 1)*sizeof( int));
> + if (i1 < 0)
> + i1 = 0;
> + if (i2 >= size)
> + i2 = size-1;
> + memmove(sortbuf, data+i1, (i2-i1+
g_memmove()
@@ +330,5 @@ 1)*sizeof( int));
> + i1 = 0;
> + if (i2 >= size)
> + i2 = size-1;
> + memmove(sortbuf, data+i1, (i2-i1+
> + qsort(sortbuf, i2-i1+1, sizeof(int), cmpint);
I wonder if there's glib wrapper for that one...
@@ +364,5 @@ input_lines * sizeof(int));
> + };
> + int i;
> + float y = 0.0;
> + int line_ind = 0;
> + int *offsets = (int *)malloc(
g_malloc()
@@ +372,5 @@
> +
> + if (debugpath != NULL) {
> + sprintf(name, "%s/offsets%d.dat", debugpath, (int)time(NULL));
> + debugfile = fopen(name, "wb");
> + }
Drop debug stuff except logs
@@ +945,5 @@ elements( vfs5011_ initialization) ; sequence. actions = vfs5011_ initialization; sequence. device = dev; sequence. receive_ buf = VFS5011_ RECEIVE_ BUF_SIZE) ;
> + array_n_
> + data->init_
> + data->init_
> + data->init_
> + malloc(
g_malloc()
@@ +989,5 @@ reset_device( dev->udev) ;
> + int r = libusb_
> + if (r != 0) {
> + fp_err("Failed to reset the device");
> + return r;
> + }
Is it necessary? Btw, doc says that handle may be invalid after reset, and you're reusing same handle later.
@@ +1000,5 @@ new(dev- >dev, open_loop, DEV_OPEN_ NUM_STATES) ; complete) ;
> +
> + struct fpi_ssm *ssm;
> + ssm = fpi_ssm_
> + ssm->priv = dev;
> + fpi_ssm_start(ssm, open_loop_
And no one waits for this ssm (and async USB reqs) to complete here. It's a race condition I've described earlier. Please move device initialization into dev_activate().
@@ +1035,5 @@ struct fp_img_dev *dev) deactivate_ complete( dev);
> +}
> +
> +static void dev_deactivate(
> +{
> + fpi_imgdev_
How do you ensure that _all_ transfers are completed now? fpi_imgdev_ deactivate_ complete should be called only after deactivation has been completed. For this driver "cancel" button in fprint_demo won't work.