linux-kernel - Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110326134059.GC23260@riccoc20.at.omicron.at>
Date:	Sat, 26 Mar 2011 14:40:59 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	John Stultz <john.stultz@...aro.org>
Cc:	linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
	netdev@...r.kernel.org, devicetree-discuss@...ts.ozlabs.org,
	linux-arm-kernel@...ts.infradead.org,
	linuxppc-dev@...ts.ozlabs.org, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Arnd Bergmann <arnd@...db.de>,
	Christoph Lameter <cl@...ux.com>,
	David Miller <davem@...emloft.net>,
	Krzysztof Halasa <khc@...waw.pl>,
	Peter Zijlstra <peterz@...radead.org>,
	Rodolfo Giometti <giometti@...ux.it>,
	Thomas Gleixner <tglx@...utronix.de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Mike Frysinger <vapier@...too.org>,
	Paul Mackerras <paulus@...ba.org>,
	Russell King <linux@....linux.org.uk>
Subject: Re: [PATCH V12 4/4] ptp: Added a clock driver for the National
 Semiconductor PHYTER.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/