Re: [PATCH v2 4/4] usb: refine phy notify operation during connection and disconnection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 09, 2012 at 10:25:21AM -0400, Alan Stern wrote:
> On Tue, 9 Oct 2012, Chen Peter-B29397 wrote:
> 
> > > > @@ -4187,6 +4190,10 @@ static void hub_port_connect_change(struct
> > > usb_hub *hub, int port1,
> > > >  		}
> > > >  	}
> > > >
> > > > +	if (hcd->phy && !hdev->parent &&
> > > > +		!(portstatus & USB_PORT_STAT_CONNECTION))
> > > > +			usb_phy_notify_disconnect(hcd->phy, udev->speed);
> > > 
> > > What happens if udev is NULL (see the test in the next statement)?
> > > 
> > 
> > I will add the condition of (udev), the thing I want to do is: 
> 
> You shouldn't add a new condition.  Instead you should move your code 
> under the existing "if" statement.

Thanks, I will
> 
> > when the device is disconnected, the pcd interrupt will occur, and
> > the code will be there, I need to tell the phy driver, the disconnection
> > occurs, and the speed of the disconnected usb device.
> > 
> > > > +
> > > >  	/* Disconnect any existing devices under this port */
> > > >  	if (udev)
> > > >  		usb_disconnect(&hub->ports[port1 - 1]->child);
> > > > @@ -4212,13 +4219,6 @@ static void hub_port_connect_change(struct
> > > usb_hub *hub, int port1,
> > > >  		}
> > > >  	}
> > > >
> > > > -	if (hcd->phy && !hdev->parent) {
> > > > -		if (portstatus & USB_PORT_STAT_CONNECTION)
> > > > -			usb_phy_notify_connect(hcd->phy, port1);
> > > > -		else
> > > > -			usb_phy_notify_disconnect(hcd->phy, port1);
> > > 
> > > Is the second argument supposed to be a port number, like here, or a
> > > speed value, like above?  Clearly something is wrong, either in the old
> > > code or in the new code.
> > > 
> > The first patch at this patchset changes the API of usb_phy_notify_disconnect:
> 
> That means the first patch breaks the code.  People running "git 
> bisect" might happen to hit a commit in between the two patches, and 
> their kernels won't compile.  That's not acceptable.

So, a good practice is merging .h (changed API) and .c(using this API)
into a commit?
> 
> Alan Stern
> 
> 

-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux