> -----Original Message-----
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Varghese, Vipin
> Sent: Thursday, December 6, 2018 2:41 PM
> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 01/20] net/ice: add base code
>
> snipped
> > > > > > >
> > > > > > > I this README persistent in upcoming releases of 'driver/net/ice'?
> > > > > > Yes.
> > > > > If Linux driver is enabled in 4.20.1 or higher, then will the
> > > > > wording 'This directory contains source code of FreeBSD ice
> > > > > driver of' still
> > > hold true?
> > > > Although I don't understand why we talk about the Linux driver
> > > > version, but I think the answer is yes.
> > > Ok, reason for linux driver is because 1. you would be planning to
> > > push the default kernel driver to linux for ICE.
> > > 2. the documentation states FreeBSD 2018.10.30, so if there is
> > > future enhancement pulled from linux driver this would added here too?
> > Sure, we'll keep updating this code.
> thanks
>
> >
> > >
> > > >
> > > > >
> > > > > > >
> > > > > snipped
> > > > > > > > +#define ICE_AQC_MAN_MAC_UPDATE_LAA 0
> > > > > > > > +#define ICE_AQC_MAN_MAC_UPDATE_LAA_WOL (BIT(0) <<
> > > > > > > > ICE_AQC_MAN_MAC_WR_S)
> > > > > > >
> > > > > > > Can the code be rearranged for?
> > > > > > We don’t want to change the base code for the sake of maintenance.
> > > > > I do not follow this, is not your team or individual maintaining the
> > > > > same?
> > > > > because there should be maintainer for this PMD.
> > > > This code is not implemented by us. You can take us as a
> > > > representative of the develop team.
> > > > If any bug, we'll hande it.
> > > Ok, currently the team which maintains the code do not want to
> > > change the order of code for sake of maintenance. Confusing
> > > approach, but I leave this to other members to comment.
> > >
> > > >
> > > > >
> Snipped
>
> > > > > > >
> > > > > > > Does the NIC support physical loopback? I am not able to find
> > > > > > > here.
> > > > > > Not sure about it. But no plan for this at this stage.
> > > > > Please add this in release note and PMD documentation the same.
> > > > No, we list all the things done. It doesn't make sense to list
> > > > everything not supported or not implemented.
> > > I think it is necessary, because application 'testpmd' has option to
> > > 'set tx loopback (port_id) (on|off)'. So If ICE DSI PMD does not
> > > support it and in testpmd it fails both DTS team and DPDK user
> > > should be made aware via documentation for limitation.
> > If the feature is not supported, the not supported failure is
> > returned. That's a RTE layer design and common solution for all the devices.
> I am more concerned about DPDK error values and DTS. If DTS uses the
> loopback as pass case it should pass and if it is feature not supported it
> should
> be documented.
>
> Note: In version 1 I enquired about unit or DTS validation for PMD. Is this
> still
> holding good?
I agree it's better to document the feature that is supported or not, actually
we have this mechanism in DPDK doc.
I think the gap here is there is no tx loopback related description in
doc/guides/nics/features.rst
Driver is only responsible to update the doc/guides/nics/features/ice.ini for
the features that be include in features.rst.
So the issue is not related with this patch from my view.
>
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > > +#define ICE_AQ_PHY_ENA_LOW_POWER BIT(2)
> > > > > > >
> > > > > > > Does Low Power PMD is exposed to DPDK? If yes, can you
> > > > > > > mention the performance numbers or variance in Release
> documents?
> > > > > > No plan for it at this release.
> > > > > Would not it be better to not add here or update as comment and
> > > > > release note about the same. Right now it is like dead code.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Snipped
> > > > > > >
> > > > > > > > +
> > > > > > > > +/* Memory types */
> > > > > > > > +enum ice_memset_type {
> > > > > > > > + ICE_NONDMA_MEM = 0,
> > > > > > > > + ICE_DMA_MEM
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +/* Memcpy types */
> > > > > > > > +enum ice_memcpy_type {
> > > > > > > > + ICE_NONDMA_TO_NONDMA = 0,
> > > > > > > > + ICE_NONDMA_TO_DMA,
> > > > > > > > + ICE_DMA_TO_DMA,
> > > > > > > > + ICE_DMA_TO_NONDMA
> > > > > > > > +};
> > > > > > > > +
> > > > > > >
> > > > > > > Is this exposed to user (rte_eth_dev) API? If yes, can you
> > > > > > > please let know the performance impact in RX|TX in release notes
> too.
> > > > > > No plan for it at this release.
> > > > > Please update
> > > > > a. what is difference least as comments.
> > > > > b. in release notes about the same.
> > > > >
> > > > > snipped
