From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::344; helo=mail-wm1-x344.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5F912211CFFCD for ; Thu, 28 Feb 2019 02:15:01 -0800 (PST) Received: by mail-wm1-x344.google.com with SMTP id n19so8590361wmi.1 for ; Thu, 28 Feb 2019 02:15:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=00ccG7+gC76ptAY57+ElBBzJQQunxJU7vTRoDfas35U=; b=lvzgoJSC1UYQU7zorWI7zW2yJWOwU6yn+dtsD4eNVoK2A8c7sEZzBYPIQQIACPP6wH rMY/sOODl/eHmqNFpMppPOAXtx7HIoPlq3qoSL3rZ0nloQfPIle0ulH1id44QcbPZjIO q8JNilK4rvB4XF3tp4hnr4I3k0oV98LMvodTmrQ69t2N7MbKckpBVlATeaeoIIgaZlqJ +YR2caQVhAVBpCySBMPOlDxQz5/aqrWycA7LAZdL81yKWYf5m9a5dNS5y+380sFOUzkx kL/Uq70Hh225jTV6YwYiJC5GkZW+GUXjr9ZdXyp9RKWDiichifF99AzOtss4/DTCcb6C br8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=00ccG7+gC76ptAY57+ElBBzJQQunxJU7vTRoDfas35U=; b=SXeRTekZ2q3c9aVzwvecdL3JsF58v5/fnzsf5W1/mef4jji7BJsnI6SOXV6eUQ9c+M TwOmqkEjrEH/fVdPd/wdnyDK9WHPCSE+TZjv5ATrss6vAikOtG33rZ//62j6gQ0FiQGY s73kIp34suwYDkZrUxGGt1JzDXQDIDQ/C7caE2BOtPNJoMHBkc6X2ypPvy3EshhA7+DW pjmNWJLSRoOINdF/JtcwSHnGDRczxUt9i1QhnyJL1udzuatFwlic3p3kwjWLTHA87dVw gGk8oEBCiAxf73UzTvE/UrMGSI1IWO6fQb/+NeyPfwePu+/bTZzWs07LU0wQdVktoZHv l59A== X-Gm-Message-State: APjAAAXEpa8LNBzVhuvDUIGOtKtibADLOd7pcD/gPSNvDwCOFdkrJfcc iIJN33LOS/xWmah9A3EZUFhefQ== X-Google-Smtp-Source: APXvYqzzu1UOgJNqv+XOPjnsQEBJtb/cd07eCyVRI0UwE95RDEz2qz8M5wcYtW7q5N+XI38O+NOQ+Q== X-Received: by 2002:a7b:c34c:: with SMTP id l12mr910312wmj.126.1551348899465; Thu, 28 Feb 2019 02:14:59 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id z15sm33717671wrh.18.2019.02.28.02.14.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 28 Feb 2019 02:14:58 -0800 (PST) Date: Thu, 28 Feb 2019 10:14:56 +0000 From: Leif Lindholm To: "Ooi, Tzy Way" Cc: "edk2-devel@lists.01.org" , "Loh, Tien Hock" , "ard.biesheuvel@linaro.org" , "See, Chin Liang" Message-ID: <20190228101456.2ljaify5nknatzgi@bivouac.eciton.net> References: <20190131083200.4231-1-tzy.way.ooi@intel.com> <20190131182241.ano4hlyoj2kjcxoe@bivouac.eciton.net> <4cd9903090f81e376a055ecd7692dc35e1a0551c.camel@intel.com> MIME-Version: 1.0 In-Reply-To: <4cd9903090f81e376a055ecd7692dc35e1a0551c.camel@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH] EmbeddedPkg/DwEmacSnpDxe: Add designware emac support This add support for designware emac controller X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 10:15:01 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Tzy Way, On Thu, Feb 28, 2019 at 08:39:32AM +0000, Ooi, Tzy Way wrote: > Thanks for your comment. I will modify the driver to comply to UEFI > driver model. Excellent, thanks. > 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. This was the decision we made when formalising the process for bringing hardware support into TianoCore, whereas the edk2 repository would strictly import support for industry-standard components. With DesignWare, a case could perhaps be made that like ARM PrimeCells, they exist in many different devices from many different vendors. But if so, EmbeddedPkg is not their natural home. Some of the drivers already under EmbeddedPkg clearly violate this definition, but that is because they predate the definition of the process. Hmm, I probably should add a Readme.md about this in EmbeddedPkg/Drivers. Best Regards, Leif > 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" > > > > > > Contributed-under: TianCore Contribution Agreement 1.1 > > > Signed-off-by: Ooi, Tzy Way > > > --- > > > .../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