Hi Bruce, 

-----Original Message-----
From: Richardson, Bruce
Sent: Friday, February 12, 2016 6:36 AM
To: Lu, Wenzhuo
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 00/12] update ixgbe base driver

On Tue, Jan 19, 2016 at 10:48:01AM +0800, Wenzhuo Lu wrote:

quoted

*add new devices and mac type
*use PHY token for X550EM_a
*set VF mac address only when acked by PF *always turn PHY power on
when requested *add definition of a register for mac SGMII busy
*support fuse for both x550em_x V1 and V2 *remove duplicate setting
for fdir no match drop *consolidate MDIO speed-setting *use link
instead of I2C combined abstraction


Wenzhuo Lu (12):
  ixgbe/base: update readme
  ixgbe/base: add new devices and mac type
  ixgeb/base: use PHY token for X550EM_a
  ixgbe/base: set VF mac address only when acked by PF
  ixgbe/base: always turn PHY power on when requested
  ixgbe/base: add definition of a register for mac SGMII busy
  ixgbe/base: support fuse for both x550em_x V1 and V2
  ixgbe/base: remove duplicate setting for fdir no match drop
  ixgbe/base: consolidate MDIO speed-setting
  ixgbe/base: use link instead of I2C combined abstraction
  ixgbe: support new devices and mac types
  doc: update release note for ixgbe base code update

Hi Wenzhuo

Thanks for this patchset. Just some feedback on it.

* Can you please take the new features etc. called out in the doc patch and just
add the RN update as part of the patch adding the feature itself. Those additions
that were part of a previous base code update (presumably part of a previous
release) shouldn't go in the latest release notes. There shouldn't need to be a
separate doc patch in this set.
* The order of the patches could do with being changed. I'd suggest having the
readme updated as the last of the base code update series, right before the
ixgbe patch.
* The "new devices and mac types" patch, only seems to add one new device
type, so I'd call it out explicitly in the title.
* The X550em_x V1/V2 patch is similarly adding new device support so should go
alongside the other new device patch. I'd suggest titling both patches similarly
e.g.
	 ixgbe/base: add x550em_a device support
	 ixgbe/base: add x550em_x V2 device support

* Some of the titles are a bit long or are not a great summary of the primary
purpose of the patch. For example: "always turn PHY power on when requested"
might be better titled: "ignore manageability for phy power on".

Can you maybe send out a V2 with the above changes taken into account -
especially the release note split?

Sure, I'll send a V2. Thanks for the comments.

Thanks,
/Bruce