On 10/07/2014 12:07 PM, Michal Nazarewicz wrote:
> On Tue, Oct 07 2014, Robert Baldyga <r.bald...@samsung.com> wrote:
>> @@ -1411,8 +1425,17 @@ static void ffs_data_closed(struct ffs_data *ffs)
>>      ENTER();
>>  
>>      if (atomic_dec_and_test(&ffs->opened)) {
>> -            ffs->state = FFS_CLOSING;
>> -            ffs_data_reset(ffs);
>> +            if (ffs->zombie_mode) {
>> +                    ffs->state = FFS_ZOMBIE;
>> +                    if (ffs->epfiles)
>> +                            ffs_epfiles_delete(ffs->epfiles,
>> +                                               ffs->eps_count);
> 
> This looks suspicious.  Why isn't it:
> 
> +                     if (ffs->epfiles) {
> +                             ffs_epfiles_destroy(ffs->epfiles,
> +                                                ffs->eps_count);
> +                             ffs->epfiles = NULL;
> +                     }
> 
> If ffs->epfiles is not NULLed, call to ffs_data_reset in ffs_data_open
> will call ffs_epfiles_destroy which we don't want, do we?
> 

We do. When epfile->dentry == NULL, ffs_epfiles_destroy() will do only
kfree(epfiles). We want to do it.

We don't want to have ffs->epfiles being equal NULL before calling
ffs_func_eps_disable(), which iterates on this table.

Hmm, we could also change ffs_func_eps_disable() to not touch
ffs->epfiles if it's NULL. It seems to be better solution.

>> +                    if (ffs->setup_state == FFS_SETUP_PENDING)
>> +                            __ffs_ep0_stall(ffs);
>> +            } else {
>> +                    ffs->state = FFS_CLOSING;
>> +                    ffs_data_reset(ffs);
>> +            }
>>      }
>>  
>>      ffs_data_put(ffs);
> 
>> @@ -93,6 +93,26 @@ enum ffs_state {
>>      FFS_ACTIVE,
>>  
>>      /*
>> +     * Function is visible to host, but it's not functional. All
>> +     * setup requests are stalled and another transfers are refused.
> 
> “and transfers on other endpoints are refused.”
> 
>> +     * All epfiles, excepting ep0, are deleted so there is no way
> 
> s/excepting/except/
> 
>> +     * to perform any operations on them.
>> +     *
>> +     * This state is set after closing all functionfs files, when
>> +     * mount parameter "zombie=1" has been set. Function will remain
>> +     * in zombie state until filesystem will be umounted or ep0 will
> 
> s/will be umounted/is unmounted/
> s/ep0 will be/ep0 is/
> 
>> +     * be opened again. In the second case functionfs state will be
>> +     * reseted, and it will be ready for descriptors and strings
> 
> s/reseted/reset/
> 
>> +     * writing.
>> +     *
>> +     * This is useful only when functionfs is composed to gadget
>> +     * with another function which can perform some critical
>> +     * operations, and it's strongly desired to have this operations
>> +     * completed, even after functionfs files closure.
>> +     */
>> +    FFS_ZOMBIE,
>> +
>> +    /*
>>       * All endpoints have been closed.  This state is also set if
>>       * we encounter an unrecoverable error.  The only
>>       * unrecoverable error is situation when after reading strings
> 

Thanks,
Robert Baldyga
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to