>> On Fri, Jan 17 2014, Robert Baldyga wrote:
>>> + case FFS_ACTIVE:
>>> + switch (FFS_SETUP_STATE(ffs)) {
>>> + case FFS_NO_SETUP:
>>> + if (ffs->ev.count)
>>> + mask |= POLLIN;
>>> + break;
>>> +
>>> + case FFS_SETUP_PENDING:
>>> + mask |= (POLLIN | POLLOUT);
>>> + break;
>>> +
>>> + case FFS_SETUP_CANCELED:
> On 01/17/2014 12:20 PM, Michal Nazarewicz wrote:
>> The FFS_SETUP_CANCELED and FFS_SETUP_PENDING are the same case. ep0
>> never recovers from FFS_SETUP_CANCELED on its own, it waits for user
>> space to attempt to read or write something. So unless I'm missing
>> something the two cases should be merged together and in both POLLIN and
>> POLLOUT needs to be set.
On Fri, Jan 17 2014, Robert Baldyga wrote:
> Hmm, it seems like it doesn't work this way. In both read and write
> functions in ep0 file operations, there is check at the beginning:
>
> if (FFS_SETUP_STATE(ffs) == FFS_SETUP_CANCELED)
> return -EIDRM;
>
> So read/write does nothing when FFS_SETUP_CANCELED state is set.
FFS_SETUP_STATE is defined as:
#define FFS_SETUP_STATE(ffs) \
((enum ffs_setup_state)cmpxchg(&(ffs)->setup_state, \
FFS_SETUP_CANCELED, FFS_NO_SETUP))
So what the if condition does is:
state = ffs->setup_state;
if (state == FFS_SETUP_CANCELED) {
ffs->setup_state = FFS_NO_SETUP;
return -EIDRM;
}
Perhaps the macro should be renamed (and changed to static inline).
Plus now that I look at it, it requires memory barriers. I'll prepare
a patch for that.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature
