Re: USB port numbering

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Thu, 18 Oct 2018 02:45:35 +0200
    From:        manu%netbsd.org@localhost (Emmanuel Dreyfus)
    Message-ID:  <1nwuhrv.17wehqe12fccjvM%manu%netbsd.org@localhost>


  | --- sys/dev/usb/uhub.c  27 Sep 2018 14:52:26 -0000      1.136.2.2
  | +++ sys/dev/usb/uhub.c  18 Oct 2018 00:32:58 -0000
  | @@ -821,10 +821,10 @@
  |         /* XXXSMP usb */
  |         KERNEL_LOCK(1, curlwp);
  |  
  |         nports = hub->uh_hubdesc.bNbrPorts;
  | -       for (port = 0; port < nports; port++) {
  | -               rup = &hub->uh_ports[port];
  | +       for (port = 1; port <= nports; port++) {
  | +               rup = &hub->uh_ports[port - 1];
  |                 if (rup->up_dev == NULL)
  |                         continue;
  |                 if ((rc = usb_disconnect_port(rup, self, flags)) != 0) {
  |                         /* XXXSMP usb */

That part of your change is puerly cosmetic, and affects nothing (or shouldn't)
except might generate fractionally less effeicient code (it is normally 
cheaper to start at 0, than 1, it is an easier const to generate on many 
arch's) - though the compiler might optimise that difference away.
0 base indexing is also more traditional C and more in line with the way
most of us (here) think, I suspect.

  | @@ -869,12 +869,13 @@
  |         struct usbd_hub *hub = sc->sc_hub->ud_hub;
  |         struct usbd_device *dev;
  |         int port;
  |  
  | -       for (port = 0; port < hub->uh_hubdesc.bNbrPorts; port++) {
  | -               dev = hub->uh_ports[port].up_dev;
  | +       for (port = 1; port <= hub->uh_hubdesc.bNbrPorts; port++) {
  | +               dev = hub->uh_ports[port - 1].up_dev;
  |                 if (dev == NULL)
  |                         continue;
  | +printf("uhub_rescan > usbd_reattach_device port %d\n", port);
  |                 usbd_reattach_device(sc->sc_dev, dev, port, locators);
  |         }
  |         return 0;
  |  }

That one is where the difference is, but a simpler change would be
to pass port+1 to usbd_reattach_device() rather than port, to make
the numbering consistent, but this one does not matter so much
and if other code uses base 1 port numbering then doing it the way
you did should be fine ... and since that is what uhub_explore()
does, perhaps it is the best way here.

[Aside: obviously the debug printf should go away!]

  | @@ -894,10 +895,10 @@
  |                 /* should never happen; children are only created after
  | init */
  |                 panic("hub not fully initialised, but child deleted?");
  |  
  |         nports = devhub->ud_hub->uh_hubdesc.bNbrPorts;
  | -       for (port = 0; port < nports; port++) {
  | -               dev = devhub->ud_hub->uh_ports[port].up_dev;
  | +       for (port = 1; port <= nports; port++) {
  | +               dev = devhub->ud_hub->uh_ports[port - 1].up_dev;
  |                 if (!dev || dev->ud_subdevlen == 0)
  |                         continue;
  |                 for (i = 0; i < dev->ud_subdevlen; i++) {
  |                         if (dev->ud_subdevs[i] == child) {

This one is like the first, puerly cosmetic.    That is, in both, "port"
is used only as the loop variable, and to use as the array index to
get the pointer, which is what is actually used.

kre