netdev - Re: [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities negotiation

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241115124515.GO1062410@kernel.org>
Date: Fri, 15 Nov 2024 12:45:15 +0000
From: Simon Horman <horms@...nel.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Milena Olech <milena.olech@...el.com>, intel-wired-lan@...ts.osuosl.org,
	netdev@...r.kernel.org, anthony.l.nguyen@...el.com,
	przemyslaw.kitszel@...el.com,
	Emil Tantilov <emil.s.tantilov@...el.com>,
	Pavan Kumar Linga <pavan.kumar.linga@...el.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>
Subject: Re: [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities
 negotiation

On Thu, Nov 14, 2024 at 03:49:27PM -0500, Willem de Bruijn wrote:
> Milena Olech wrote:
> > Tx timestamp capabilities are negotiated for the uplink Vport.
> > Driver receives information about the number of available Tx timestamp
> > latches, the size of Tx timestamp value and the set of indexes used
> > for Tx timestamping.
> > 
> > Add function to get the Tx timestamp capabilities and parse the uplink
> > vport flag.
> > 
> > Co-developed-by: Emil Tantilov <emil.s.tantilov@...el.com>
> > Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
> > Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> > Reviewed-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> > Signed-off-by: Milena Olech <milena.olech@...el.com>
> 
> A few minor points. No big concerns from me.
> 
> >  struct idpf_vc_xn_manager;
> >  
> > +#define idpf_for_each_vport(adapter, iter) \
> > +	for (struct idpf_vport **__##iter = &(adapter)->vports[0], \
> > +	     *iter = *__##iter; \
> > +	     __##iter < &(adapter)->vports[(adapter)->num_alloc_vports]; \
> > +	     iter = *(++__##iter))
> > +
> 
> Perhaps more readable to just use an int:
> 
>     for (int i = 0; iter = &(adapter)->vports[i], i < (adapter)->num_alloc_vports; i++)
> 
> >  /**
> > @@ -517,6 +524,60 @@ static int idpf_ptp_create_clock(const struct idpf_adapter *adapter)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * idpf_ptp_release_vport_tstamp - Release the Tx timestamps trakcers for a
> 
> s/trakcers/trackers
> 
> > +/**
> > + * struct idpf_ptp_tx_tstamp - Parametrs for Tx timestamping
> 
> s/Parametrs/Parameters
> 
> > + * @list_member: the list member strutcure
> 
> s/strutcure/Structure
> 
> Please use a spell checker, don't rely on reviewers.

To add to that:

* Capabilities is misspelt in the subject
* checkpatch.pl --codespell will spell-check the patch

> 
> Also, going forward, IMHO documentation can be limited to APIs and
> non-obvious functions/structs/fields.
>