From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Stultz Subject: Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER. Date: Wed, 23 Mar 2011 14:46:25 -0700 Message-ID: <1300916785.848.54.camel@work-vm> References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Richard Cochran Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Alan Cox , Arnd Bergmann , Christoph Lameter , David Miller , Krzysztof Halasa , Peter Zijlstra , Rodolfo Giometti , Thomas Gleixner , Benjamin Herrenschmidt , Mike Frysinger , Paul Mackerras , Russell King List-Id: linux-api@vger.kernel.org On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote: > This patch adds support for the PTP clock found on the DP83640. > The basic clock operations and one external time stamp have > been implemented. Just locking rule comment nits here. > +static int tdr_write(int bc, struct phy_device *dev, > + const struct timespec *ts, u16 cmd) > +{ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec & 0xffff);/* ns[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec >> 16); /* ns[31:16] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec & 0xffff); /* sec[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec >> 16); /* sec[31:16]*/ > + > + ext_write(bc, dev, PAGE4, PTP_CTL, cmd); > + > + return 0; > +} The above needs to hold the extreg_lock, and should be commented as such. And again, the function names are sort of generic, and could use a dp83640_ prefix or something. thanks -john From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER. From: John Stultz To: Richard Cochran In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Date: Wed, 23 Mar 2011 14:46:25 -0700 Message-ID: <1300916785.848.54.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 support for the PTP clock found on the DP83640. > The basic clock operations and one external time stamp have > been implemented. Just locking rule comment nits here. > +static int tdr_write(int bc, struct phy_device *dev, > + const struct timespec *ts, u16 cmd) > +{ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec & 0xffff);/* ns[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec >> 16); /* ns[31:16] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec & 0xffff); /* sec[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec >> 16); /* sec[31:16]*/ > + > + ext_write(bc, dev, PAGE4, PTP_CTL, cmd); > + > + return 0; > +} The above needs to hold the extreg_lock, and should be commented as such. And again, the function names are sort of generic, and could use a dp83640_ prefix or something. thanks -john From mboxrd@z Thu Jan 1 00:00:00 1970 From: john.stultz@linaro.org (John Stultz) Date: Wed, 23 Mar 2011 14:46:25 -0700 Subject: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER. In-Reply-To: References: Message-ID: <1300916785.848.54.camel@work-vm> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote: > This patch adds support for the PTP clock found on the DP83640. > The basic clock operations and one external time stamp have > been implemented. Just locking rule comment nits here. > +static int tdr_write(int bc, struct phy_device *dev, > + const struct timespec *ts, u16 cmd) > +{ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec & 0xffff);/* ns[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec >> 16); /* ns[31:16] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec & 0xffff); /* sec[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec >> 16); /* sec[31:16]*/ > + > + ext_write(bc, dev, PAGE4, PTP_CTL, cmd); > + > + return 0; > +} The above needs to hold the extreg_lock, and should be commented as such. And again, the function names are sort of generic, and could use a dp83640_ prefix or something. thanks -john From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756709Ab1CWVqk (ORCPT ); Wed, 23 Mar 2011 17:46:40 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:44027 "EHLO e37.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752524Ab1CWVqi (ORCPT ); Wed, 23 Mar 2011 17:46:38 -0400 Subject: Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER. From: John Stultz To: Richard Cochran Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Alan Cox , Arnd Bergmann , Christoph Lameter , David Miller , Krzysztof Halasa , Peter Zijlstra , Rodolfo Giometti , Thomas Gleixner , Benjamin Herrenschmidt , Mike Frysinger , Paul Mackerras , Russell King In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Date: Wed, 23 Mar 2011 14:46:25 -0700 Message-ID: <1300916785.848.54.camel@work-vm> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote: > This patch adds support for the PTP clock found on the DP83640. > The basic clock operations and one external time stamp have > been implemented. Just locking rule comment nits here. > +static int tdr_write(int bc, struct phy_device *dev, > + const struct timespec *ts, u16 cmd) > +{ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec & 0xffff);/* ns[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec >> 16); /* ns[31:16] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec & 0xffff); /* sec[15:0] */ > + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec >> 16); /* sec[31:16]*/ > + > + ext_write(bc, dev, PAGE4, PTP_CTL, cmd); > + > + return 0; > +} The above needs to hold the extreg_lock, and should be commented as such. And again, the function names are sort of generic, and could use a dp83640_ prefix or something. thanks -john