From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753526AbbJBRfI (ORCPT ); Fri, 2 Oct 2015 13:35:08 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:37388 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752002AbbJBRfF (ORCPT ); Fri, 2 Oct 2015 13:35:05 -0400 Date: Fri, 2 Oct 2015 12:35:00 -0500 From: Felipe Balbi To: Peter Senna Tschudin CC: , , , , , , , Subject: Re: [PATCH 10/14] RFC: usb/host/fotg210: Add function scan_frame_queue() Message-ID: <20151002173500.GU5552@saruman.tx.rr.com> Reply-To: References: <20150915165002.GP19948@saruman.tx.rr.com> <1442847678-7970-1-git-send-email-peter.senna@gmail.com> <1442847678-7970-11-git-send-email-peter.senna@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="WcQ7DTTOeW3GIUV5" Content-Disposition: inline In-Reply-To: <1442847678-7970-11-git-send-email-peter.senna@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --WcQ7DTTOeW3GIUV5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 21, 2015 at 05:01:14PM +0200, Peter Senna Tschudin wrote: > checkpatch complains about too many leading tabs because the if > statement starts after 6 tabs: >=20 > scan_iosoc() -> for() -> while() -> switch() -> if() -> for() -> if() >=20 > There is also a goto statement going backwards in case of failure. This > patch creates a new inline function named scan_frame_queue() containing > the last 4 nesting levels, and removes the need of backwards goto, > making the code easier to read. After the patch it becomes: >=20 > scan_iosoc() -> for() -> while() -> scan_frame_queue() >=20 > Signed-off-by: Peter Senna Tschudin > --- > drivers/usb/host/fotg210-hcd.c | 142 ++++++++++++++++++++++-------------= ------ > 1 file changed, 76 insertions(+), 66 deletions(-) >=20 > diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hc= d.c > index 13cca41..e60a239 100644 > --- a/drivers/usb/host/fotg210-hcd.c > +++ b/drivers/usb/host/fotg210-hcd.c > @@ -4645,13 +4645,81 @@ done: > return status; > } > =20 > -/*----------------------------------------------------------------------= ---*/ > +static inline int scan_frame_queue(struct fotg210_hcd *fotg210, unsigned= frame, > + unsigned now_frame, bool live) > +{ > + unsigned uf; > + bool modified; > + union fotg210_shadow q, *q_p; > + __hc32 type, *hw_p; > + > + /* scan each element in frame's queue for completions */ > + q_p =3D &fotg210->pshadow[frame]; > + hw_p =3D &fotg210->periodic[frame]; > + q.ptr =3D q_p->ptr; > + type =3D Q_NEXT_TYPE(fotg210, *hw_p); > + modified =3D false; > + > + while (q.ptr !=3D NULL) { while (q.ptr) is enough > + switch (hc32_to_cpu(fotg210, type)) { > + case Q_TYPE_ITD: > + /* If this ITD is still active, leave it for comment style > + * later processing ... check the next entry. > + * No need to check for activity unless the > + * frame is current. > + */ > + if (frame =3D=3D now_frame && live) { > + rmb(); > + for (uf =3D 0; uf < 8; uf++) { > + if (q.itd->hw_transaction[uf] & > + ITD_ACTIVE(fotg210)) > + break; > + } > + if (uf < 8) { > + q_p =3D &q.itd->itd_next; > + hw_p =3D &q.itd->hw_next; > + type =3D Q_NEXT_TYPE(fotg210, > + q.itd->hw_next); > + q =3D *q_p; > + break; > + } > + } > + > + /* Take finished ITDs out of the schedule comment style > + * and process them: recycle, maybe report > + * URB completion. HC won't cache the > + * pointer for much longer, if at all. > + */ > + *q_p =3D q.itd->itd_next; > + *hw_p =3D q.itd->hw_next; > + type =3D Q_NEXT_TYPE(fotg210, q.itd->hw_next); > + wmb(); > + modified =3D itd_complete(fotg210, q.itd); > + q =3D *q_p; > + break; > + default: > + fotg210_dbg(fotg210, "corrupt type %d frame %d shadow %p\n", > + type, frame, q.ptr); > + /* FALL THROUGH */ > + case Q_TYPE_QH: > + case Q_TYPE_FSTN: > + /* End of the iTDs and siTDs */ > + q.ptr =3D NULL; > + break; > + } > + > + /* assume completion callbacks modify the queue */ > + if (unlikely(modified && fotg210->isoc_count > 0)) > + return -1; can't you add a proper error code here ? other than these, patch looks good. --=20 balbi --WcQ7DTTOeW3GIUV5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWDsBDAAoJEIaOsuA1yqREKUMP/jTuqkVH3EhEfDY65sRKESt0 jFW/0MlPd4WmfnSNEC50Qyt1jk1N77S20OVxEvlTYcLBjRbLO4uaaSS22uvIqPqg gytOz00MdI6yO7iN/CW3KxCYz/EOnoSXZ+rNM7ZBU2ybifZVZPlRYCQHp/cQIVWJ GdUkH15f7agJr6z+MfGsAAeHjxbeapAia9N30QYqwLtf4tpwXLjZQTugA/jN8hAF lUcBiuKIBxk9poJ4tqMhXw62Nfr8ubTfd9cZg7w8SwDSjXpLuOnHFDmrFKGseIQf uW42o+voybBj7GerfUkB4d+aCP4kEZ5OnnJ3ObqDV9zckAUgd5amdVlpOcAHL7cW sNy5fWpSA9Z0Mb13R7VDqZabXjIAQ7BcQx6ob4z4gx13j4rrWkk15/52zoRg1uaU /a+QlLcoGBnjz3Ih2K+ykAygySQNqzCsIGLM+Oe2DPGQ1HJse9o/rkKTCb73OdVP xL7eq4JHPnryq+X4tQB8n7/6lEsUh0nj/Ldh0G2HWKalrK8fDCgf7ANVb4FwiS+l tHK51LvPAtMMBuHRbvl+siJye+AjCs1Muy5/5JE7czaxC/ERUXpkaPCyQOGsSvPs 9xIS9a+BAXDI0k6VnOp+2aoTbyjSRKvl+5cl7MUFr+42a/mFOzPdk5gwGifwCPP2 SZkkO2f79d+/22dvm+I1 =eSXy -----END PGP SIGNATURE----- --WcQ7DTTOeW3GIUV5--