Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Thursday, September 22, 2022 9:49 AM
> To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
> tho...@monjalon.net; hemant.agra...@nxp.com
> Cc: maxime.coque...@redhat.com; t...@redhat.com; m...@ashroe.eu;
> Richardson, Bruce <bruce.richard...@intel.com>;
> david.march...@redhat.com; step...@networkplumber.org; Zhang,
> Mingshan <mingshan.zh...@intel.com>
> Subject: RE: [EXT] [PATCH v7 5/7] bbdev: add new operation for FFT
> processing
>
> Hi Nicolas,
> > > > > > +The structure passed for each FFT operation is given below,
> > > > > > +with the operation flags forming a bitmask in the ``op_flags``
> > > > > > field.
> > > > > > +
> > > > > > +.. code-block:: c
> > > > > > +
> > > > > > + struct rte_bbdev_op_fft {
> > > > > > + struct rte_bbdev_op_data base_input;
> > > > > > + struct rte_bbdev_op_data base_output;
> > > > > > + struct rte_bbdev_op_data power_meas_output;
> > > > > > + uint32_t op_flags;
> > > > > > + uint16_t input_sequence_size;
> > > > > > + uint16_t input_leading_padding;
> > > > > > + uint16_t output_sequence_size;
> > > > > > + uint16_t output_leading_depadding;
> > > > > > + uint8_t window_index[RTE_BBDEV_MAX_CS_2];
> > > > > > + uint16_t cs_bitmap;
> > > > > > + uint8_t num_antennas_log2;
> > > > > > + uint8_t idft_log2;
> > > > > > + uint8_t dft_log2;
> > > > > > + int8_t cs_time_adjustment;
> > > > > > + int8_t idft_shift;
> > > > > > + int8_t dft_shift;
> > > > > > + uint16_t ncs_reciprocal;
> > > > > > + uint16_t power_shift;
> > > > > > + uint16_t fp16_exp_adjust;
> > > > > > + };
> > > > >
> > > > > Why is this codeblock added in this guide? Isn't it covered in
> > > > > the doxygen API doc?
> > > >
> > > > It is but here this detailed in context and with additional details.
> > > > Note that this is the exact same format being used for all the
> > > > other existing operations.
> > > > Are you okay to keep as is?
> > >
> > > I would suggest to remove it and others as well. I think we can add
> > > more info In the API guide if needed.
> > > Otherwise it will be difficult to maintain/track in future for any
> > > changes that are done in these structs.
> >
> > Do we really want to do this now?
> > That documentation is tightly tied with the API, so the doc would
> > always have to be updated when the API structure is modified.
> > If you read this through most of the documentation would become barely
> > readable without direct pointer to the relevant structure and related
> > parameters (not just for FFT but also for the existing operations).
> > I see also some code snippets insertions for other parallel
> > documentation when relevant.
> > I can see your point about avoid duplicates but in that very case it
> > is arguably sensible to be readable and would personally not change it
> > in so far as I am concerned.
> >
> > Are you okay to keep current documentation flow for now as is, and we
> > can look into improving/updating documentation as a future item based
> > on agreed guidance?
> >
> The maintenance of this documentation is difficult in longer run.
> It will be unnecessary burden for the developer to sync both code and
> documentation.
>
> We have seen this thing in past that people tend to forget about updating
> documentation.
> And we are trying to remove unnecessary code snippets from the
> documentation.
> We should not add new such code in documentation.
> I would say remove this one from this patch. And you can submit a patch
> later to remove the rest.
OK will remove the C code-block and push a v8 now. Thanks.
>
> Regards,
> Akhil
