> -----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. > > cheers > > -- > balbi Thanks, Yousaf -- 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
