| Message ID | 20240313-igc_txts_comment-v1-1-4e8438739323@linutronix.de (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net] igc: Remove stale comment about Tx timestamping | expand |
Kurt Kanzenbach <kurt@linutronix.de> writes: > The initial igc Tx timestamping implementation used only one register for > retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for > multiple in-flight TX timestamps") added support for utilizing all four of > them e.g., for multiple domain support. Remove the stale comment/FIXME. > > Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > --- Ugh, sorry for forgetting about that. Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Cheers,
On 3/13/24 14:03, Kurt Kanzenbach wrote: > The initial igc Tx timestamping implementation used only one register for > retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for > multiple in-flight TX timestamps") added support for utilizing all four of > them e.g., for multiple domain support. Remove the stale comment/FIXME. > > Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") I would remove fixes tag (but keep the mention in commit msg). And I would also target it to iwl-next when the window will open. Rationale: it's really not a fix. > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > --- > drivers/net/ethernet/intel/igc/igc_main.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c > index 2e1cfbd82f4f..35ad40a803cb 100644 > --- a/drivers/net/ethernet/intel/igc/igc_main.c > +++ b/drivers/net/ethernet/intel/igc/igc_main.c > @@ -1642,10 +1642,6 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb, > > if (unlikely(test_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags) && > skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) { > - /* FIXME: add support for retrieving timestamps from > - * the other timer registers before skipping the > - * timestamping request. > - */ > unsigned long flags; > u32 tstamp_flags; > > > --- > base-commit: 67072c314f5f0ec12a7a51a19f7156eebb073654 > change-id: 20240313-igc_txts_comment-81629dfc8b8a > > Best regards,
On 2024-03-14 11:21:38 [+0100], Przemek Kitszel wrote: > On 3/13/24 14:03, Kurt Kanzenbach wrote: > > The initial igc Tx timestamping implementation used only one register for > > retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for > > multiple in-flight TX timestamps") added support for utilizing all four of > > them e.g., for multiple domain support. Remove the stale comment/FIXME. > > > > Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") > > I would remove fixes tag (but keep the mention in commit msg). > And I would also target it to iwl-next when the window will open. > > Rationale: it's really not a fix. It is a fix as it removes something that is not accurate. But it only changes a comment so it has not outcome in the binary. I think what you mean is that you wish that it will not be backported stable. Still people reading the code of a v6.6 kernel might get confused. Sebastian
On 3/14/24 11:35, Sebastian Andrzej Siewior wrote: > On 2024-03-14 11:21:38 [+0100], Przemek Kitszel wrote: >> On 3/13/24 14:03, Kurt Kanzenbach wrote: >>> The initial igc Tx timestamping implementation used only one register for >>> retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for >>> multiple in-flight TX timestamps") added support for utilizing all four of >>> them e.g., for multiple domain support. Remove the stale comment/FIXME. >>> >>> Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") >> >> I would remove fixes tag (but keep the mention in commit msg). >> And I would also target it to iwl-next when the window will open. >> >> Rationale: it's really not a fix. > > It is a fix as it removes something that is not accurate. But it only > changes a comment so it has not outcome in the binary. I think what you > mean is that you wish that it will not be backported stable. Still > people reading the code of a v6.6 kernel might get confused. > > Sebastian > You are right that this will cause no harm to backport it as is too, I'm fine with that after a second though, so: Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
On 3/13/2024 15:03, Kurt Kanzenbach wrote: > The initial igc Tx timestamping implementation used only one register for > retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for > multiple in-flight TX timestamps") added support for utilizing all four of > them e.g., for multiple domain support. Remove the stale comment/FIXME. > > Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > --- > drivers/net/ethernet/intel/igc/igc_main.c | 4 ---- > 1 file changed, 4 deletions(-) Tested-by: Naama Meir <naamax.meir@linux.intel.com>
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 2e1cfbd82f4f..35ad40a803cb 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -1642,10 +1642,6 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb, if (unlikely(test_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags) && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) { - /* FIXME: add support for retrieving timestamps from - * the other timer registers before skipping the - * timestamping request. - */ unsigned long flags; u32 tstamp_flags;
The initial igc Tx timestamping implementation used only one register for retrieving Tx timestamps. Commit 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") added support for utilizing all four of them e.g., for multiple domain support. Remove the stale comment/FIXME. Fixes: 3ed247e78911 ("igc: Add support for multiple in-flight TX timestamps") Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> --- drivers/net/ethernet/intel/igc/igc_main.c | 4 ---- 1 file changed, 4 deletions(-) --- base-commit: 67072c314f5f0ec12a7a51a19f7156eebb073654 change-id: 20240313-igc_txts_comment-81629dfc8b8a Best regards,