linux-kernel - Re: [PATCH v9 4/5] xhci: mediatek: support MTK xHCI host controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <560D1CA9.1050009@linaro.org>
Date:	Thu, 1 Oct 2015 12:44:41 +0100
From:	Daniel Thompson <daniel.thompson@...aro.org>
To:	Chunfeng Yun <chunfeng.yun@...iatek.com>,
	Mathias Nyman <mathias.nyman@...el.com>
Cc:	Rob Herring <robh+dt@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Felipe Balbi <balbi@...com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Roger Quadros <rogerq@...com>, linux-usb@...r.kernel.org,
	linux-mediatek@...ts.infradead.org,
	John Crispin <blogic@...nwrt.org>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	Kishon Vijay Abraham I <kishon@...com>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v9 4/5] xhci: mediatek: support MTK xHCI host controller

On 29/09/15 04:01, Chunfeng Yun wrote:
> There some vendor quirks for MTK xhci host controller:
> 1. It defines some extra SW scheduling parameters for HW
>    to minimize the scheduling effort for synchronous and
>    interrupt endpoints. The parameters are put into reseved
>    DWs of slot context and endpoint context.
> 2. Its IMODI unit for Interrupter Moderation register is
>    8 times as much as that defined in xHCI spec.
> 3. Its TDS in  Normal TRB defines a number of packets that
>    remains to be transferred for a TD after processing all
>    Max packets in all previous TRBs.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@...iatek.com>

I've done some basic soak tests, both with a directly attached USB3 HDD 
and, given the extra code to manage isochronous xfer, also with a hub, 
disc and two audio interfaces.

Tested-by: Daniel Thompson <daniel.thompson@...aro.org>


> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 57f40a1..243f696 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -68,6 +68,7 @@
>   #include <linux/slab.h>
>   #include "xhci.h"
>   #include "xhci-trace.h"
> +#include "xhci-mtk.h"
>
>   /*
>    * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
> @@ -3044,18 +3045,27 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
>   			      struct urb *urb, unsigned int num_trbs_left)
>   {
>   	u32 maxp, total_packet_count;
> +	u32 skip_current_trb = 0;

A bit of a nitpick but why do we need skip_current_trb? Testing 
(xhci->quirks & XHCI_MTK_HOST) twice would make what the code does, and 
why, more obvious.

Anyhow with that looked at, and the caveat that I'm not much of USB 
expert, you're welcome to add my Reviewed-by: to v10.

>
> -	if (xhci->hci_version < 0x100)
> -		return ((td_total_len - transferred) >> 10);
> -
> -	maxp = GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
> -	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
> +	if (xhci->hci_version < 0x100) {
> +		/* for MTK xHCI, TD size doesn't include this TRB */
> +		if (xhci->quirks & XHCI_MTK_HOST)
> +			skip_current_trb = 1;
> +		else
> +			return ((td_total_len - transferred) >> 10);
> +	}
>
>   	/* One TRB with a zero-length data packet. */
>   	if (num_trbs_left == 0 || (transferred == 0 && trb_buff_len == 0) ||
>   	    trb_buff_len == td_total_len)
>   		return 0;
>
> +	if (skip_current_trb)
> +		trb_buff_len = 0;
> +
> +	maxp = GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
> +	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
> +
>   	/* Queueing functions don't count the current TRB into transferred */
>   	return (total_packet_count - ((transferred + trb_buff_len) / maxp));
>   }


Daniel.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/