Benoît Audouard wrote : | Sorry ludovic, neither Sl33p3r, nor I, had time to have a look at your | patch yesterday.
OK. I just ended looking at it .. Here are basically my remarks on the
patch:
u_main.c:
/* Add LUA */
init_waitqueue_head (&ins->sync_q2);
ins->AdiApiRWPending = FALSE;
ins->APICmvError = FALSE;
ins->ApiTestReq=FALSE;
ins->GPRegister = 0;
init_timer(&ins->BootTimer);
/**/
We do not in fact need any of these modifs:
- sync_q2 waitqueue isn't used (we never put any process on this queue)
- APICmvError: just set, never used.
- ApiTestReq : just set, never used.
- GPRegister: set to 0. Tested against bit 0 set or not (thus
always FALSE)
- BootTimer: Ti,er is just initialized, never used.
eu_msg.c:
- eu_user_send_msg : useless. Used only in Me.c, in the case where a
CMV has been put in the "queue" with Type == 1... But this type is
only set in eu_user_send_msg function, which is then never called.
spin_lock(&ins->lock);
OutboundPending = ins->AdiModemSm.OutboundPending;
if ( (OutboundPending==FALSE) && (ins->AdiApiRWPending==FALSE) )
ins->AdiModemSm.OutboundPending = TRUE;
spin_unlock(&ins->lock);
/*
* if no message pending, its safe to send another one
*/
if ( (OutboundPending == FALSE) && (ins->AdiApiRWPending==FALSE) )
...
This is just a serialization of the CMVs sent to the modem : we
wait for the modem to return something before sending another
CMV. Not sure if it's usefull.
#define ADDMSG_FROM_TEXTFILE(Seq,Msg)
This is IMHO the bug which was causing the CMVs to not function. In
CVS I updated the eu_build_msg to be able to treat the case dest == src.
Me.c
In ProcessIncomingCmv,
if ( (ins->AdiApiRWPending) && (
(Msg.type==MP_FUNCTION_TYPE_MEMACCESS)
....
Useless: AdiApiRWPending is always FALSE (as eu_userusend_msg is
never called)
Not sure if the part that wait for a modem response is usefull..
Sm.c
STATE_DISABLE state is useless : never set as GPRegister is == 0, and
thus the following code is never entered:
if (ins->GPRegister & DISABLE_STATE_MACHINE)
/**/
{
/**/
ins->PrevState = tmpState;
/**/
tmpState = pAdiSM->CurrentAdiState = STATE_DISABLE;
/**/
}
/**/
So to sum-up, I think the bug was due to the bad handling of the src ==
dst case in eu_build_msg. I fixed this in CVS, but for now, it's the
only modification I've done (it's late now :( ). Some other will find
their way to the CVS ( some case in th estate machine, and perhaps the
serialization of the CMVs), but I do not have ti,e *right* now ...
Ludovic, would it be possible to test again againts the last CVS version?
--
Frederick Ros aka Sleeper
Watch out for off-by-one errors.
- The Elements of Programming Style (Kernighan & Plaugher)
pgp5nriApZxuq.pgp
Description: PGP signature
