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

["Ooi, Tzy Way" <tzy.way.ooi@intel.com>]

Hi Leif,

Thanks for your comment. I will modify the driver to comply to UEFI
driver model.

I would like to ask why this driver should be submitted to edk2-
platforms instead of edk2? This driver is a generic driver which is
target to work on various platform.

Best regards,
Tzy Way


On Thu, 2019-01-31 at 18:22 +0000, Leif Lindholm wrote:
> 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