Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753409AbcDKIQd (ORCPT ); Mon, 11 Apr 2016 04:16:33 -0400 Received: from mga03.intel.com ([134.134.136.65]:17126 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752539AbcDKIQ3 (ORCPT ); Mon, 11 Apr 2016 04:16:29 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,462,1455004800"; d="asc'?scan'208";a="956021019" From: Felipe Balbi To: changbin.du@intel.com Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Du\, Changbin" Subject: Re: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void In-Reply-To: <1460108062-31398-2-git-send-email-changbin.du@intel.com> References: <1460108062-31398-1-git-send-email-changbin.du@intel.com> <1460108062-31398-2-git-send-email-changbin.du@intel.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Mon, 11 Apr 2016 11:14:26 +0300 Message-ID: <878u0kpnod.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3285 Lines: 112 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, changbin.du@intel.com writes: > diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h > index 07fbc2d..71e3180 100644 > --- a/drivers/usb/dwc3/debug.h > +++ b/drivers/usb/dwc3/debug.h > @@ -217,11 +217,11 @@ static inline const char *dwc3_gadget_event_type_st= ring(u8 event) > void dwc3_trace(void (*trace)(struct va_format *), const char *fmt, ...); >=20=20 > #ifdef CONFIG_DEBUG_FS > -extern int dwc3_debugfs_init(struct dwc3 *); > +extern void dwc3_debugfs_init(struct dwc3 *); > extern void dwc3_debugfs_exit(struct dwc3 *); > #else > -static inline int dwc3_debugfs_init(struct dwc3 *d) > -{ return 0; } > +static inline void dwc3_debugfs_init(struct dwc3 *d) > +{ } > static inline void dwc3_debugfs_exit(struct dwc3 *d) > { } > #endif > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c > index 9ac37fe..071b286 100644 > --- a/drivers/usb/dwc3/debugfs.c > +++ b/drivers/usb/dwc3/debugfs.c > @@ -618,69 +618,39 @@ static const struct file_operations dwc3_link_state= _fops =3D { > .release =3D single_release, > }; >=20=20 > -int dwc3_debugfs_init(struct dwc3 *dwc) > +void dwc3_debugfs_init(struct dwc3 *dwc) > { > struct dentry *root; > - struct dentry *file; > - int ret; >=20=20 > root =3D debugfs_create_dir(dev_name(dwc->dev), NULL); > - if (!root) { > - ret =3D -ENOMEM; > - goto err0; > - } > + if (IS_ERR_OR_NULL(root)) > + return; We can definitely keep on going, but I'd still like to know that we enabled CONFIG_DEBUG_FS but failed to create a file or a directory. Seems like this should read as follows: if (IS_ERR_OR_NULL(root)) { if (!root) dev_err(dwc->dev, "Can't create debugfs root\n"); return; } ditto to all bellow. > dwc->root =3D root; >=20=20 > dwc->regset =3D kzalloc(sizeof(*dwc->regset), GFP_KERNEL); > if (!dwc->regset) { > - ret =3D -ENOMEM; > - goto err1; > + debugfs_remove_recursive(root); you're now duplicating debugfs_remove_recursive(root) in all braches and that's error prone. It's probably better to keep our gotos, but change them so they read as follows: if (!dwc->regset) goto err1; [...] return; /* this is our successful exit point */ err1: debugfs_remove_recursive(root); kfree(dwc->regset); =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXC1ziAAoJEIaOsuA1yqREpbAP+wXvKyf2nBt52NMHTDQJ1Bm5 j/WvPRM34KJwdsT3EnTLLLnU1kc0VfgNAvEVY6wsxOOUROz58r1z4inmUwNT1r/g vrbq//MwWee/XisZPnyEMeuz7fGBHBzxzr9L8YExof9TDUate7ssFm8uSs1ou3Ai EpNGFS/dLkLFFdAZ9EaHFAaiVDF1EkboamRbbz2fd/zUgK8b3Q5CS4RO/X9VsOt5 4SiItXSQH+EBZmeKgNfYBhA/a5q3agfUH3d/uaiWOT66pGCUCQuGeWcrJvsMIY3m sWDU7zc8Wh/F/hy7YTyIEzD7AlAWmNg1OkMblAaNra+1As2GJ1eyE2GD6aiso+DB kKXleQYQ0jMOI58BSbvUNCMW9PJ5F4dBwe4t/qnFBkZ7edVBCLPGAp7flSDppj8O iVy8haU0LgFqsl/xOUFgh+3EyHp8AgivogEnF4aORPvnCprRa6mw9uE7TBhlX1l7 ZWy0JwucBFX9SUBuULSscY+QcziVyuYT67r3Zb4Yw+l+BkEqj7EUGlbSRHWfqpy0 y5ii8LtDmb1FU5PXRdYE5P76rPVYuuZ3Gk5So/egUg+p2QrStGR02iJZ+u4h614X Me7OU30Px4fkEemiOBgbkFs6ZCAGUEbQeTNJFXoiNOZ8QuZXqJvX8OaGJOOc/ACw p2QMCrKPVgfwjYm1z3BW =Idz4 -----END PGP SIGNATURE----- --=-=-=--