Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756791AbZDRPpo (ORCPT ); Sat, 18 Apr 2009 11:45:44 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754857AbZDRPpc (ORCPT ); Sat, 18 Apr 2009 11:45:32 -0400 Received: from courier.cs.helsinki.fi ([128.214.9.1]:47120 "EHLO mail.cs.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754022AbZDRPpb (ORCPT ); Sat, 18 Apr 2009 11:45:31 -0400 Date: Sat, 18 Apr 2009 18:45:29 +0300 (EEST) From: "=?ISO-8859-1?Q?Ilpo_J=E4rvinen?=" X-X-Sender: ijjarvin@wrl-59.cs.helsinki.fi To: Peter Holik cc: LKML , Netdev , linux-usb@vger.kernel.org Subject: Re: [Resend][PATCH] usb driver for intellon int51x1 based PLC like devolo dlan duo In-Reply-To: <0681089db5bdc5acc805ed52130dc9a5.squirrel@webmail.it-technology.at> Message-ID: References: <0681089db5bdc5acc805ed52130dc9a5.squirrel@webmail.it-technology.at> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2391 Lines: 57 On Sat, 18 Apr 2009, Peter Holik wrote: > >> +static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev, > >> + struct sk_buff *skb, gfp_t flags) > >> +{ > >> + int pack_len = skb->len; > >> + int headroom = skb_headroom(skb); > >> + int tailroom = skb_tailroom(skb); > >> + int need_tail = 0; > >> + __le16 *len; > >> + > >> + /* > >> + * usbnet would send a ZLP if packetlength mod urbsize == 0 for us, > >> + * but we need to know ourself, because this would add to the length > >> + * we send down to the device... > >> + */ > >> + if (!((pack_len + INT51X1_HEADER_SIZE) % dev->maxpacket)) > >> + need_tail = 1; > >> + > >> + /* if packet and our header is smaler than 64 pad to 64 (+ ZLP) */ > >> + if ((pack_len + INT51X1_HEADER_SIZE + need_tail) < dev->maxpacket + 1) > >> + need_tail = dev->maxpacket + 1 - pack_len - INT51X1_HEADER_SIZE; > > > > This is totally crazy code fragment, first need_tail is used like a > > boolean? But on the same some +1 scalar trick is being done? Is there some > > reason why DIV_ROUND_UP (linux/kernel.h) won't do what you want here and > > then you can trivally find diff = new - old ? > > > > maybe this version is not so crazy for you? > > /* if packet and our header is smaler than 64 pad to 64 (+ ZLP) */ > if ((pack_len + INT51X1_HEADER_SIZE) < dev->maxpacket) > need_tail = dev->maxpacket - pack_len - INT51X1_HEADER_SIZE + 1; > /* > * usbnet would send a ZLP if packetlength mod urbsize == 0 for us, > * but we need to know ourself, because this would add to the length > * we send down to the device... > */ > else if (!((pack_len + INT51X1_HEADER_SIZE) % dev->maxpacket)) > need_tail = 1; Certainly looks more obvious, having that + need_tail and the related + 1 on the other side of the condition made the original very hard to read (and I failed to read it correctly which was the reason for the wrong DIV_ROUND_UP comment I made). This one is much better. I guess adding a temporary var for pack_len + INT51X1_HEADER_SIZE would also make it somewhat nicer looking. -- i. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/