Larry, I seem to have neglected to respond to this email. Dan has suggested skipping the changes to rtl871x_ioctl_rtl.h and rtl871x_ioctl_rtl.c. What do you think? IIRC none of these oid functions are used anyway. I'd just assumed that they are for some planned/hoped SNMP interface. On Tue, Aug 23, 2011 at 10:58:42AM -0500, Larry Finger wrote: > On 08/23/2011 03:53 AM, Ali Bahar wrote: > >On Tue, Aug 23, 2011 at 11:35:55AM +0300, Dan Carpenter wrote: > >> > >>[PATCH 4/5] removes the _netdev_priv() function, but this patch is > >>still removing call sites. In other words, patch 4/5 breaks the > >>build. > > > >You're right. Patch 4 won't build by itself. I paid a lot of attention > >to this at the outset, but this got missed. Thanks. > > > >So 4 needs to be squashed into 5. Or are you suggesting an > >alternative? > > To make the point perfectly clear, the code must compile after each > patch is added as a bisection might land in the middle. If possible, I know. I set out that way, but it got lost in the shuffle. > it should also work. I don't have a full-blown Integration Testing team here, so I do only _compilation_ tests for the minor patches, and full testing for the whole shebbang. > I think that all of the _netdev_priv => netdev_priv changes should > be in a separate patch. In addition, reviewing would be a lot easier So, considering Dan's suggestions as well, 1 patch for _netdev_priv => netdev_priv, 1 patch for os_free_netdev => free_netdev, 1 patch for _usb_alloc_urb removal, 1 patch for _usb_submit_urb removal, and Dan has suggested (as well as tested) putting all the includes in one commit. As you can imagine, headers had choked a fair bit in my own builds, but I will give this a try anyway: 1 commit for all the changes to include directives, and > if the copyright header changes were not mixed with changes in the > code. 1 patch for all remaining copyright banners. Greg has applied some already, but the other files' will be separated into a single patch in order to make easier the reviews of the behemoth patch. 1 patch/commit for each of: - MSEC removal - LedControl871x's redundant NULL check, - and any other such separable changes I stumble upon. No changes to - rtl8712_cmd.c - r8712_wep_encrypt() and r8712_wep_decrypt() This means that curfragnum and length will remain unsigned. 'length' is assigned from a subtraction. But recv_frame_hdr.len has not changed, nor has rx_pkt_attrib. If the packet lengths are that wrong, then it will be wrong regardless of whether we have the negative sign or not. These length changes seem very much like an indiscriminate, global replace! - rtl871x_security.c - the sign for 'pipe', in usb_ops_linux.c and TBD: - skipping the changes to rtl871x_ioctl_rtl.h and rtl871x_ioctl_rtl.c. and 1 patch for the remainder. Effectively, this'll merge (what, if any, is left of) 4 with 5 -- as they are numbered in the _current_ patch set. If not otherwise separable, this will include: - recv_frame.u.mem will be removed. - NR_RECVBUFF remains 8, but with Larry's explanatory comment. - Larry's various feedback on Patch 5. regards, ali > >Larry Finger's 2010 commit (of the Realtek tarball) seemed to have > >been done en masse. So that's what I was going to do, originally. It > >looks like this one, too, should be. > > The initial inclusion of a staging driver sometimes operates under > different rules. As I recall, I originally was submitting each > routine and its header file in separate patches, but I was asked to > combine them to a single patch in the end. Now that the driver has > been around for some time and it is being used, there is a new rule. > > Larry _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
