linux-kernel - Re: [PATCH v2] USB: gadget: s3c-hsotg: add isochronous transfers support

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] [day] [month] [year] [list]

Message-ID: <20131004143602.GF16059@radagast>
Date:	Fri, 4 Oct 2013 09:36:02 -0500
From:	Felipe Balbi <balbi@...com>
To:	Robert Baldyga <r.baldyga@...sung.com>
CC:	<balbi@...com>, <gregkh@...uxfoundation.org>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<b.zolnierkie@...sung.com>, <m.szyprowski@...sung.com>,
	<andrzej.p@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH v2] USB: gadget: s3c-hsotg: add isochronous transfers
 support

On Wed, Oct 02, 2013 at 12:34:21PM +0200, Robert Baldyga wrote:
> On 10/01/2013 04:45 PM, Felipe Balbi wrote:
> Hello,
> >Hi,
> >
> >On Tue, Sep 24, 2013 at 11:47:16AM +0200, Robert Baldyga wrote:
> >>Hello,
> >>
> >>This is update for my proposal for isochronous transfers support in s3c-hsotg
> >>driver. I've fixed issuses pointed by Bartlomiej Zolnierkiewicz. For more
> >>information, please check the change log at the end of the mail.
> >
> >this shouldn't be part of commit log.
> >
> >>This patch adds isochronous transfer support. It adds few modifications:
> >>- Modify s3c_hsotg_write_fifo() function. It actually calculates transfer
> >>   size, taking into account Multi Count value, which indicates number of
> >>   transactions per microframe.
> >>- Fix s3c_hsotg_start_req() function by setting number of packets to Multi
> >>   Count field in DIEPTSIZ register for isochronous endpoints.
> >>- Fix s3c_hsotg_set_ep_maxpacket() function. Field wMaxPacketSize of endpoint
> >>   descriptor is now splitted into maximum packet size value and number of
> >>   additional transaction per microframe.
> >>- Modify s3c_hsotg_epint() function. Some interrupts are ignored for
> >>   isochronous endpoints, (e.g. INTknTXFEmpMsk) becouse isochronous request is
> >>   always transfered in single transaction, which ends with XferCompl interrupt.
> >>   Add Odd/Even microframe toggle to allow data transfering in each microframe.
> >>- Fix s3c_hsotg_ep_enable() function by supporting isochronous endpoint type.
> >
> >you're doing way too many things in a single patch.
> 
> All these changes concerns to isochronous transfers support, and I
> think they should not be splitted into number of patches. This entire
> patch adds one compact functionality and it has no sense to put small
> parts of this functionality to separated patches.

you have at least two bug fixes here which should be splitted into their
own patches.

I'm not taking this patch the way it is.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)