On Tue, Mar 29, 2016 at 08:52:38PM +0000, Rasesh Mody wrote:

Hi Bruce,

quoted

From: Bruce Richardson [mailto:bruce.richardson@intel.com]
Sent: Tuesday, March 22, 2016 4:30 AM

On Tue, Mar 22, 2016 at 11:21:25AM +0000, Richardson, Bruce wrote:

quoted

I've had a quick scan over this patchset, and as you've probably seen I've

made some public comments on it. General comments on the whole
patchset are:

quoted

* Please run checkpatch on the patchset and clear up as many issues as
you can. There are a number of typos called out which especially must
be fixed. Both myself and Thomas always run checkpatch against patches
before applying them. [I suggest using Thomas's checkpatches.sh script
to do the checks as it disables many unnecessary warnings from
checkpatch]
* Please put in commit descriptions for all patches bar those doing trivial

things. The first three patches probably don't need a commit message, but
the rest do.

quoted

/Bruce

quoted

-----Original Message-----
From: Rasesh Mody [mailto:rasesh.mody@qlogic.com]
Sent: Saturday, March 19, 2016 12:53 AM
To: thomas.monjalon@6wind.com; Richardson, Bruce
[off-list ref]
Cc: dev@dpdk.org; ameen.rahman@qlogic.com; harish.patil@qlogic.com;
sony.chacko@qlogic.com; Rasesh Mody [off-list ref]
Subject: [PATCH v3 00/10] qede: Add qede PMD

Submitting v3 patch series for QEDE PMD. There is no code change
from v2 series except PMD version change. Earlier we had generated
and tested the
v2 series against dpdk tree then latest.

The v3 series includes:
 - Patches generated and tested against latest dpdk-next-net
 - Reworked MAINTAINERS patch to make it apply cleanly
 - Incorporated Overview.rst update in the documentation patch

Please Apply.

Thanks!
Rasesh

Rasesh Mody (10):
  qede: Add maintainers
  qede: Add documentation
  qede: Add license file
  qede: Add base driver
  qede: Add core driver
  qede: Add L2 support
  qede: Add SRIOV support
  qede: Add attention support
  qede: Add DCBX support
  qede: Enable PMD build

Clang gives a compile error after applying this patchset. Please investigate.

== Build drivers/net/qede
  CC base/ecore_dev.o
fatal error: unknown warning option '-Wno-shift-negative-value'; did you
mean
      '-Wno-shift-sign-overflow'? [-Wunknown-warning-option]
/home/bruce/next-net/dpdk-next-net/mk/internal/rte.compile-
pre.mk:126: recipe for target 'base/ecore_dev.o' failed

This is seen with clang 3.6 on Fedora 23.
	"clang version 3.6.0 (tags/RELEASE_360/final)"

We had compiled all our previously submitted driver patches against clang v3.8 on  RH7.1 and didn't see a similar error. The '-Wno-shift-negative-value' option got introduced after 3.6.0 release.  Pls. let us know if we need to support a minimum version of clang. 
        "clang version 3.8.0 (trunk 249596) (llvm/trunk 249595)"
 

Thanks for the update. I'm not sure that we have a minimum version of clang that
we have to support - we tend to test more with gcc than clang. If the latest
clang version most common distro's compiles this fine, I'd say it's ok. [The latest
on Fedora 23 is actually 3.7, so we are ok with that].
Anyone in the community have a minimum clang version that they need DPDK to
support?

quoted

Similarly, 32-bit (i686) build fails:

== Build drivers/net/qede
  CC base/ecore_dev.o
/home/bruce/next-net/dpdk-next-
net/drivers/net/qede/base/ecore_dev.c: In function
‘ecore_chain_alloc_sanity_check’:
/home/bruce/next-net/dpdk-next-
net/drivers/net/qede/base/ecore_dev.c:2571:32: error: format ‘%lx’
expects argument of type ‘long unsigned int’, but argument 6 has type ‘u64
{aka long long unsigned int}’ [-Werror=format=] compilation terminated due
to -Wfatal-errors.
cc1: all warnings being treated as errors

This is seen with gcc 5.3.1 on Fedora 23.

For 32-bit, we are preparing our driver to compile against gcc version 4.3.4.

Similary, we had compile tested all our previously submitted driver patches on FreeBSD using:
        FreeBSD clang version 3.6.1 and
        gcc version 4.8.5

Great, thanks.

/Bruce

Thanks!
Rasesh

quoted

Regards,
/Bruce