Re: ucom(4) hang up

[KIYOHARA Takashi <kiyohara%kk.iij4u.or.jp@localhost>]

Hi! Steve,


From: Steve Woodford <scw%NetBSD.org@localhost>
Date: Fri, 19 Feb 2010 14:29:14 +0000

> On Friday 19 February 2010 12:56:25 KIYOHARA Takashi wrote:
> 
> > I understood this problem.
> > uftdi(4) might receive state information only, not the data part.
> > uftdi_read() will return cp+2 and set 0 to cc.
> > And, neither uc_index nor uc_len never become the same though
> > ucomreadcb() calls ucom_read_complete().
> 
> Thanks for the analysis. Can you test if the patch, below, fixes the 
> problem?

Oops,  your patch not clear this problem.
If uftdi receives three data (msr, lsr, data), uftdi_read() will be
set to cp+2 and cc=1. In this case, ucom_read_complete() will never
call rint().

> @@ -1167,7 +1167,7 @@
>  
>               splx(s);
>  
> -             if (ub->ub_index == ub->ub_len) {
> +             if (ub->ub_index >= ub->ub_len) {
>                       SIMPLEQ_REMOVE_HEAD(&sc->sc_ibuff_full, ub_link);
>  
>                       ucomsubmitread(sc, ub);


I propose two methods. We should choose either.

1. sc_methods->ucom_read() returns the data total count received to
   variable cc.  If 'ub_index < ub_len', ucom_read_complete() calls
   rint.

Index: uftdi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uftdi.c,v
retrieving revision 1.45
diff -u -r1.45 uftdi.c
--- uftdi.c     18 Jan 2010 20:57:13 -0000      1.45
+++ uftdi.c     20 Feb 2010 06:36:33 -0000
@@ -428,7 +429,6 @@

        /* Pick up status and adjust data part. */
        *ptr += 2;
-       *count -= 2;
 }

 Static void


2. If 'ub_len > 0', calls rint() in ucom_read_complete().  Perhaps,
   this is equal to ucom before 1.81.

Index: ucom.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ucom.c,v
retrieving revision 1.82
diff -u -r1.82 ucom.c
--- ucom.c      6 Jan 2010 20:37:56 -0000       1.82
+++ ucom.c      20 Feb 2010 06:56:44 -0000
@@ -1156,18 +1156,21 @@

                s = spltty();

-               while (ub->ub_index < ub->ub_len && !sc->sc_rx_stopped) {
+               while (ub->ub_len > 0 && !sc->sc_rx_stopped) {
                        /* Give characters to tty layer. */
                        if ((*rint)(ub->ub_data[ub->ub_index], tp) == -1) {
                                /* Overflow: drop remainder */
-                               ub->ub_index = ub->ub_len;
-                       } else
+                               ub->ub_index += ub->ub_len;
+                               ub->ub_len = 0;
+                       } else {
                                ub->ub_index++;
+                               ub->ub_len--;
+                       }
                }

                splx(s);

-               if (ub->ub_index == ub->ub_len) {
+               if (ub->ub_len == 0) {
                        SIMPLEQ_REMOVE_HEAD(&sc->sc_ibuff_full, ub_link);

                        ucomsubmitread(sc, ub);


I recommend the latter.

Thanks,
--
kiyohara