From: "RichardHo [何明忠]" <richardho@ami.com>
To: "Chang, Abner" <Abner.Chang@amd.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"quic_rcran@quicinc.com" <quic_rcran@quicinc.com>
Cc: "Andrew Fish" <afish@apple.com>,
"Leif Lindholm" <quic_llindhol@quicinc.com>,
"Michael D Kinney" <michael.d.kinney@intel.com>,
"Michael Kubacki" <michael.kubacki@microsoft.com>,
"Zhiguang Liu" <zhiguang.liu@intel.com>,
"Liming Gao" <gaoliming@byosoft.com.cn>,
"TonyLo [羅金松]" <TonyLo@ami.com>
Subject: Re: [edk2-devel] [PATCH] UsbNetworkPkg: add USB network devices support
Date: Mon, 12 Sep 2022 03:48:18 +0000 [thread overview]
Message-ID: <DM5PR10MB1660767C4E72E63721675BD2B0449@DM5PR10MB1660.namprd10.prod.outlook.com> (raw)
In-Reply-To: <MN2PR12MB3966B96BA4292D1CD107DCDEEA439@MN2PR12MB3966.namprd12.prod.outlook.com>
Hi Abner,
Ans #1. I think the USB network feature is better in the UsbNetworkPkg. It is easy to control USB network stack in this package.
The package is same as NetworkPkg(The NetworkPkg gathers all network stack features). The UsbNetworkPkg could gather the USB network stack.
Ans #2. OK. I will rename it next.
Ans #4 Other driver could use EFI_OPEN_PROTOCOL_BY_DRIVER with USB_ETHERNET_PROTOCOL to own on the specific USB CDC device.
But, the specific USB CDC driver need to install the binding driver before NetworkCommon driver.
Ans #5 NetworkCommon driver build a Mac device path that link PciIo device path.
So, SNP driver could found PciIo device path when NetworkCommon driver to install the EFI_NETWORK_INTERFACE_IDENTIFIER_PROTOCOL.
Thanks,
Richard
-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: 2022年9月9日 3:07 PM
To: devel@edk2.groups.io; quic_rcran@quicinc.com; RichardHo [何明忠] <RichardHo@ami.com>
Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney <michael.d.kinney@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>; Zhiguang Liu <zhiguang.liu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; TonyLo [羅金松] <TonyLo@ami.com>
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add USB network devices support
**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**
[AMD Official Use Only - General]
Hi Richard,
That is pretty hard to give the in-line comment in this huge patch email. You would be also hard to find the comments given to each module or the protocol header file.
So fist all of, please organize your change into several patches. For example, 1. UsbEthernetProtocol.h 2. UsbRndis module 3. UsbCdcNcm module 4. UsbCdcEcm module 5. NetworkCommon module
Some rough feedbacks to this change before I giving the comments to each patch after you resending the new patch set.
1. I suggest having UsbNetwork folder under MdeModulePkg/Usb/. Then under UsbNetwork, you can have those four USB network related modules. However, we can Rename NetwrokCommon to UsbNetwork if your are ok with it.
2. UsbEhernetProtocol is not an EFI protocol, so please add EdkII as the prefix to UsbEthernetProtocol, it would be EdkIIUsbEthernetProtocol. And we can have UsbEthernetProtocol.h under MdeModulePkg/Include/Protocol 4. From this change, NetwrokCommon driver listens to USB_ETHERNET_PROTOCOL and install EFI NII protocol for each instance. However, the upper layer driver wouldn't know the CDC interface class/subclass/protocol if it only listen to USB_ETHERNET_PROTOCOL and install its own SNP on the specific USB CDC device; in the case if there are multiple USB CDC devices attached on the system. Is my understanding correct?
5. I don’t see the UsbEthernetProtocol based SNP protocol is installed and the SNP driver under NetworkPkg has the dependency with PciIo. Do we have to implement UsbEthernetProtocol based SNP by our own?
I will check the functionality on our platform if I get the chance.
Thanks
Abner
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca
> Cran via groups.io
> Sent: Saturday, September 3, 2022 5:48 AM
> To: devel@edk2.groups.io; richardho@ami.com
> Cc: Andrew Fish <afish@apple.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Michael Kubacki
> <michael.kubacki@microsoft.com>; Zhiguang Liu
> <zhiguang.liu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> TonyLo [羅金松]
> <TonyLo@ami.com>
> Subject: Re: [edk2-devel] [PATCH] UsbNetworkPkg: add USB network
> devices support
>
> [CAUTION: External Email]
>
> I've pushed this patch to a branch at
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam1
> 1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fgith
> &data=05%7C01%7Crichardho%40ami.com%7C1ffd2a8dbf6f4cda9da908da9231
> e426%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C637983040232493161%7
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XK3dw8vGMtln2Ig6Rcieyw6l
> orLdsvy2Oko9gAllSK0%3D&reserved=0
> ub.com%2Fbcran%2Fedk2%2Ftree%2Fusb-
> net&data=05%7C01%7Cabner.chang%40amd.com%7C2c812dc15b9542b
> 8a64308da8d2cdf2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
> 637977521120662673%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7
> C&sdata=ZFcSw3AL0MnljvyODrv1dYAL8LuadwCYe65xhE1hXWY%3D&a
> mp;reserved=0 .
>
> --
> Rebecca Cran
>
> On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote:
> > UsbNetworkPkg provides network functions for USB ACM, USB NCM, and
> USB
> > RNDIS network device.
> >
> > Signed-off-by: Richard Ho <richardho@ami.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Reviewed-by: Tony Lo <tonylo@ami.com>
> > ---
> > UsbNetworkPkg/Config/UsbNetworkPkg.inc.dsc | 9 +
> > .../Config/UsbNetworkPkgComponentsDxe.inc.dsc | 20 +
> > .../Config/UsbNetworkPkgComponentsDxe.inc.fdf | 20 +
> > .../Config/UsbNetworkPkgDefines.inc.dsc | 23 +
> > .../Include/Protocol/UsbEthernetProtocol.h | 872 +++++++++
> > UsbNetworkPkg/NetworkCommon/ComponentName.c | 264 +++
> > UsbNetworkPkg/NetworkCommon/DriverBinding.c | 583 ++++++
> > UsbNetworkPkg/NetworkCommon/DriverBinding.h | 263 +++
> > UsbNetworkPkg/NetworkCommon/NetworkCommon.inf | 43 +
> > UsbNetworkPkg/NetworkCommon/PxeFunction.c | 1734
> +++++++++++++++++
> > UsbNetworkPkg/ReadMe.md | 65 +
> > UsbNetworkPkg/ReleaseNotes.md | 11 +
> > UsbNetworkPkg/UsbCdcEcm/ComponentName.c | 170 ++
> > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c | 504 +++++
> > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h | 211 ++
> > UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf | 41 +
> > UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c | 861 ++++++++
> > UsbNetworkPkg/UsbCdcNcm/ComponentName.c | 170 ++
> > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c | 508 +++++
> > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h | 245 +++
> > UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf | 41 +
> > UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c | 946 +++++++++
> > UsbNetworkPkg/UsbNetworkPkg.dec | 32 +
> > UsbNetworkPkg/UsbRndis/ComponentName.c | 172 ++
> > UsbNetworkPkg/UsbRndis/UsbRndis.c | 848 ++++++++
> > UsbNetworkPkg/UsbRndis/UsbRndis.h | 569 ++++++
> > UsbNetworkPkg/UsbRndis/UsbRndis.inf | 41 +
> > UsbNetworkPkg/UsbRndis/UsbRndisFunction.c | 1587
> +++++++++++++++
> > 28 files changed, 10853 insertions(+)
> > create mode 100644 UsbNetworkPkg/Config/UsbNetworkPkg.inc.dsc
> > create mode 100644
> UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
> > create mode 100644
> UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.fdf
> > create mode 100644
> UsbNetworkPkg/Config/UsbNetworkPkgDefines.inc.dsc
> > create mode 100644
> UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h
> > create mode 100644
> UsbNetworkPkg/NetworkCommon/ComponentName.c
> > create mode 100644 UsbNetworkPkg/NetworkCommon/DriverBinding.c
> > create mode 100644 UsbNetworkPkg/NetworkCommon/DriverBinding.h
> > create mode 100644
> UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
> > create mode 100644 UsbNetworkPkg/NetworkCommon/PxeFunction.c
> > create mode 100644 UsbNetworkPkg/ReadMe.md
> > create mode 100644 UsbNetworkPkg/ReleaseNotes.md
> > create mode 100644 UsbNetworkPkg/UsbCdcEcm/ComponentName.c
> > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c
> > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h
> > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf
> > create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c
> > create mode 100644 UsbNetworkPkg/UsbCdcNcm/ComponentName.c
> > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c
> > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h
> > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf
> > create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c
> > create mode 100644 UsbNetworkPkg/UsbNetworkPkg.dec
> > create mode 100644 UsbNetworkPkg/UsbRndis/ComponentName.c
> > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.c
> > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.h
> > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndis.inf
> > create mode 100644 UsbNetworkPkg/UsbRndis/UsbRndisFunction.c
>
>
>
>
>
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
next prev parent reply other threads:[~2022-09-12 3:48 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-02 5:24 [PATCH] UsbNetworkPkg: add USB network devices support RichardHo [何明忠]
2022-09-02 21:48 ` [edk2-devel] " Rebecca Cran
2022-09-09 7:06 ` Chang, Abner
2022-09-12 3:48 ` RichardHo [何明忠] [this message]
2022-09-12 7:55 ` Chang, Abner
2022-09-21 2:34 ` RichardHo [何明忠]
2022-09-22 12:52 ` Chang, Abner
2022-09-06 15:05 ` Rebecca Cran
2022-09-07 5:31 ` RichardHo [何明忠]
2022-09-07 17:46 ` Rebecca Cran
2022-09-23 5:03 ` 回复: " gaoliming
2022-09-23 5:54 ` RichardHo [何明忠]
2022-09-23 7:17 ` 回复: " gaoliming
2022-11-26 18:56 ` Rebecca Cran
2022-11-28 5:24 ` RichardHo [何明忠]
2022-12-05 3:14 ` Rebecca Cran
2022-12-06 9:15 ` RichardHo [何明忠]
-- strict thread matches above, loose matches on Subject: below --
2022-08-31 7:51 RichardHo [何明忠]
2022-08-31 13:28 ` [edk2-devel] " Rebecca Cran
2022-09-01 2:51 ` RichardHo [何明忠]
2022-09-01 9:05 ` Chang, Abner
2022-09-01 10:47 ` RichardHo [何明忠]
2022-09-01 12:38 ` Rebecca Cran
2022-08-31 2:31 RichardHo [何明忠]
2022-08-31 12:15 ` [edk2-devel] " Rebecca Cran
2022-08-19 9:17 richardho
2022-08-19 15:54 ` [edk2-devel] " Pedro Falcato
2022-08-19 15:56 ` Pedro Falcato
2022-08-29 10:57 ` RichardHo [何明忠]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM5PR10MB1660767C4E72E63721675BD2B0449@DM5PR10MB1660.namprd10.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox