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:
>
> scan_iosoc() -> for() -> while() -> switch() -> if() -> for() -> if()
>
> 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:
>
> scan_iosoc() -> for() -> while() -> scan_frame_queue()
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@xxxxxxxxx>
> ---
> drivers/usb/host/fotg210-hcd.c | 142 ++++++++++++++++++++++-------------------
> 1 file changed, 76 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.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;
> }
>
> -/*-------------------------------------------------------------------------*/
> +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 = &fotg210->pshadow[frame];
> + hw_p = &fotg210->periodic[frame];
> + q.ptr = q_p->ptr;
> + type = Q_NEXT_TYPE(fotg210, *hw_p);
> + modified = false;
> +
> + while (q.ptr != 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 == now_frame && live) {
> + rmb();
> + for (uf = 0; uf < 8; uf++) {
> + if (q.itd->hw_transaction[uf] &
> + ITD_ACTIVE(fotg210))
> + break;
> + }
> + if (uf < 8) {
> + q_p = &q.itd->itd_next;
> + hw_p = &q.itd->hw_next;
> + type = Q_NEXT_TYPE(fotg210,
> + q.itd->hw_next);
> + q = *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 = q.itd->itd_next;
> + *hw_p = q.itd->hw_next;
> + type = Q_NEXT_TYPE(fotg210, q.itd->hw_next);
> + wmb();
> + modified = itd_complete(fotg210, q.itd);
> + q = *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 = 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.
--
balbi
Attachment:
signature.asc
Description: PGP signature
