usb/140242: dev/usb/controller/ehci_ix4xx.c shortcomings

[Sebastian Huber-4]

>Number:         140242
>Category:       usb
>Synopsis:       dev/usb/controller/ehci_ix4xx.c shortcomings
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-usb
>State:          open
>Quarter:        
>Keywords:      
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Nov 03 10:00:12 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Sebastian Huber
>Release:        9-current
>Organization:

embedded brains GmbH
>Environment:
>Description:
In ehci_ixp_attach() setting of sc->sc_bus.usbrev is superfluous since this is also set in ehci_init().

Calling ehci_reset() without a valid sc->sc_offs value makes no sense.
>How-To-Repeat:

>Fix:
Index: ehci_ixp4xx.c                                              
===================================================================
--- ehci_ixp4xx.c       (revision 198848)                          
+++ ehci_ixp4xx.c       (working copy)                            
@@ -157,8 +157,6 @@                                                
                return (ENOMEM);                                  
        }                                                          
                                                                   
-       sc->sc_bus.usbrev = USB_REV_2_0;                          
-                                                                  
        /* NB: hints fix the memory location and irq */            
                                                                   
        rid = 0;                                                  
@@ -230,7 +228,6 @@                                                
                     | EHCI_SCFLG_BIGEMMIO                        
                     | EHCI_SCFLG_NORESTERM                        
                     ;                                            
-       (void) ehci_reset(sc);                                    
                                                                   
        err = ehci_init(sc);                                      
        if (!err) {

>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
freebsd-usb@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."

[Hans Petter Selasky]

On Tuesday 03 November 2009 10:52:52 Sebastian Huber wrote:

> >Number:         140242
> >Category:       usb
> >Synopsis:       dev/usb/controller/ehci_ix4xx.c shortcomings
> >Confidential:   no
> >Severity:       non-critical
> >Priority:       low
> >Responsible:    freebsd-usb
> >State:          open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Tue Nov 03 10:00:12 UTC 2009
> >Closed-Date:
> >Last-Modified:
> >Originator:     Sebastian Huber
> >Release:        9-current
> >Organization:
>
> embedded brains GmbH
>
> >Environment:
> >Description:
>
> In ehci_ixp_attach() setting of sc->sc_bus.usbrev is superfluous since this
> is also set in ehci_init().
>
> Calling ehci_reset() without a valid sc->sc_offs value makes no sense.
>
> >How-To-Repeat:
> >
> >Fix:
>
> Index: ehci_ixp4xx.c
> ===================================================================
> --- ehci_ixp4xx.c       (revision 198848)
> +++ ehci_ixp4xx.c       (working copy)
> @@ -157,8 +157,6 @@
>                 return (ENOMEM);
>         }
>
> -       sc->sc_bus.usbrev = USB_REV_2_0;
> -
>         /* NB: hints fix the memory location and irq */
>
>         rid = 0;
> @@ -230,7 +228,6 @@
>
>                      | EHCI_SCFLG_BIGEMMIO
>                      | EHCI_SCFLG_NORESTERM
>
>                      ;
> -       (void) ehci_reset(sc);
>
>         err = ehci_init(sc);
>         if (!err) {
>
> >Release-Note:
> >Audit-Trail:
> >Unformatted:

Hi,

Thanks for your patch.

What is the correct "sc->sc_offs" value? The sc_offs field is used by more
than reset, and should be set correctly. I.E. The right solution is not to
remove the ehci_reset() call.

--HPS

_______________________________________________
freebsd-usb@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."

[Hans Petter Selasky]

On Tuesday 03 November 2009 10:52:52 Sebastian Huber wrote:

> >Number:         140242
> >Category:       usb
> >Synopsis:       dev/usb/controller/ehci_ix4xx.c shortcomings
> >Confidential:   no
> >Severity:       non-critical
> >Priority:       low
> >Responsible:    freebsd-usb
> >State:          open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Tue Nov 03 10:00:12 UTC 2009
> >Closed-Date:
> >Last-Modified:
> >Originator:     Sebastian Huber
> >Release:        9-current
> >Organization:
>
> embedded brains GmbH
>
> >Environment:
> >Description:
>
> In ehci_ixp_attach() setting of sc->sc_bus.usbrev is superfluous since this
> is also set in ehci_init().
>
> Calling ehci_reset() without a valid sc->sc_offs value makes no sense.
>
> >How-To-Repeat:
> >
> >Fix:
>
> Index: ehci_ixp4xx.c
> ===================================================================
> --- ehci_ixp4xx.c       (revision 198848)
> +++ ehci_ixp4xx.c       (working copy)
> @@ -157,8 +157,6 @@
>                 return (ENOMEM);
>         }
>
> -       sc->sc_bus.usbrev = USB_REV_2_0;
> -
>         /* NB: hints fix the memory location and irq */
>
>         rid = 0;
> @@ -230,7 +228,6 @@
>
>                      | EHCI_SCFLG_BIGEMMIO
>                      | EHCI_SCFLG_NORESTERM
>
>                      ;
> -       (void) ehci_reset(sc);
>
>         err = ehci_init(sc);
>         if (!err) {
>
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
>

Committed to USB P4:

http://p4web.freebsd.org/chv.cgi?CH=170109

--HPS

_______________________________________________
freebsd-usb@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."