From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH V12 3/4] ptp: Added a clock driver for the IXP46x. From: John Stultz To: Richard Cochran In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Date: Wed, 23 Mar 2011 14:43:00 -0700 Message-ID: <1300916580.848.51.camel@work-vm> Mime-Version: 1.0 Cc: Rodolfo Giometti , Arnd Bergmann , Peter Zijlstra , linux-api@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, Russell King , Paul Mackerras , Thomas Gleixner , Alan Cox , netdev@vger.kernel.org, Mike Frysinger , Christoph Lameter , linuxppc-dev@lists.ozlabs.org, David Miller , linux-arm-kernel@lists.infradead.org, Krzysztof Halasa List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote: > This patch adds a driver for the hardware time stamping unit found on the > IXP465. The basic clock operations and an external trigger are implemented. > > Signed-off-by: Richard Cochran > Acked-by: John Stultz My ack is still fine, but found a few more nits to consider on another pass... > +static int match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq) [snip] > +static void do_rx_timestamp(struct port *port, struct sk_buff *skb) [snip] > +static u64 sys_time_read(struct ixp46x_ts_regs *regs) [snip] > +static void sys_time_write(struct ixp46x_ts_regs *regs, u64 ns) I know these are static, but these function names are sort of generic names, and make grep/cscoping for similar sounding functions a little noisy. Maybe add a ipx_ prefix just to be more obvious? thanks -john