Re: [PATCH] EmbeddedPkg/DwEmacSnpDxe: Add designware emac support This add support for designware emac controller

[Leif Lindholm <leif.lindholm@linaro.org>]

Hi Tzy Way,

Thank you for this contribution.

I do have some high-level comments.

First of all, my best guess is that you have used Lan9118Dxe for
reference when developing this driver. This is somewhat unfortunate.
I am reminded that
a) we badly need to migrate that driver (and Lan91xDxe) to
   edk2-platforms.
b) we badly need to convert those drivers to UEFI driver model and
   NonDiscoverableDeviceRegistrationLib.
Those two predate the NonDiscoverable implementation, so have been
left as is, but any new drivers really need to implement proper driver
model.
Additionally, this driver should be submitted to edk2-platforms rather
than edk2.

Secondly, searching online for "designware emac" does not find
unambigously the product this refers to. This is where it would be
usful with a proper commit message and explain in a bit more detail
what the driver is.

On Thu, Jan 31, 2019 at 04:32:00PM +0800, tzy.way.ooi@intel.com wrote:
> From: "Ooi, Tzy Way" <tzy.way.ooi@intel.com>
> 
> Contributed-under: TianCore Contribution Agreement 1.1
> Signed-off-by: Ooi, Tzy Way <tzy.way.ooi@intel.com>
> ---
>  .../Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c       | 1368 +++++++++++++++++
>  .../Drivers/DwEmacSnpDxe/DwEmacSnpDxe.h       |  236 +++
>  .../Drivers/DwEmacSnpDxe/DwEmacSnpDxe.inf     |   69 +
>  .../Drivers/DwEmacSnpDxe/EmacDxeUtil.c        |  676 ++++++++
>  .../Drivers/DwEmacSnpDxe/EmacDxeUtil.h        |  378 +++++
>  EmbeddedPkg/Drivers/DwEmacSnpDxe/PhyDxeUtil.c |  604 ++++++++
>  EmbeddedPkg/Drivers/DwEmacSnpDxe/PhyDxeUtil.h |  324 ++++
>  EmbeddedPkg/EmbeddedPkg.dec                   |    4 +
>  EmbeddedPkg/EmbeddedPkg.dsc                   |    1 +
>  9 files changed, 3660 insertions(+)

Thirdly, please generate patches as described in
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
This greatly simplifies review.

Best Regards,

Leif