[edk2-devel] [PATCH] UsbNetworkPkg: add USB network devices support
Chang, Abner via groups.io
abner.chang=amd.com at groups.io
Fri Sep 9 07:06:58 UTC 2022
[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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Rebecca
> Cran via groups.io
> Sent: Saturday, September 3, 2022 5:48 AM
> To: devel at edk2.groups.io; richardho at ami.com
> Cc: Andrew Fish <afish at apple.com>; Leif Lindholm
> <quic_llindhol at quicinc.com>; Michael D Kinney
> <michael.d.kinney at intel.com>; Michael Kubacki
> <michael.kubacki at microsoft.com>; Zhiguang Liu <zhiguang.liu at intel.com>;
> Liming Gao <gaoliming at byosoft.com.cn>; TonyLo [羅金松]
> <TonyLo at 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> 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 at ami.com>
> > Cc: Andrew Fish <afish at apple.com>
> > Cc: Leif Lindholm <quic_llindhol at quicinc.com>
> > Cc: Michael D Kinney <michael.d.kinney at intel.com>
> > Cc: Michael Kubacki <michael.kubacki at microsoft.com>
> > Cc: Zhiguang Liu <zhiguang.liu at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Reviewed-by: Tony Lo <tonylo at 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
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93569): https://edk2.groups.io/g/devel/message/93569
Mute This Topic: https://groups.io/mt/93121092/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list