On Wed, May 27, 2015 at 04:28:50AM +0000, Kaukab, Yousaf wrote: > > -----Original Message----- > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > Sent: Wednesday, May 27, 2015 12:20 PM > > To: Kaukab, Yousaf > > Cc: balbi@xxxxxx; linux-usb@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH 01/11] usb: phy: return error on failure > > > > On Wed, May 27, 2015 at 12:31:28AM +0000, Kaukab, Yousaf wrote: > > > > -----Original Message----- > > > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > > > Sent: Tuesday, May 26, 2015 11:17 PM > > > > To: Kaukab, Yousaf > > > > Cc: linux-usb@xxxxxxxxxxxxxxx; balbi@xxxxxx > > > > Subject: Re: [PATCH 01/11] usb: phy: return error on failure > > > > > > > > On Tue, May 05, 2015 at 04:14:35PM +0200, Mian Yousaf Kaukab wrote: > > > > > If the requested phy operation can't be done, return error instead > > > > > of success to let the caller know. > > > > > > > > > > Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@xxxxxxxxx> > > > > > --- > > > > > include/linux/usb/phy.h | 10 +++++----- > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h > > > > > index bc91b5d..d2b03db 100644 > > > > > --- a/include/linux/usb/phy.h > > > > > +++ b/include/linux/usb/phy.h > > > > > @@ -262,7 +262,7 @@ usb_phy_set_power(struct usb_phy *x, unsigned > > > > > mA) { > > > > > if (x && x->set_power) > > > > > return x->set_power(x, mA); > > > > > - return 0; > > > > > + return -EOPNOTSUPP; > > > > > > > > the idea is that these operations are actually optional, so it's ok to return 0. > > > > > > Caller can ignore the error if its optional for it. There are others > > > > ok, I have to spell it out. > > > > It's not optional for the *caller*, it's optional for the PHY driver to implement > > it. > > > > > who have put the similar validity checks before usb phy api calls > > > because they want to return error in case the call fails. We can > > > remove such checks by returning the error here. > > > > no... what we're doing here is: > > > > if the phy driver implements this call, then let's call that callback and pass > > along any error it might return, otherwise we just return success because that > > means this phy doesn't really implement this call. > > > > This is done is many other subsystems where some function pointers might be > > optional to be implemented. Just think about the error handling. When we > > return 0: > > > > ret = usb_phy_set_power(x, 100); > > if (ret < 0) > > bail(); > > > > /* continue */ > > > > if we return EOPNOTSUPP: > > > > ret = usb_phy_set_power(x, 100); > > if (ret < 0 && ret != -EOPNOTSUPP) > > bail(); > > > > /* continue */ > > Got it. You can drop this series for now. I will resend a subset to > fix IS_ERR_OR_NULL stuff. yeah, that needs to go, indeed. -- balbi
Attachment:
signature.asc
Description: Digital signature
