| Message ID | 20130511140219.GG8399@zhudong.nay.redhat.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, May 11, 2013 at 10:02:19PM +0800, Dong Zhu wrote: > > Currently kernel only support setting the hw time stamping policy > through ioctl,now add a method to check which packets(Outgoing and > Incoming) are time stamped by nic. I don't really see a use case here. Applications needing time stamping should just set the policy that they need. > struct hwtstamp_config { > + int rw; Also, you can't just go adding fields to an ioctl. > int flags; > int tx_type; > int rx_filter; > -- Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 11-05-2013 18:02, Dong Zhu wrote: > From e6a55411486de8a09b859e73140bf35c0ee36047 Mon Sep 17 00:00:00 2001 > From: Dong Zhu <bluezhudong@gmail.com> > Date: Sat, 11 May 2013 21:44:54 +0800 > Subject: [PATCH] igb: add a method to get the nic hw time stamping policy Please, don't send this header with your patches. > Currently kernel only support setting the hw time stamping policy > through ioctl,now add a method to check which packets(Outgoing and > Incoming) are time stamped by nic. > Signed-off-by: Dong Zhu <bluezhudong@gmail.com> > --- > drivers/net/ethernet/intel/igb/igb_ptp.c | 29 +++++++++++++++++++++++++++++ > include/uapi/linux/net_tstamp.h | 4 +++- > 2 files changed, 32 insertions(+), 1 deletion(-) > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c > index 7e8c477..8c06346 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c > @@ -577,6 +577,34 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, > if (config.flags) > return -EINVAL; > > + if (config.rw == 0) > + goto set_policy; > + else if (config.rw == 1) { Both arms of the *if* statement should have {} if one does, according to Documentation/CodingStyle. > + if (config.tx_type || config.rx_filter) > + return -EINVAL; > + > + regval = rd32(E1000_TSYNCTXCTL); > + Empty line not needed here. > + if (regval & E1000_TSYNCTXCTL_ENABLED) > + config.tx_type = HWTSTAMP_TX_ON; > + else > + config.tx_type = HWTSTAMP_TX_OFF; > + > + regval = rd32(E1000_TSYNCRXCTL); > + ... and here. > + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) > + config.rx_filter = HWTSTAMP_FILTER_NONE; > + else if (E1000_TSYNCRXCTL_TYPE_ALL == > + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) > + config.rx_filter = HWTSTAMP_FILTER_ALL; > + else > + return -ERANGE; > + > + goto end; > + } else > + return -EINVAL; Same comment about missing {}. This *if*, however, asks to be converted to *switch*... > diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h > index ae5df12..77147da 100644 > --- a/include/uapi/linux/net_tstamp.h > +++ b/include/uapi/linux/net_tstamp.h > @@ -28,9 +28,10 @@ enum { > /** > * struct hwtstamp_config - %SIOCSHWTSTAMP parameter > * > + * @rw: 0/1 represents set/get the hw time stamp policy > * @flags: no flags defined right now, must be zero > * @tx_type: one of HWTSTAMP_TX_* > - * @rx_type: one of one of HWTSTAMP_FILTER_* > + * @rx_filter: one of one of HWTSTAMP_FILTER_* You don't mention this change in the changelog. Most probably it should be in a separate patch. > @@ -39,6 +40,7 @@ enum { > * 32 and 64 bit systems, don't break this! > */ > struct hwtstamp_config { > + int rw; Why not 'bool'? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, May 11, 2013 at 05:31:43PM +0200, Richard Cochran wrote: > On Sat, May 11, 2013 at 10:02:19PM +0800, Dong Zhu wrote: > > > > Currently kernel only support setting the hw time stamping policy > > through ioctl,now add a method to check which packets(Outgoing and > > Incoming) are time stamped by nic. > > I don't really see a use case here. Applications needing time stamping > should just set the policy that they need. I think it is necessary to check which type of packets are stamped by nic. For I350 (igb), it only support HWTSTAMP_FILTER_NONE and HWTSTAMP_FILTER_ALL of outgoing packets.For 82599EB(ixgbe), we can check more. I add a new member of hwtstamp_config to judge the ioctl request is read or write, we can specify the rw in the userspace using hwstamp_ctl application to get the time stamped info.
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7e8c477..8c06346 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -577,6 +577,34 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, if (config.flags) return -EINVAL; + if (config.rw == 0) + goto set_policy; + else if (config.rw == 1) { + if (config.tx_type || config.rx_filter) + return -EINVAL; + + regval = rd32(E1000_TSYNCTXCTL); + + if (regval & E1000_TSYNCTXCTL_ENABLED) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) + config.rx_filter = HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) + config.rx_filter = HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + goto end; + } else + return -EINVAL; + +set_policy: switch (config.tx_type) { case HWTSTAMP_TX_OFF: tsync_tx_ctl = 0; @@ -707,6 +735,7 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, regval = rd32(E1000_RXSTMPL); regval = rd32(E1000_RXSTMPH); +end: return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? -EFAULT : 0; } diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h index ae5df12..77147da 100644 --- a/include/uapi/linux/net_tstamp.h +++ b/include/uapi/linux/net_tstamp.h @@ -28,9 +28,10 @@ enum { /** * struct hwtstamp_config - %SIOCSHWTSTAMP parameter * + * @rw: 0/1 represents set/get the hw time stamp policy * @flags: no flags defined right now, must be zero * @tx_type: one of HWTSTAMP_TX_* - * @rx_type: one of one of HWTSTAMP_FILTER_* + * @rx_filter: one of one of HWTSTAMP_FILTER_* * * %SIOCSHWTSTAMP expects a &struct ifreq with a ifr_data pointer to * this structure. dev_ifsioc() in the kernel takes care of the @@ -39,6 +40,7 @@ enum { * 32 and 64 bit systems, don't break this! */ struct hwtstamp_config { + int rw; int flags; int tx_type; int rx_filter;