David:
You last message in this thread gave me a lot to think about. I will
reply to a few items in context, then discuss the rest separately.
On Wed, 4 Jun 2003, David Brownell wrote:
> Have a look at the EZ-USB doc to see the details, which
> I fudged a bit. It initiates re-enumeration, I forget
> exactly how, and clearly can't lose power since that'd
> mean the firmware got lost. (It's all detailed in one
> of the earlier chapters in the technical reference manual,
> which is a big PDF doc you can download from Cypress.)
I'll check it out at some point. But it still sounds strange. For the
time being, let's try to ignore it.
> > I would argue that usb_set_configuration() doesn't yet behave properly
> > outside the stage you mention.
>
> I know for a fact that "previously" it didn't, and that even
> in the current code the sysfs files are still handled wrong.
> Were there other issues you're thinking of? The main open
> issue I'm aware of is handling the interfaces in sysfs.
That's what I meant. From the standpoint of a device-driver writer it's
a pretty big issue, since sysfs does the actual calls to probe() and
disconnect().
> So implement the hub's work_struct logic correctly. It's
> clearly simpler if only that one task is handling this stuff.
To tell the truth, I don't care many threads handle this stuff or how the
proper ordering is maintained. It's an "implementation detail".
> > Why must it be used? In thinking it over, I don't see anything bad about
> > submitting URBs at any time, except after the struct usb_device is gone,
> > which will never happen. Do the host drivers maintain lists of currently
> > available endpoints, and will they get upset if the list changes while an
> > URB is in flight?
>
> Yes, the HCDs have lists of currently active endpoints;
> in fact, the HC may be using them. Yes, changing them
> needs to be sycnchronized with usbcore, because things
> can break rudely if something smashes that hardware state.
> (Only UHCI is at all forgiving in that respect.)
So when a reset or config change occurs, we will have to go through the
whole process of disabling the endpoints, terminating any active URBs for
those endpoints, and refusing to accept any new ones? That's pretty much
already written, IIRC.
> >>If we take the simple and "obviously correct" path of disconnecting
> >>all drivers along the reset paths, we can allow reset completion
> >>handlers to re-bind before we kick in the sysfs code that manages
> >>other binding logic.
> >
> >
> > This doesn't make sense. The only way to disconnect a driver is by
> > calling its disconnect() routine, and when that happens the driver will
> > erase all its saved state concerning the device. There won't be anything
> > left to re-bind reset completion handlers or anything else to.
>
> Now you're talking about a different issue: drivers re-using
> state. It's why I mentioned re-using the original usbcore pointers
> on these reset paths, until we know it's impossible (because the
> descriptors changed). It's not the best handle on the device
> (since pointers get re-used), but it's one option.
It's not a different issue. How can you talk about allowing reset
completion handlers to re-bind when, for example, as a result of the
disconnection the driver may have been rmmod'ed? There's no way
(currently, at least) for a driver to know that it's being unbound
temporarily for a reset rather than permanently.
> I think saving the old driver state should be the responsibility
> of the driver that calls usb_reset_device(), not usbcore.
This is the same issue. What about other drivers bound to the device?
They have no way to know the reason they are being unbound. And to
re-bind them later you would have to call their probe() routine; how do
you tell them that this is a re-binding rather than a new device?
For that matter, why go to the trouble of unbinding the drivers at all?
Wouldn't it be enough just to refuse the URBs they submit? Especially if
there was a callback you could use to ask them to stop submitting for a
while?
> The whole model of a "hard" reset that retains driver state
> has always bothered me. I don't know how we ended up with
> it ... in particular, did anyone even explore the notion of
> a "soft" reset, like set_config(0) then set_config(previous)?
> Normally the whole point of a "hard" reset is to re-init.
>
> We only use that "hard" reset in usb-storage; one of the
> SCSI-ish scanner drivers, where there's a "FIXME untested"
> comment; and usbfs, with 'err("this function is broken")'
> in the runtime ... and we get a lot of problems when
> usb-storage runs down those paths, too.
This is a very important point. As far as I know, the reset in
usb-storage was "inherited" from SCSI, as a last-ditch way of trying to
put a device back into working order. It could be removed entirely, but
it would be a shame to do that if there were any cases where it could
genuinely do some good. I don't recall seeing any, largely because it
hasn't worked well enough in the past to have a chance of helping anyone.
Maybe that will change now.
I don't believe there's any real reason that usb-storage's reset couldn't
be replaced by a set_config(0) - set_config(prior) scheme. There's just a
vague nebulous feeling that one might do a more thorough reset than the
other. After all, a reset involves signalling at the electrical level
whereas a set_config only lives at the protocol level. Obviously this
depends entirely on the internal details of individual devices. But
wouldn't you think that if the device crashed, reset would have a better
chance of working than set_config?
Now back to the major topic. It had not occurred to me to worry about
what would happen in the case of a failed firmware download. Following
the re-initialization, what should we do -- treat it as the same old
device still bound to the same drivers, or treat it as a new device that
just happens to look like the old one? The second way runs the risk of
getting caught in an unending loop, so I'm inclined to agree with you that
the first is the way to go. It does make a failed altered() work like a
reset().
What about a failed reset() looking like an altered()? That's not so
clear to me. Yes, certainly, if following a reset the device has changed
you would want to treat it like altered(). But the use I had envisioned
for reset() was a lightweight error-recovery procedure that would not
check for changes. Considering the time required for a reset to work,
however, maybe it's not really necessary to make it lightweight.
So... Should usb-storage continue to use usb_device_reset() as an
error-recovery tool? Or should it become something used only to signal
that a device has (or may have) changed internally?
Alan Stern
-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
