From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Cochran Subject: Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER. Date: Sat, 26 Mar 2011 14:40:59 +0100 Message-ID: <20110326134059.GC23260@riccoc20.at.omicron.at> References: <1300916785.848.54.camel@work-vm> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1300916785.848.54.camel@work-vm> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: John Stultz Cc: Rodolfo Giometti , Peter Zijlstra , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Russell King , Thomas Gleixner , Alan Cox , netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mike Frysinger , Christoph Lameter , linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, David Miller , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Krzysztof Halasa List-Id: linux-api@vger.kernel.org On Wed, Mar 23, 2011 at 02:46:25PM -0700, John Stultz wrote: > On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote: > > +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. Okay, will do. > And again, the function names are sort of generic, and could use a > dp83640_ prefix or something. So, I will rename the IXP functions in the other driver to make them more unique, but in this case I really prefer to keep the short names for the sake of readability. Just about every operation on this PHY requires four 16-bit writes, where each write can really mean two writes, with the first one to set a "page" register for the following write. Keeping the tabular style (shown above) makes it much more clear what is going on, IMHO. If the function names become longer, that would force line breaks and spoil the nice formatting. Also, these names are not in use anywhere else in the kernel's eight million LOC. Thanks, Richard