From: Jeff Garzik <jeff@garzik.org>
To: Joseph Kogut <joseph.kogut@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Drivers: net: 8139cp: Improved conformance to the Linux coding style guidelines.
Date: Fri, 16 Jul 2010 17:40:02 -0400 [thread overview]
Message-ID: <4C40D1B2.7000503@garzik.org> (raw)
In-Reply-To: <1279286391-31464-1-git-send-email-joseph.kogut@gmail.com>
On 07/16/2010 09:19 AM, Joseph Kogut wrote:
> Fixed several issues that made the 8139C+ driver nonconformant to the Linux coding style guidelines.
>
> Signed-off-by: Joseph Kogut<joseph.kogut@gmail.com>
> ---
> drivers/net/8139cp.c | 304 +++++++++++++++++++++++++-------------------------
> 1 files changed, 153 insertions(+), 151 deletions(-)
This patch is still failing in places to recognize obvious intent of the
code writer.
> /*
> Copyright 2001-2004 Jeff Garzik<jgarzik@pobox.com>
>
> - Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com) [tg3.c]
> - Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com) [sungem.c]
> - Copyright 2001 Manfred Spraul [natsemi.c]
> - Copyright 1999-2001 by Donald Becker. [natsemi.c]
> - Written 1997-2001 by Donald Becker. [8139too.c]
> - Copyright 1998-2001 by Jes Sorensen,<jes@trained-monkey.org>. [acenic.c]
> + Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com) [tg3.c]
> + Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com) [sungem.c]
> + Copyright 2001 Manfred Spraul [natsemi.c]
> + Copyright 1999-2001 by Donald Becker. [natsemi.c]
> + Written 1997-2001 by Donald Becker. [8139too.c]
> + Copyright 1998-2001 by Jes Sorensen,<jes@trained-monkey.org>. [acenic.c]
what is the point of this? I would leave copyright messages as-is.
> @@ -1295,32 +1295,32 @@ static void mdio_write(struct net_device *dev, int phy_id, int location,
> }
>
> /* Set the ethtool Wake-on-LAN settings */
> -static int netdev_set_wol (struct cp_private *cp,
> +static int netdev_set_wol(struct cp_private *cp,
> const struct ethtool_wolinfo *wol)
> {
> u8 options;
>
> - options = cpr8 (Config3)& ~(LinkUp | MagicPacket);
> + options = cpr8(Config3)& ~(LinkUp | MagicPacket);
> /* If WOL is being disabled, no need for complexity */
> if (wol->wolopts) {
> - if (wol->wolopts& WAKE_PHY) options |= LinkUp;
> - if (wol->wolopts& WAKE_MAGIC) options |= MagicPacket;
> + if (wol->wolopts& WAKE_PHY) options |= LinkUp;
> + if (wol->wolopts& WAKE_MAGIC) options |= MagicPacket;
> }
>
> - cpw8 (Cfg9346, Cfg9346_Unlock);
> - cpw8 (Config3, options);
> - cpw8 (Cfg9346, Cfg9346_Lock);
> + cpw8(Cfg9346, Cfg9346_Unlock);
> + cpw8(Config3, options);
> + cpw8(Cfg9346, Cfg9346_Lock);
>
> options = 0; /* Paranoia setting */
> - options = cpr8 (Config5)& ~(UWF | MWF | BWF);
> + options = cpr8(Config5)& ~(UWF | MWF | BWF);
> /* If WOL is being disabled, no need for complexity */
> if (wol->wolopts) {
> - if (wol->wolopts& WAKE_UCAST) options |= UWF;
> - if (wol->wolopts& WAKE_BCAST) options |= BWF;
> - if (wol->wolopts& WAKE_MCAST) options |= MWF;
> + if (wol->wolopts& WAKE_UCAST) options |= UWF;
> + if (wol->wolopts& WAKE_BCAST) options |= BWF;
> + if (wol->wolopts& WAKE_MCAST) options |= MWF;
> }
you just un-aligned things that were nicely tab-aligned
> @@ -1328,35 +1328,36 @@ static int netdev_set_wol (struct cp_private *cp,
> }
>
> /* Get the ethtool Wake-on-LAN settings */
> -static void netdev_get_wol (struct cp_private *cp,
> - struct ethtool_wolinfo *wol)
> +static void netdev_get_wol(struct cp_private *cp,
> + struct ethtool_wolinfo *wol)
> {
> u8 options;
>
> wol->wolopts = 0; /* Start from scratch */
> - wol->supported = WAKE_PHY | WAKE_BCAST | WAKE_MAGIC |
> - WAKE_MCAST | WAKE_UCAST;
> + wol->supported = WAKE_PHY | WAKE_BCAST | WAKE_MAGIC | WAKE_MCAST | WAKE_UCAST;
> +
> /* We don't need to go on if WOL is disabled */
> - if (!cp->wol_enabled) return;
> + if (!cp->wol_enabled)
> + return;
>
> - options = cpr8 (Config3);
> - if (options & LinkUp) wol->wolopts |= WAKE_PHY;
> - if (options & MagicPacket) wol->wolopts |= WAKE_MAGIC;
> + options = cpr8(Config3);
> + if (options & LinkUp) wol->wolopts |= WAKE_PHY;
> + if (options & MagicPacket) wol->wolopts |= WAKE_MAGIC;
>
> options = 0; /* Paranoia setting */
> - options = cpr8 (Config5);
> - if (options & UWF) wol->wolopts |= WAKE_UCAST;
> - if (options & BWF) wol->wolopts |= WAKE_BCAST;
> - if (options & MWF) wol->wolopts |= WAKE_MCAST;
> + options = cpr8(Config5);
> + if (options & UWF) wol->wolopts |= WAKE_UCAST;
> + if (options & BWF) wol->wolopts |= WAKE_BCAST;
> + if (options & MWF) wol->wolopts |= WAKE_MCAST;
> }
>
> -static void cp_get_drvinfo (struct net_device *dev, struct ethtool_drvinfo *info)
ditto
Also, it's disappointing to see so much diff noise created by
s/func ()/func()/
but I don't suppose I'll win that battle.
Jeff
prev parent reply other threads:[~2010-07-16 21:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-16 13:19 [PATCH] Drivers: net: 8139cp: Improved conformance to the Linux coding style guidelines Joseph Kogut
2010-07-16 20:13 ` David Miller
2010-07-16 21:40 ` Jeff Garzik [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C40D1B2.7000503@garzik.org \
--to=jeff@garzik.org \
--cc=davem@davemloft.net \
--cc=joseph.kogut@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).