On Fri, 22 Feb 2013, Peter Hurley wrote:
> ISTM the usb_serial_port private data should not be freed until
> port_release().
I don't think that's right. In general, a driver doesn't know what's
going to happen to a device after the two are unbound from each other.
Another driver may get bound to that same device, and may overwrite the
old private-data pointer with its own new one.
The same reasoning applies here. The serial subdriver isn't involved
in the creation of the usb_serial_port structure, and therefore it
shouldn't be involved in that structure's destruction.
> If usb traffic isn't halted until port_release() ...
>
> static void port_release(struct device *dev)
> {
> struct usb_serial_port *port = to_usb_serial_port(dev);
> int i;
>
> dev_dbg(dev, "%s\n", __func__);
>
> /*
> * Stop all the traffic before cancelling the work, so that
> * nobody will restart it by calling usb_serial_port_softint.
> */
> ====> kill_traffic(port);
> cancel_work_sync(&port->work);
>
> then what happens here if ->port_remove() has already been called?
>
> static void garmin_write_bulk_callback(struct urb *urb)
> {
> struct usb_serial_port *port = urb->context;
>
> if (port) {
> struct garmin_data *garmin_data_p =
> usb_get_serial_port_data(port);
>
> if (GARMIN_LAYERID_APPL == getLayerId(urb->transfer_buffer)) {
>
> ====> if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
> gsp_send_ack(garmin_data_p,
> ((__u8 *)urb->transfer_buffer)[4]);
> }
Clearly this is a mistake. Probably it is the result of historical
confusion between the overall USB device and the multiple serial ports
embodied within it. At any rate, it is clear that all port-related USB
activity should be stopped when the port is unregistered.
Whether this can be carried out by the usb-serial core or needs help
from the subdriver, I leave up to you guys.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
