A few design questions wrt the hso driver.

A few design questions wrt the hso driver.

[Denis Joseph Barrow]

Hi,
I'm currently beginning to test the modem functionality of the hso driver.

Simple question first.
There is this comment near the top of hso.c
Interface 2:	Standard modem interface - circuit switched interface, should
		not be used.

Who put this comment in & why? is it obselete?
The modem port at least works on minicom till the point
of setting up the ppp connection.

I'm not a modem guru
Also has anyone ideas on how to test the hso_serial_tiocmset
TIOCM_RTS TIOCM_DTR stuff.

My project lead Filip wants me to implement code to report
on DCD/RxCarrier DTR/TxCarrier to the kernel any ideas
where I might find an example of such stuff implemented
in the kernel & a means to test it.

Now a more involved question.
The suspend resume code in hso.c hso_get_activity &
hso_put_activity code to me looks very racy but the code seems to be working relatively reliably
because the schedule resume stuff happens at a slow pace. 
Despite the codes simplicity I do not have a good feel for whether it
is stable or not & don't feel like an authority on how to make the code better.

The more obvious possible issues I see with it the code are,
I could be wrong if I am please say so.
1) On smp systems there is a
workqueue for each cpu which means
that if one cpu workqueue is not going to be scheduled soon & the other 
workqueue is, if a suspend is queued on the cpu which is busy
& a resume is later queued on the cpu with soon to run workqueue
the resume will most likely happen before the suspend i.e. out of order.

2) Also only the schedule_work will only queue the
request once so if multiple schedule works happen
only the first one is accepted even if you wanted
to change the order of the suspend resume requests later on
they won't reorder.

I've also noticed a spurious crash twice near the top of hso_serial_close as
in the latest hso 1.6 driver shipped by option, I've been
unable to acertain the cause of this crash but suspect the suspend/resume
disconnect device reconnect device gremlins are at work here.

-- 
best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: A few design questions wrt the hso driver.

[Andrew Bird (Sphere Systems)]

Hi Denis,
	The 'should not be used' comment was in the original driver from Option. I 
think the point they were trying to make is that it's only to be used to make 
circuit switched connections, and if you want packet switched connections you 
should be using the network interface. As far as I know it actually functions 
OK in both scenarios.

Best regards,


Andrew

On Wednesday 01 October 2008 13:16, Denis Joseph Barrow wrote:
> Hi,
> I'm currently beginning to test the modem functionality of the hso driver.
>
> Simple question first.
> There is this comment near the top of hso.c
> Interface 2:	Standard modem interface - circuit switched interface, should
> 		not be used.
>
> Who put this comment in & why? is it obselete?
> The modem port at least works on minicom till the point
> of setting up the ppp connection.
>
> I'm not a modem guru
> Also has anyone ideas on how to test the hso_serial_tiocmset
> TIOCM_RTS TIOCM_DTR stuff.
>
> My project lead Filip wants me to implement code to report
> on DCD/RxCarrier DTR/TxCarrier to the kernel any ideas
> where I might find an example of such stuff implemented
> in the kernel & a means to test it.
>
> Now a more involved question.
> The suspend resume code in hso.c hso_get_activity &
> hso_put_activity code to me looks very racy but the code seems to be
> working relatively reliably because the schedule resume stuff happens at a
> slow pace.
> Despite the codes simplicity I do not have a good feel for whether it
> is stable or not & don't feel like an authority on how to make the code
> better.
>
> The more obvious possible issues I see with it the code are,
> I could be wrong if I am please say so.
> 1) On smp systems there is a
> workqueue for each cpu which means
> that if one cpu workqueue is not going to be scheduled soon & the other
> workqueue is, if a suspend is queued on the cpu which is busy
> & a resume is later queued on the cpu with soon to run workqueue
> the resume will most likely happen before the suspend i.e. out of order.
>
> 2) Also only the schedule_work will only queue the
> request once so if multiple schedule works happen
> only the first one is accepted even if you wanted
> to change the order of the suspend resume requests later on
> they won't reorder.
>
> I've also noticed a spurious crash twice near the top of hso_serial_close
> as in the latest hso 1.6 driver shipped by option, I've been
> unable to acertain the cause of this crash but suspect the suspend/resume
> disconnect device reconnect device gremlins are at work here.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: A few design questions wrt the hso driver.

[Paulius Zaleckas]

Denis Joseph Barrow wrote:
> Hi,
> I'm currently beginning to test the modem functionality of the hso driver.
> 
> Simple question first.
> There is this comment near the top of hso.c
> Interface 2:	Standard modem interface - circuit switched interface, should
> 		not be used.

AFAIK this interface is disabled in most 'hso' devices firmware.
Although it was enabled in some devices what made them work with 'option'
driver. Thats how some IDs got there :)

> 
> Who put this comment in & why? is it obselete?
> The modem port at least works on minicom till the point
> of setting up the ppp connection.

Re: A few design questions wrt the hso driver.

[Alan Stern]

On Wed, 1 Oct 2008, Denis Joseph Barrow wrote:

> Now a more involved question.
> The suspend resume code in hso.c hso_get_activity &
> hso_put_activity code to me looks very racy but the code seems to be working relatively reliably
> because the schedule resume stuff happens at a slow pace. 
> Despite the codes simplicity I do not have a good feel for whether it
> is stable or not & don't feel like an authority on how to make the code better.
> 
> The more obvious possible issues I see with it the code are,
> I could be wrong if I am please say so.
> 1) On smp systems there is a
> workqueue for each cpu which means
> that if one cpu workqueue is not going to be scheduled soon & the other 
> workqueue is, if a suspend is queued on the cpu which is busy
> & a resume is later queued on the cpu with soon to run workqueue
> the resume will most likely happen before the suspend i.e. out of order.
> 
> 2) Also only the schedule_work will only queue the
> request once so if multiple schedule works happen
> only the first one is accepted even if you wanted
> to change the order of the suspend resume requests later on
> they won't reorder.

I agree that the code looks very racy.  You may be able to improve it
by using the new infrastructure in this patch:

	http://marc.info/?l=linux-usb&m=122279021325200&w=2

It is intended specifically for handling asynchronous suspends and 
resumes.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: A few design questions wrt the hso driver.

[Denis Joseph Barrow]

Hi Alan,
Thank you good sir, I'll look into it.

Alan Stern wrote:
> On Wed, 1 Oct 2008, Denis Joseph Barrow wrote:
> 
>> Now a more involved question.
>> The suspend resume code in hso.c hso_get_activity &
>> hso_put_activity code to me looks very racy but the code seems to be working relatively reliably
>> because the schedule resume stuff happens at a slow pace. 
>> Despite the codes simplicity I do not have a good feel for whether it
>> is stable or not & don't feel like an authority on how to make the code better.
>>
>> The more obvious possible issues I see with it the code are,
>> I could be wrong if I am please say so.
>> 1) On smp systems there is a
>> workqueue for each cpu which means
>> that if one cpu workqueue is not going to be scheduled soon & the other 
>> workqueue is, if a suspend is queued on the cpu which is busy
>> & a resume is later queued on the cpu with soon to run workqueue
>> the resume will most likely happen before the suspend i.e. out of order.
>>
>> 2) Also only the schedule_work will only queue the
>> request once so if multiple schedule works happen
>> only the first one is accepted even if you wanted
>> to change the order of the suspend resume requests later on
>> they won't reorder.
> 
> I agree that the code looks very racy.  You may be able to improve it
> by using the new infrastructure in this patch:
> 
> 	http://marc.info/?l=linux-usb&m=122279021325200&w=2
> 
> It is intended specifically for handling asynchronous suspends and 
> resumes.
> 
> Alan Stern


-- 
best regards,
D.J. Barrow