Date: Thu, 11 Nov 2004 22:31:30 +0100
From: Francois Romieu <romieu@fr.zoreil.com>
To: Jason McMullan <jason.mcmullan@timesys.com>
Cc: linux-kernel@vger.kernel.org, jgarzik@pobox.com
Subject: Re: [PATCH] MII bus API for PHY devices
Message-ID: <20041111213130.GA13327@electric-eye.fr.zoreil.com>
References: <20041111194526.GA17735@jmcmullan.timesys>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20041111194526.GA17735@jmcmullan.timesys>
User-Agent: Mutt/1.4.1i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1186
Lines: 26

Jason McMullan <jason.mcmullan@timesys.com> :
> First patch for consolidation of PHY handling into one location.

A little pass of polish could make it more cool imho:
- macro abuse;
- unchecked malloc;
- use plain old style multi-lines C comments (/* ...\n * ... \n * ... \n */) ?
- whitespace/tabulation damage (search for series of 2 or more spaces);
- hidden return: please put them on a separate line;
- no need to BUG_ON when there is an immediate dereference;
- mixed case variables/names (pLEAse DO not dO tHAT);
- unneeded checks ? How could VALID_BUS() in mii_bus_write() fail ?
- use goto when locking primitives are used (btw the last check in
  mii_bus_register is not needed and you can s/mii_bus[bus_id]/bus/ 
  a few times in this function) ?
- add a break in mii_bus_unregister() ?
- I'd probably favor ternary operators here and there (your choice, really)
- u32 and uint16_t in the same file: choose one style ?

--
Ueimor
-
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/