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

[Simon Horman <horms@kernel.org>]

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@intel.com>
> > Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> > Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> > Signed-off-by: Milena Olech <milena.olech@intel.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.
>