| Message ID | 20190516100028.48256-2-yangbo.lu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ENETC: support hardware timestamping | expand |
>-----Original Message----- >From: Y.b. Lu [...] >Subject: [PATCH 1/3] enetc: add hardware timestamping support > [...] Hi Yangbo, These enetc patches targeting net-next will have to be rebased on the latest enetc net.git commits, otherwise there will be some merge conflicts for enetc.c and enetc_ethtool.c. Thanks, Claudiu see 22fb43f36006 "enetc: Add missing link state info for ethtool" f4a0be84d73e "enetc: Fix NULL dma address unmap for Tx BD extensions"
On Thu, May 16, 2019 at 09:59:08AM +0000, Y.b. Lu wrote: > +config FSL_ENETC_HW_TIMESTAMPING > + bool "ENETC hardware timestamping support" > + depends on FSL_ENETC || FSL_ENETC_VF > + help > + Enable hardware timestamping support on the Ethernet packets > + using the SO_TIMESTAMPING API. Because the RX BD ring dynamic > + allocation hasn't been supported and it's too expensive to use s/it's/it is/ > + extended RX BDs if timestamping isn't used, the option was used > + to control hardware timestamping/extended RX BDs to be enabled > + or not. ..., this option enables extended RX BDs in order to support hardware timestamping. > static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget) > { > struct net_device *ndev = tx_ring->ndev; > + struct enetc_ndev_priv *priv = netdev_priv(ndev); > int tx_frm_cnt = 0, tx_byte_cnt = 0; > struct enetc_tx_swbd *tx_swbd; > + union enetc_tx_bd *txbd; > + bool do_tstamp; > int i, bds_to_clean; > + u64 tstamp = 0; Please keep in reverse Christmas tree order as much as possible: union enetc_tx_bd *txbd; int i, bds_to_clean; bool do_tstamp; u64 tstamp = 0; > i = tx_ring->next_to_clean; > tx_swbd = &tx_ring->tx_swbd[i]; > bds_to_clean = enetc_bd_ready_count(tx_ring, i); > > + do_tstamp = false; > + > while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) { > bool is_eof = !!tx_swbd->skb; > > + if (unlikely(tx_swbd->check_wb)) { > + txbd = ENETC_TXBD(*tx_ring, i); > + > + if (!(txbd->flags & ENETC_TXBD_FLAGS_W)) > + goto no_wb; > + > + if (tx_swbd->do_tstamp) { > + enetc_get_tx_tstamp(&priv->si->hw, txbd, > + &tstamp); > + do_tstamp = true; > + } > + } > +no_wb: This goto seems strange and unnecessary. How about this instead? if (txbd->flags & ENETC_TXBD_FLAGS_W && tx_swbd->do_tstamp) { enetc_get_tx_tstamp(&priv->si->hw, txbd, &tstamp); do_tstamp = true; } > enetc_unmap_tx_buff(tx_ring, tx_swbd); > if (is_eof) { > + if (unlikely(do_tstamp)) { > + enetc_tstamp_tx(tx_swbd->skb, tstamp); > + do_tstamp = false; > + } > napi_consume_skb(tx_swbd->skb, napi_budget); > tx_swbd->skb = NULL; > } > @@ -167,6 +169,11 @@ struct enetc_cls_rule { > > #define ENETC_MAX_BDR_INT 2 /* fixed to max # of available cpus */ > > +enum enetc_hw_features { This is a poor choice of name. It sounds like it describes HW capabilities, but you use it to track whether a feature is requested at run time. > + ENETC_F_RX_TSTAMP = BIT(0), > + ENETC_F_TX_TSTAMP = BIT(1), > +}; > + > struct enetc_ndev_priv { > struct net_device *ndev; > struct device *dev; /* dma-mapping device */ > @@ -178,6 +185,7 @@ struct enetc_ndev_priv { > u16 rx_bd_count, tx_bd_count; > > u16 msg_enable; > + int hw_features; This is also poorly named. How about "tstamp_request" instead? > > struct enetc_bdr *tx_ring[16]; > struct enetc_bdr *rx_ring[16]; Thanks, Richard
>-----Original Message----- >From: Richard Cochran <richardcochran@gmail.com> >Sent: Thursday, May 16, 2019 5:33 PM >To: Y.b. Lu <yangbo.lu@nxp.com> >Cc: netdev@vger.kernel.org; David Miller <davem@davemloft.net>; Claudiu >Manoil <claudiu.manoil@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Rob >Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-arm- >kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Subject: Re: [PATCH 1/3] enetc: add hardware timestamping support > >On Thu, May 16, 2019 at 09:59:08AM +0000, Y.b. Lu wrote: > [...] > >> static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget) >> { >> struct net_device *ndev = tx_ring->ndev; >> + struct enetc_ndev_priv *priv = netdev_priv(ndev); >> int tx_frm_cnt = 0, tx_byte_cnt = 0; >> struct enetc_tx_swbd *tx_swbd; >> + union enetc_tx_bd *txbd; >> + bool do_tstamp; >> int i, bds_to_clean; >> + u64 tstamp = 0; > >Please keep in reverse Christmas tree order as much as possible: For the xmass tree part, Yangbo, better move the priv and txbd declarations inside the scope of the if() {} block where they are actually used, i.e.: if (unlikely(tx_swbd->check_wb)) { struct enetc_ndev_priv *priv = netdev_priv(ndev); union enetc_tx_bd *txbd; [...] } > > union enetc_tx_bd *txbd; > int i, bds_to_clean; > bool do_tstamp; > u64 tstamp = 0; > >> i = tx_ring->next_to_clean; >> tx_swbd = &tx_ring->tx_swbd[i]; >> bds_to_clean = enetc_bd_ready_count(tx_ring, i); >> >> + do_tstamp = false; >> + >> while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) { >> bool is_eof = !!tx_swbd->skb; >> >> + if (unlikely(tx_swbd->check_wb)) { >> + txbd = ENETC_TXBD(*tx_ring, i); >> + >> + if (!(txbd->flags & ENETC_TXBD_FLAGS_W)) >> + goto no_wb; >> + >> + if (tx_swbd->do_tstamp) { >> + enetc_get_tx_tstamp(&priv->si->hw, txbd, >> + &tstamp); >> + do_tstamp = true; >> + } >> + } >> +no_wb: > >This goto seems strange and unnecessary. How about this instead? > > if (txbd->flags & ENETC_TXBD_FLAGS_W && > tx_swbd->do_tstamp) { > enetc_get_tx_tstamp(&priv->si->hw, txbd, &tstamp); > do_tstamp = true; > } > Absolutely, somehow I missed this. I guess the intention was to be able to support multiple if() blocks for the writeback case (W flag set) but the code is much better off without the goto. >> enetc_unmap_tx_buff(tx_ring, tx_swbd); >> if (is_eof) { >> + if (unlikely(do_tstamp)) { >> + enetc_tstamp_tx(tx_swbd->skb, tstamp); >> + do_tstamp = false; >> + } >> napi_consume_skb(tx_swbd->skb, napi_budget); >> tx_swbd->skb = NULL; >> } >> @@ -167,6 +169,11 @@ struct enetc_cls_rule { >> >> #define ENETC_MAX_BDR_INT 2 /* fixed to max # of available cpus */ >> >> +enum enetc_hw_features { > >This is a poor choice of name. It sounds like it describes HW >capabilities, but you use it to track whether a feature is requested >at run time. > >> + ENETC_F_RX_TSTAMP = BIT(0), >> + ENETC_F_TX_TSTAMP = BIT(1), >> +}; >> + >> struct enetc_ndev_priv { >> struct net_device *ndev; >> struct device *dev; /* dma-mapping device */ >> @@ -178,6 +185,7 @@ struct enetc_ndev_priv { >> u16 rx_bd_count, tx_bd_count; >> >> u16 msg_enable; >> + int hw_features; > >This is also poorly named. How about "tstamp_request" instead? > This ndev_priv variable was intended to gather flags for all the active h/w related features, i.e. keeping count of what h/w offloads are enabled for the current device (at least for those that don't have already a netdev_features_t flag). I wouldn't waste an int for 2 timestamp flags, I'd rather have a more generic name. Maybe active_offloads then? Anyway, the name can be changed later too, when other offloads will be added. Thanks, Claudiu
> -----Original Message----- > From: Claudiu Manoil > Sent: Thursday, May 16, 2019 9:31 PM > To: Y.b. Lu <yangbo.lu@nxp.com>; netdev@vger.kernel.org; Richard Cochran > <richardcochran@gmail.com>; David Miller <davem@davemloft.net>; Shawn > Guo <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org> > Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org > Subject: RE: [PATCH 1/3] enetc: add hardware timestamping support > > >-----Original Message----- > >From: Y.b. Lu > [...] > >Subject: [PATCH 1/3] enetc: add hardware timestamping support > > > [...] > > Hi Yangbo, > > These enetc patches targeting net-next will have to be rebased on the latest > enetc net.git commits, otherwise there will be some merge conflicts for enetc.c > and enetc_ethtool.c. > Thanks, > Claudiu > > see > 22fb43f36006 "enetc: Add missing link state info for ethtool" > f4a0be84d73e "enetc: Fix NULL dma address unmap for Tx BD extensions" [Y.b. Lu] Thanks Claudiu. I should have noticed that. Let me send v2 after net-next is open.
Hi, > -----Original Message----- > From: Richard Cochran <richardcochran@gmail.com> > Sent: Thursday, May 16, 2019 10:33 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev@vger.kernel.org; David Miller <davem@davemloft.net>; Claudiu > Manoil <claudiu.manoil@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Rob > Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: [EXT] Re: [PATCH 1/3] enetc: add hardware timestamping support > > > On Thu, May 16, 2019 at 09:59:08AM +0000, Y.b. Lu wrote: > > > +config FSL_ENETC_HW_TIMESTAMPING > > + bool "ENETC hardware timestamping support" > > + depends on FSL_ENETC || FSL_ENETC_VF > > + help > > + Enable hardware timestamping support on the Ethernet packets > > + using the SO_TIMESTAMPING API. Because the RX BD ring dynamic > > + allocation hasn't been supported and it's too expensive to use > > s/it's/it is/ [Y.b. Lu] Will modify it. BTW, may I know what's the purpose of dropping single quote character? For searching, script checking, or something else? If require to not use single quote character, I will also modify some other places in Kconfig messages. > > > + extended RX BDs if timestamping isn't used, the option was used > > + to control hardware timestamping/extended RX BDs to be enabled > > + or not. > > ..., this option enables extended RX BDs in order to support hardware > timestamping. [Y.b. Lu] Will rephrase it. > > > static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int > > napi_budget) { > > struct net_device *ndev = tx_ring->ndev; > > + struct enetc_ndev_priv *priv = netdev_priv(ndev); > > int tx_frm_cnt = 0, tx_byte_cnt = 0; > > struct enetc_tx_swbd *tx_swbd; > > + union enetc_tx_bd *txbd; > > + bool do_tstamp; > > int i, bds_to_clean; > > + u64 tstamp = 0; > > Please keep in reverse Christmas tree order as much as possible: > > union enetc_tx_bd *txbd; > int i, bds_to_clean; > bool do_tstamp; > u64 tstamp = 0; > > > i = tx_ring->next_to_clean; > > tx_swbd = &tx_ring->tx_swbd[i]; > > bds_to_clean = enetc_bd_ready_count(tx_ring, i); > > > > + do_tstamp = false; > > + > > while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) { > > bool is_eof = !!tx_swbd->skb; > > > > + if (unlikely(tx_swbd->check_wb)) { > > + txbd = ENETC_TXBD(*tx_ring, i); > > + > > + if (!(txbd->flags & ENETC_TXBD_FLAGS_W)) > > + goto no_wb; > > + > > + if (tx_swbd->do_tstamp) { > > + enetc_get_tx_tstamp(&priv->si->hw, txbd, > > + &tstamp); > > + do_tstamp = true; > > + } > > + } > > +no_wb: > > This goto seems strange and unnecessary. How about this instead? > > if (txbd->flags & ENETC_TXBD_FLAGS_W && > tx_swbd->do_tstamp) { > enetc_get_tx_tstamp(&priv->si->hw, txbd, > &tstamp); > do_tstamp = true; > } > > > enetc_unmap_tx_buff(tx_ring, tx_swbd); > > if (is_eof) { > > + if (unlikely(do_tstamp)) { > > + enetc_tstamp_tx(tx_swbd->skb, tstamp); > > + do_tstamp = false; > > + } > > napi_consume_skb(tx_swbd->skb, napi_budget); > > tx_swbd->skb = NULL; > > } > > @@ -167,6 +169,11 @@ struct enetc_cls_rule { > > > > #define ENETC_MAX_BDR_INT 2 /* fixed to max # of available cpus */ > > > > +enum enetc_hw_features { > > This is a poor choice of name. It sounds like it describes HW capabilities, but > you use it to track whether a feature is requested at run time. > > > + ENETC_F_RX_TSTAMP = BIT(0), > > + ENETC_F_TX_TSTAMP = BIT(1), > > +}; > > + > > struct enetc_ndev_priv { > > struct net_device *ndev; > > struct device *dev; /* dma-mapping device */ @@ -178,6 +185,7 > @@ > > struct enetc_ndev_priv { > > u16 rx_bd_count, tx_bd_count; > > > > u16 msg_enable; > > + int hw_features; > > This is also poorly named. How about "tstamp_request" instead? > > > > > struct enetc_bdr *tx_ring[16]; > > struct enetc_bdr *rx_ring[16]; > > Thanks, > Richard
Hi, > -----Original Message----- > From: Claudiu Manoil > Sent: Thursday, May 16, 2019 11:31 PM > To: Richard Cochran <richardcochran@gmail.com>; Y.b. Lu > <yangbo.lu@nxp.com> > Cc: netdev@vger.kernel.org; David Miller <davem@davemloft.net>; Shawn > Guo <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org > Subject: RE: [PATCH 1/3] enetc: add hardware timestamping support > > > >-----Original Message----- > >From: Richard Cochran <richardcochran@gmail.com> > >Sent: Thursday, May 16, 2019 5:33 PM > >To: Y.b. Lu <yangbo.lu@nxp.com> > >Cc: netdev@vger.kernel.org; David Miller <davem@davemloft.net>; Claudiu > >Manoil <claudiu.manoil@nxp.com>; Shawn Guo <shawnguo@kernel.org>; > Rob > >Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-arm- > >kernel@lists.infradead.org; linux-kernel@vger.kernel.org > >Subject: Re: [PATCH 1/3] enetc: add hardware timestamping support > > > >On Thu, May 16, 2019 at 09:59:08AM +0000, Y.b. Lu wrote: > > > [...] > > > >> static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int > >> napi_budget) { > >> struct net_device *ndev = tx_ring->ndev; > >> + struct enetc_ndev_priv *priv = netdev_priv(ndev); > >> int tx_frm_cnt = 0, tx_byte_cnt = 0; > >> struct enetc_tx_swbd *tx_swbd; > >> + union enetc_tx_bd *txbd; > >> + bool do_tstamp; > >> int i, bds_to_clean; > >> + u64 tstamp = 0; > > > >Please keep in reverse Christmas tree order as much as possible: > > For the xmass tree part, Yangbo, better move the priv and txbd declarations > inside the scope of the if() {} block where they are actually used, i.e.: > > if (unlikely(tx_swbd->check_wb)) { > struct enetc_ndev_priv *priv = netdev_priv(ndev); > union enetc_tx_bd *txbd; > [...] > } > [Y.b. Lu] Will do that. > > > > union enetc_tx_bd *txbd; > > int i, bds_to_clean; > > bool do_tstamp; > > u64 tstamp = 0; > > > >> i = tx_ring->next_to_clean; > >> tx_swbd = &tx_ring->tx_swbd[i]; > >> bds_to_clean = enetc_bd_ready_count(tx_ring, i); > >> > >> + do_tstamp = false; > >> + > >> while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) { > >> bool is_eof = !!tx_swbd->skb; > >> > >> + if (unlikely(tx_swbd->check_wb)) { > >> + txbd = ENETC_TXBD(*tx_ring, i); > >> + > >> + if (!(txbd->flags & ENETC_TXBD_FLAGS_W)) > >> + goto no_wb; > >> + > >> + if (tx_swbd->do_tstamp) { > >> + enetc_get_tx_tstamp(&priv->si->hw, txbd, > >> + &tstamp); > >> + do_tstamp = true; > >> + } > >> + } > >> +no_wb: > > > >This goto seems strange and unnecessary. How about this instead? > > > > if (txbd->flags & ENETC_TXBD_FLAGS_W && > > tx_swbd->do_tstamp) { > > enetc_get_tx_tstamp(&priv->si->hw, txbd, &tstamp); > > do_tstamp = true; > > } > > > > Absolutely, somehow I missed this. I guess the intention was to be able to > support multiple > if() blocks for the writeback case (W flag set) but the code is much better off > without the goto. [Y.b. Lu] Will use this to support current single tstamp writeback case. > > >> enetc_unmap_tx_buff(tx_ring, tx_swbd); > >> if (is_eof) { > >> + if (unlikely(do_tstamp)) { > >> + enetc_tstamp_tx(tx_swbd->skb, tstamp); > >> + do_tstamp = false; > >> + } > >> napi_consume_skb(tx_swbd->skb, napi_budget); > >> tx_swbd->skb = NULL; > >> } > >> @@ -167,6 +169,11 @@ struct enetc_cls_rule { > >> > >> #define ENETC_MAX_BDR_INT 2 /* fixed to max # of available cpus */ > >> > >> +enum enetc_hw_features { > > > >This is a poor choice of name. It sounds like it describes HW > >capabilities, but you use it to track whether a feature is requested at > >run time. > > > >> + ENETC_F_RX_TSTAMP = BIT(0), > >> + ENETC_F_TX_TSTAMP = BIT(1), > >> +}; > >> + > >> struct enetc_ndev_priv { > >> struct net_device *ndev; > >> struct device *dev; /* dma-mapping device */ @@ -178,6 +185,7 @@ > >> struct enetc_ndev_priv { > >> u16 rx_bd_count, tx_bd_count; > >> > >> u16 msg_enable; > >> + int hw_features; > > > >This is also poorly named. How about "tstamp_request" instead? > > > > This ndev_priv variable was intended to gather flags for all the active h/w > related features, i.e. keeping count of what h/w offloads are enabled for the > current device (at least for those that don't have already a netdev_features_t > flag). > I wouldn't waste an int for 2 timestamp flags, I'd rather have a more generic > name. > Maybe active_offloads then? > > Anyway, the name can be changed later too, when other offloads will be > added. [Y.b. Lu] How about using active_offloads, and add TODO comments in enum enetc_active_offloads? > > Thanks, > Claudiu
On Mon, May 20, 2019 at 03:20:23AM +0000, Y.b. Lu wrote: > > > +config FSL_ENETC_HW_TIMESTAMPING > > > + bool "ENETC hardware timestamping support" > > > + depends on FSL_ENETC || FSL_ENETC_VF > > > + help > > > + Enable hardware timestamping support on the Ethernet packets > > > + using the SO_TIMESTAMPING API. Because the RX BD ring dynamic > > > + allocation hasn't been supported and it's too expensive to use > > > > s/it's/it is/ > > [Y.b. Lu] Will modify it. BTW, may I know what's the purpose of dropping single quote character? For searching, script checking, or something else? Simply because "it's" is informal speech, but the Kconfig help is formal technical documentation. (Or at least it should be!) Thanks, Richard
diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig index 8429f5c1d810..fc6f30d2ef91 100644 --- a/drivers/net/ethernet/freescale/enetc/Kconfig +++ b/drivers/net/ethernet/freescale/enetc/Kconfig @@ -29,3 +29,14 @@ config FSL_ENETC_PTP_CLOCK packets using the SO_TIMESTAMPING API. If compiled as module (M), the module name is fsl-enetc-ptp. + +config FSL_ENETC_HW_TIMESTAMPING + bool "ENETC hardware timestamping support" + depends on FSL_ENETC || FSL_ENETC_VF + help + Enable hardware timestamping support on the Ethernet packets + using the SO_TIMESTAMPING API. Because the RX BD ring dynamic + allocation hasn't been supported and it's too expensive to use + extended RX BDs if timestamping isn't used, the option was used + to control hardware timestamping/extended RX BDs to be enabled + or not. diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 5bb9eb35d76d..2137973daba9 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -13,7 +13,8 @@ #define ENETC_MAX_SKB_FRAGS 13 #define ENETC_TXBDS_MAX_NEEDED ENETC_TXBDS_NEEDED(ENETC_MAX_SKB_FRAGS + 1) -static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb); +static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb, + int hw_features); netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev) { @@ -33,7 +34,7 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_BUSY; } - count = enetc_map_tx_buffs(tx_ring, skb); + count = enetc_map_tx_buffs(tx_ring, skb, priv->hw_features); if (unlikely(!count)) goto drop_packet_err; @@ -105,7 +106,8 @@ static void enetc_free_tx_skb(struct enetc_bdr *tx_ring, } } -static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb) +static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb, + int hw_features) { struct enetc_tx_swbd *tx_swbd; struct skb_frag_struct *frag; @@ -137,7 +139,10 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb) count++; do_vlan = skb_vlan_tag_present(skb); - do_tstamp = skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP; + do_tstamp = (hw_features & ENETC_F_TX_TSTAMP) && + (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP); + tx_swbd->do_tstamp = do_tstamp; + tx_swbd->check_wb = tx_swbd->do_tstamp; if (do_vlan || do_tstamp) flags |= ENETC_TXBD_FLAGS_EX; @@ -299,22 +304,68 @@ static int enetc_bd_ready_count(struct enetc_bdr *tx_ring, int ci) return pi >= ci ? pi - ci : tx_ring->bd_count - ci + pi; } +static void enetc_get_tx_tstamp(struct enetc_hw *hw, union enetc_tx_bd *txbd, + u64 *tstamp) +{ + u32 lo, hi; + + lo = enetc_rd(hw, ENETC_SICTR0); + hi = enetc_rd(hw, ENETC_SICTR1); + if (lo <= txbd->wb.tstamp) + hi -= 1; + *tstamp = (u64)hi << 32 | txbd->wb.tstamp; +} + +static void enetc_tstamp_tx(struct sk_buff *skb, u64 tstamp) +{ + struct skb_shared_hwtstamps shhwtstamps; + + if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) { + memset(&shhwtstamps, 0, sizeof(shhwtstamps)); + shhwtstamps.hwtstamp = ns_to_ktime(tstamp); + skb_tstamp_tx(skb, &shhwtstamps); + } +} + static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget) { struct net_device *ndev = tx_ring->ndev; + struct enetc_ndev_priv *priv = netdev_priv(ndev); int tx_frm_cnt = 0, tx_byte_cnt = 0; struct enetc_tx_swbd *tx_swbd; + union enetc_tx_bd *txbd; + bool do_tstamp; int i, bds_to_clean; + u64 tstamp = 0; i = tx_ring->next_to_clean; tx_swbd = &tx_ring->tx_swbd[i]; bds_to_clean = enetc_bd_ready_count(tx_ring, i); + do_tstamp = false; + while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) { bool is_eof = !!tx_swbd->skb; + if (unlikely(tx_swbd->check_wb)) { + txbd = ENETC_TXBD(*tx_ring, i); + + if (!(txbd->flags & ENETC_TXBD_FLAGS_W)) + goto no_wb; + + if (tx_swbd->do_tstamp) { + enetc_get_tx_tstamp(&priv->si->hw, txbd, + &tstamp); + do_tstamp = true; + } + } +no_wb: enetc_unmap_tx_buff(tx_ring, tx_swbd); if (is_eof) { + if (unlikely(do_tstamp)) { + enetc_tstamp_tx(tx_swbd->skb, tstamp); + do_tstamp = false; + } napi_consume_skb(tx_swbd->skb, napi_budget); tx_swbd->skb = NULL; } @@ -423,10 +474,34 @@ static int enetc_refill_rx_ring(struct enetc_bdr *rx_ring, const int buff_cnt) return j; } +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING +static void enetc_get_rx_tstamp(struct net_device *ndev, + union enetc_rx_bd *rxbd, + struct sk_buff *skb) +{ + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); + struct enetc_ndev_priv *priv = netdev_priv(ndev); + struct enetc_hw *hw = &priv->si->hw; + u32 lo, hi; + u64 tstamp; + + if (rxbd->r.flags & ENETC_RXBD_FLAG_TSTMP) { + lo = enetc_rd(hw, ENETC_SICTR0); + hi = enetc_rd(hw, ENETC_SICTR1); + if (lo <= rxbd->r.tstamp) + hi -= 1; + + tstamp = (u64)hi << 32 | rxbd->r.tstamp; + memset(shhwtstamps, 0, sizeof(*shhwtstamps)); + shhwtstamps->hwtstamp = ns_to_ktime(tstamp); + } +} +#endif + static void enetc_get_offloads(struct enetc_bdr *rx_ring, union enetc_rx_bd *rxbd, struct sk_buff *skb) { - /* TODO: add tstamp, hashing */ + /* TODO: hashing */ if (rx_ring->ndev->features & NETIF_F_RXCSUM) { u16 inet_csum = le16_to_cpu(rxbd->r.inet_csum); @@ -440,6 +515,10 @@ static void enetc_get_offloads(struct enetc_bdr *rx_ring, if (le16_to_cpu(rxbd->r.flags) & ENETC_RXBD_FLAG_VLAN) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), le16_to_cpu(rxbd->r.vlan_opt)); +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING + if (rx_ring->ndev->hw_features & ENETC_F_RX_TSTAMP) + enetc_get_rx_tstamp(rx_ring->ndev, rxbd, skb); +#endif } static void enetc_process_skb(struct enetc_bdr *rx_ring, @@ -1072,6 +1151,9 @@ static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring) enetc_rxbdr_wr(hw, idx, ENETC_RBICIR0, ENETC_RBICIR0_ICEN | 0x1); rbmr = ENETC_RBMR_EN; +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING + rbmr |= ENETC_RBMR_BDS; +#endif if (rx_ring->ndev->features & NETIF_F_HW_VLAN_CTAG_RX) rbmr |= ENETC_RBMR_VTE; @@ -1394,6 +1476,70 @@ int enetc_set_features(struct net_device *ndev, return 0; } +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING +static int enetc_hwtstamp_set(struct net_device *ndev, struct ifreq *ifr) +{ + struct enetc_ndev_priv *priv = netdev_priv(ndev); + struct hwtstamp_config config; + + if (copy_from_user(&config, ifr->ifr_data, sizeof(config))) + return -EFAULT; + + switch (config.tx_type) { + case HWTSTAMP_TX_OFF: + priv->hw_features &= ~ENETC_F_TX_TSTAMP; + break; + case HWTSTAMP_TX_ON: + priv->hw_features |= ENETC_F_TX_TSTAMP; + break; + default: + return -ERANGE; + } + + switch (config.rx_filter) { + case HWTSTAMP_FILTER_NONE: + priv->hw_features &= ~ENETC_F_RX_TSTAMP; + break; + default: + priv->hw_features |= ENETC_F_RX_TSTAMP; + config.rx_filter = HWTSTAMP_FILTER_ALL; + } + + return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? + -EFAULT : 0; +} + +static int enetc_hwtstamp_get(struct net_device *ndev, struct ifreq *ifr) +{ + struct enetc_ndev_priv *priv = netdev_priv(ndev); + struct hwtstamp_config config; + + config.flags = 0; + + if (priv->hw_features & ENETC_F_TX_TSTAMP) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + config.rx_filter = (priv->hw_features & ENETC_F_RX_TSTAMP) ? + HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE; + + return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? + -EFAULT : 0; +} +#endif + +int enetc_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd) +{ +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING + if (cmd == SIOCSHWTSTAMP) + return enetc_hwtstamp_set(ndev, rq); + if (cmd == SIOCGHWTSTAMP) + return enetc_hwtstamp_get(ndev, rq); +#endif + return -EINVAL; +} + int enetc_alloc_msix(struct enetc_ndev_priv *priv) { struct pci_dev *pdev = priv->si->pdev; diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h index b274135c5103..8c63ea253ab2 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.h +++ b/drivers/net/ethernet/freescale/enetc/enetc.h @@ -21,7 +21,9 @@ struct enetc_tx_swbd { struct sk_buff *skb; dma_addr_t dma; u16 len; - u16 is_dma_page; + u8 is_dma_page:1; + u8 check_wb:1; + u8 do_tstamp:1; }; #define ENETC_RX_MAXFRM_SIZE ENETC_MAC_MAXFRM_SIZE @@ -167,6 +169,11 @@ struct enetc_cls_rule { #define ENETC_MAX_BDR_INT 2 /* fixed to max # of available cpus */ +enum enetc_hw_features { + ENETC_F_RX_TSTAMP = BIT(0), + ENETC_F_TX_TSTAMP = BIT(1), +}; + struct enetc_ndev_priv { struct net_device *ndev; struct device *dev; /* dma-mapping device */ @@ -178,6 +185,7 @@ struct enetc_ndev_priv { u16 rx_bd_count, tx_bd_count; u16 msg_enable; + int hw_features; struct enetc_bdr *tx_ring[16]; struct enetc_bdr *rx_ring[16]; @@ -216,6 +224,7 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev); struct net_device_stats *enetc_get_stats(struct net_device *ndev); int enetc_set_features(struct net_device *ndev, netdev_features_t features); +int enetc_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd); /* ethtool */ void enetc_set_ethtool_ops(struct net_device *ndev); diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h index df8eb8882d92..6559cef4b07d 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h @@ -361,6 +361,12 @@ union enetc_tx_bd { u8 e_flags; u8 flags; } ext; /* Tx BD extension */ + struct { + __le32 tstamp; + u8 reserved[10]; + u8 status; + u8 flags; + } wb; /* writeback descriptor */ }; #define ENETC_TXBD_FLAGS_L4CS BIT(0) @@ -399,6 +405,9 @@ union enetc_rx_bd { struct { __le64 addr; u8 reserved[8]; +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING + u8 reserved1[16]; +#endif } w; struct { __le16 inet_csum; @@ -413,6 +422,10 @@ union enetc_rx_bd { }; __le32 lstatus; }; +#ifdef CONFIG_FSL_ENETC_HW_TIMESTAMPING + __le32 tstamp; + u8 reserved[12]; +#endif } r; }; diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c index 15876a6e7598..17e9017cf897 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c @@ -702,6 +702,7 @@ static const struct net_device_ops enetc_ndev_ops = { .ndo_set_vf_vlan = enetc_pf_set_vf_vlan, .ndo_set_vf_spoofchk = enetc_pf_set_vf_spoofchk, .ndo_set_features = enetc_pf_set_features, + .ndo_do_ioctl = enetc_ioctl, }; static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev, diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c index 64bebee9f52a..af93cb1af513 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c @@ -111,6 +111,7 @@ static const struct net_device_ops enetc_ndev_ops = { .ndo_get_stats = enetc_get_stats, .ndo_set_mac_address = enetc_vf_set_mac_addr, .ndo_set_features = enetc_vf_set_features, + .ndo_do_ioctl = enetc_ioctl, }; static void enetc_vf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
This patch is to add hardware timestamping support for ENETC. On Rx, timestamping is enabled for all frames. On Tx, we only instruct the hardware to timestamp the frames marked accordingly by the stack. Because the RX BD ring dynamic allocation hasn't been supported and it's too expensive to use extended RX BDs if timestamping isn't used, we have to use a Kconfig option to enable/disable timestamping for now. This Kconfig option will be removed once RX BD ring dynamic allocation is implemented. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/net/ethernet/freescale/enetc/Kconfig | 11 ++ drivers/net/ethernet/freescale/enetc/enetc.c | 156 +++++++++++++++++- drivers/net/ethernet/freescale/enetc/enetc.h | 11 +- .../net/ethernet/freescale/enetc/enetc_hw.h | 13 ++ .../net/ethernet/freescale/enetc/enetc_pf.c | 1 + .../net/ethernet/freescale/enetc/enetc_vf.c | 1 + 6 files changed, 187 insertions(+), 6 deletions(-)