RE: [PATCH v9 09/14] baseband/acc: add LTE processing functions

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

Hi Akhil, Maxime, 

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Subject: RE: [PATCH v9 09/14] baseband/acc: add LTE processing functions
> 
> > > > +/* Enqueue one encode operations for ACC200 device in TB mode. */
> > > > +static inline int enqueue_enc_one_op_tb(struct acc_queue *q,
> > > > +struct rte_bbdev_enc_op *op,
> > > > +		uint16_t total_enqueued_cbs, uint8_t cbs_in_tb) {
> > > > +	union acc_dma_desc *desc = NULL;
> > > > +	int ret;
> > > > +	uint8_t r, c;
> > > > +	uint32_t in_offset, out_offset, out_length, mbuf_total_left,
> > > > +		seg_total_left;
> > > > +	struct rte_mbuf *input, *output_head, *output;
> > > > +	uint16_t current_enqueued_cbs = 0;
> > > > +
> > > > +	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_cbs)
> > > > +			& q->sw_ring_wrap_mask);
> > >
> > > Maybe I did not make the comment on this patch specifically, but
> > > having a helper to get the descriptor index would make sense givent
> > > it is duplicated at several places.
> > >
> > >
> > > With this fixed, you can add:
> >
> > It is a good idea, notably for readability. But unsure we need it now
> > for 22.11 with still a lot of acc100 and acc200 commits in flight.
> > Are you okay if we to defer this small refactor to 23.03?
> > There are few similar routines which may benefit from similar wrapper
> > functions.
> > Let me know what you think.
> >
> We have time of atleast 2 weeks from now to close RC2 and all these acc
> patches.
> Do you think you need time more than that? I believe this is a simple code
> movement.

Fmhpov I am not sure this is a good practice to creep in some new changes during this window.
This is not super trivial change with always risk to break things for limited value (readability) and impact the 2nd serie, hence ideally I would have preferred to refactor in the next window cycle. 
Also we would much appreciate this acc200 serie to be applied soon if possible as the 2nd serie is likely to require a non-straight-forward rebase to be applied and is considered at risk at the moment due to this. 
Still no problem, just saying this of sharing our perspective => I did add that change in the new v10.
Thanks again, much appreciated.
Nic