Hello.

On 11-05-2013 18:02, Dong Zhu wrote:

>  From e6a55411486de8a09b859e73140bf35c0ee36047 Mon Sep 17 00:00:00 2001
> From: Dong Zhu <[email protected]>
> 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 <[email protected]>
> ---
>   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


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and 
their applications. This 200-page book is written by three acclaimed 
leaders in the field. The early access version is available now. 
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit 
http://communities.intel.com/community/wired

Reply via email to