netdev - RE: [net-next 04/14] e1000e: add support for IEEE-1588 PTP

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 18 Jan 2013 01:13:22 +0000
From:	"Allan, Bruce W" <bruce.w.allan@...el.com>
To:	Richard Cochran <richardcochran@...il.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [net-next 04/14] e1000e: add support for IEEE-1588 PTP

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@...il.com]
> Sent: Thursday, January 17, 2013 7:36 AM
> To: Kirsher, Jeffrey T
> Cc: davem@...emloft.net; Allan, Bruce W; netdev@...r.kernel.org;
> gospo@...hat.com; sassmann@...hat.com
> Subject: Re: [net-next 04/14] e1000e: add support for IEEE-1588 PTP
> 
> On Thu, Jan 17, 2013 at 03:35:09AM -0800, Jeff Kirsher wrote:
> 
> > +static struct ptp_clock_info e1000e_ptp_clock_info = {
> > +	.owner		= THIS_MODULE,
> 
> small nit: better to use a static string for .name here than ...
> 
> > +	.n_alarm	= 0,
> > +	.n_ext_ts	= 0,
> > +	.n_per_out	= 0,
> > +	.pps		= 0,
> > +	.adjfreq	= e1000e_phc_adjfreq,
> > +	.adjtime	= e1000e_phc_adjtime,
> > +	.gettime	= e1000e_phc_gettime,
> > +	.settime	= e1000e_phc_settime,
> > +	.enable		= e1000e_phc_enable,
> > +};
> > +
> > +/**
> > + * e1000e_ptp_init - initialize PTP for devices which support it
> > + * @adapter: board private structure
> > + *
> > + * This function performs the required steps for enabling PTP support.
> > + * If PTP support has already been loaded it simply calls the cyclecounter
> > + * init routine and exits.
> > + **/
> > +void e1000e_ptp_init(struct e1000_adapter *adapter)
> > +{
> > +	struct e1000_hw *hw = &adapter->hw;
> > +
> > +	adapter->ptp_clock = NULL;
> > +
> > +	if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP))
> > +		return;
> > +
> > +	adapter->ptp_clock_info = e1000e_ptp_clock_info;
> > +
> > +	snprintf(adapter->ptp_clock_info.name,
> > +		 sizeof(adapter->ptp_clock_info.name), "%pm",
> > +		 adapter->netdev->perm_addr);
> 
> ... putting any kind of address here. After some back and forth,
> discussing what the 'name' field should be, we decided on
> 
>  * @name:      A short "friendly name" to identify the clock and to
>  *             help distinguish PHY based devices from MAC based ones.
>  *             The string is not meant to be a unique id.
> 
> and most other drivers put the name of the driver here.
> 
> Other than that, this new driver looks good to me. I'll try it out
> soon.
> 
> Acked-by: Richard Cochran <richardcochran@...il.com>

Thanks for the review Richard.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html