|
[AMD Official Use Only - General]
Hi Richard, thanks for the reply. My feedback in line below,
toggle quoted message
Show quoted text
-----Original Message-----
From: RichardHo [何明忠] <RichardHo@...>
Sent: Monday, September 12, 2022 11:48 AM
To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io;
quic_rcran@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu <zhiguang.liu@...>;
Liming Gao <gaoliming@...>; TonyLo [羅金松]
<TonyLo@...>
Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add USB
network devices support
[CAUTION: External Email]
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. UsbNetworkPkg has the feature that overlaps with Bus\Usb and \NetworkPkg in my opinion, that is the reason I think we can have these modules in the existing package. Plus as I know from the edk2 community view point, we only create a new package when necessary for a totally new feature. However, this is at the discretion of edk2 stewards. You can confirm this with them. 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. My question was USB_ETHERNET_PROTOCOL is designed as an abstract protocol, with this the underlying transport (UsbIo or PciIo) is agnostic to the upper layer driver. Upper layer driver (e.g., USB_ETHERNET_PROTOCOL based SNP driver) can check whether USB_ETHERNET_PROTOCOL is installed on the given Controller handle at Supported() function. However, seems the upper layer driver has no way to distinguish the USB CDC driver interface class if I read your code right; then manages the right device (in the case of multiple USB CDC devices are attached to the system). We can either attach USB Class Device Path to the device path in Usb CDC driver or provide a function to retrieve the USB CDC interface class/subclass/protocol via USB_ETHERNET_PROTOCOL. I would prefer the former one. Thanks Abner 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@...>
Sent: 2022年9月9日 3:07 PM
To: devel@edk2.groups.io; quic_rcran@...; RichardHo [何明忠]
<RichardHo@...>
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu <zhiguang.liu@...>;
Liming Gao <gaoliming@...>; TonyLo [羅金松]
<TonyLo@...>
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@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
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%2Fnam
1
2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fn
am1
&data=05%7C01%7CAbner.Chang%40amd.com%7C92e0dc34ec2a4c9a9f
6908da94
71a29e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63798551306
1024727
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
CJBTiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=78%2FyX9SBP
4wrftOYrmnP
muF5e%2F3NsmqY7JVw4atcoGM%3D&reserved=0
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fgi
th
&data=05%7C01%7Crichardho%40ami.com%7C1ffd2a8dbf6f4cda9da908
da9231
e426%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C63798304023249
3161%7
CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
BTiI6Ik1
haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XK3dw8vGMtln2
Ig6Rcieyw6l
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@...>
Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <michael.kubacki@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Liming Gao <gaoliming@...>
Reviewed-by: Tony Lo <tonylo@...>
---
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.
|
|
|
Hi Abner,
For #4, To see below code. RNDIS driver provide the RndisUndiStart routine.
It could execute RndisUndiStart in UndiStart routine end.
The upper layer driver not need to know the USB CDC driver interface class.
It only provide the USB_ETHERNET_PROTOCOL. The UNDI protocol is easy to control by NetworkCommon driver.
I have convert the USB_ETHERNET_PROTOCOL to EDKII_USB_ETHERNET_PROTOCOL, will send the patch next.
UsbRndisDevice->Signature = USB_RNDIS_SIGNATURE;
UsbRndisDevice->NumOfInterface = Interface.InterfaceNumber;
UsbRndisDevice->UsbRndisHandle = RndisHandle;
UsbRndisDevice->UsbCdcDataHandle = 0;
UsbRndisDevice->UsbIo = UsbIo;
// ------ Below is for USB_ETHERNET_PROTOCOL
UsbRndisDevice->UsbEth.UsbEthReceive = RndisUndiReceive;
UsbRndisDevice->UsbEth.UsbEthTransmit = RndisUndiTransmit;
UsbRndisDevice->UsbEth.UsbEthInterrupt = UsbRndisInterrupt;
UsbRndisDevice->UsbEth.UsbEthMacAddress = GetUsbEthMacAddress;
UsbRndisDevice->UsbEth.UsbEthMaxBulkSize = UsbEthBulkSize;
UsbRndisDevice->UsbEth.UsbHeaderFunDescriptor = GetUsbHeaderFunDescriptor;
UsbRndisDevice->UsbEth.UsbUnionFunDescriptor = GetUsbUnionFunDescriptor;
UsbRndisDevice->UsbEth.UsbEthFunDescriptor = GetUsbRndisFunDescriptor;
UsbRndisDevice->UsbEth.SetUsbEthMcastFilter = SetUsbRndisMcastFilter;
UsbRndisDevice->UsbEth.SetUsbEthPowerPatternFilter = SetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.GetUsbEthPoewrPatternFilter = GetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.SetUsbEthPacketFilter = SetUsbRndisPacketFilter;
UsbRndisDevice->UsbEth.GetUsbEthStatistic = GetRndisStatistic;
// ------- Below is for UNDI Hook
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetState = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStart = RndisUndiStart;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStop = RndisUndiStop;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetInitInfo = RndisUndiGetInitInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetConfigInfo = RndisUndiGetConfigInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInitialize = RndisUndiInitialize;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReset = RndisUndiReset;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiShutdown = RndisUndiShutdown;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInterruptEnable = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceiveFilter = RndisUndiReceiveFilter;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStationAddress = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStatistics = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiMcastIp2Mac = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiNvData = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetStatus = RndisUndiGetStatus;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiFillHeader = RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiTransmit = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceive = NULL;
UndiStart (
IN PXE_CDB *Cdb,
IN NIC_DATA *Nic
)
{
PXE_CPB_START_31 *Cpb;
EFI_STATUS Status;
if ((Cdb->OpCode != PXE_OPCODE_START) ||
(Cdb->StatCode != PXE_STATCODE_INITIALIZE) ||
(Cdb->StatFlags != PXE_STATFLAGS_INITIALIZE) ||
(Cdb->IFnum >= (gPxe->IFcnt | gPxe->IFcntExt << 8)) ||
(Cdb->CPBsize != sizeof (PXE_CPB_START_31)) ||
(Cdb->DBsize != PXE_DBSIZE_NOT_USED) ||
(Cdb->DBaddr != PXE_DBADDR_NOT_USED) ||
(Cdb->OpFlags != PXE_OPFLAGS_NOT_USED))
{
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_INVALID_CDB;
return;
} else {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_COMPLETE;
Cdb->StatCode = PXE_STATCODE_SUCCESS;
}
if (Nic->State != PXE_STATFLAGS_GET_STATE_STOPPED) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_ALREADY_STARTED;
return;
}
Cpb = (PXE_CPB_START_31 *)(UINTN)Cdb->CPBaddr;
Nic->PxeStart.Delay = Cpb->Delay;
Nic->PxeStart.Virt2Phys = Cpb->Virt2Phys;
Nic->PxeStart.Block = Cpb->Block;
Nic->PxeStart.Map_Mem = 0;
Nic->PxeStart.UnMap_Mem = 0;
Nic->PxeStart.Sync_Mem = Cpb->Sync_Mem;
Nic->PxeStart.Unique_ID = Cpb->Unique_ID;
Nic->State = PXE_STATFLAGS_GET_STATE_STARTED;
if (Nic->UsbEth->UsbEthUndi.UsbEthUndiStart != NULL) {
Status = Nic->UsbEth->UsbEthUndi.UsbEthUndiStart (Cdb, Nic); <--- RndisUndiStart
if (EFI_ERROR (Status)) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
}
}
}
Thanks,
Richard
toggle quoted message
Show quoted text
-----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io Sent: 2022年9月12日 3:56 PM To: RichardHo [何明忠] <RichardHo@...>; devel@edk2.groups.io; quic_rcran@... Cc: Andrew Fish <afish@...>; Leif Lindholm <quic_llindhol@...>; Michael D Kinney <michael.d.kinney@...>; Michael Kubacki <michael.kubacki@...>; Zhiguang Liu <zhiguang.liu@...>; Liming Gao <gaoliming@...>; TonyLo [羅金松] <TonyLo@...> 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, thanks for the reply. My feedback in line below, -----Original Message-----
From: RichardHo [何明忠] <RichardHo@...>
Sent: Monday, September 12, 2022 11:48 AM
To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io;
quic_rcran@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add
USB network devices support
[CAUTION: External Email]
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. UsbNetworkPkg has the feature that overlaps with Bus\Usb and \NetworkPkg in my opinion, that is the reason I think we can have these modules in the existing package. Plus as I know from the edk2 community view point, we only create a new package when necessary for a totally new feature. However, this is at the discretion of edk2 stewards. You can confirm this with them. 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. My question was USB_ETHERNET_PROTOCOL is designed as an abstract protocol, with this the underlying transport (UsbIo or PciIo) is agnostic to the upper layer driver. Upper layer driver (e.g., USB_ETHERNET_PROTOCOL based SNP driver) can check whether USB_ETHERNET_PROTOCOL is installed on the given Controller handle at Supported() function. However, seems the upper layer driver has no way to distinguish the USB CDC driver interface class if I read your code right; then manages the right device (in the case of multiple USB CDC devices are attached to the system). We can either attach USB Class Device Path to the device path in Usb CDC driver or provide a function to retrieve the USB CDC interface class/subclass/protocol via USB_ETHERNET_PROTOCOL. I would prefer the former one. Thanks Abner 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@...>
Sent: 2022年9月9日 3:07 PM
To: devel@edk2.groups.io; quic_rcran@...; RichardHo [何明忠]
<RichardHo@...>
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
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@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
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%252Fnam&
amp;data=05%7C01%7Cigork%40ami.com%7C2dbbb37cd5aa464372d408da94944389%
7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C637985661785139437%7CUnkn
own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6kJ7djzYAbWay75nEMfH2eATKEWC2
j%2FoFYakFE2PuJw%3D&reserved=0
1
2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fn
am1
&data=05%7C01%7CAbner.Chang%40amd.com%7C92e0dc34ec2a4c9a9f
6908da94
71a29e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63798551306
1024727
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
CJBTiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=78%2FyX9SBP
4wrftOYrmnP
muF5e%2F3NsmqY7JVw4atcoGM%3D&reserved=0
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fgi
th
&data=05%7C01%7Crichardho%40ami.com%7C1ffd2a8dbf6f4cda9da908
da9231
e426%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C63798304023249
3161%7
CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
BTiI6Ik1
haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XK3dw8vGMtln2
Ig6Rcieyw6l
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@...>
Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <michael.kubacki@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Liming Gao <gaoliming@...>
Reviewed-by: Tony Lo <tonylo@...>
---
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. -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.
|
|
|
[AMD Official Use Only - General]
Thanks Richard.
Please just send out the v2, I will give my feedback based on that.
Please run PatchCheck and Uncrustify on your patch before sending it out. Also, it would be helpful if you can organize your patches on driver based instead of having everything all together in one patch. It would be easier for us to review and also easier for you to address the comments.
Thanks
Abner
toggle quoted message
Show quoted text
-----Original Message-----
From: RichardHo [何明忠] <RichardHo@...>
Sent: Wednesday, September 21, 2022 10:35 AM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>;
quic_rcran@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney <michael.d.kinney@...>;
Michael Kubacki <michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>; TonyLo
[羅金松] <TonyLo@...>
Subject: RE: [EXTERNAL] Re: [edk2-devel] [PATCH] UsbNetworkPkg: add USB
network devices support
Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.
Hi Abner,
For #4, To see below code. RNDIS driver provide the RndisUndiStart routine.
It could execute RndisUndiStart in UndiStart routine end.
The upper layer driver not need to know the USB CDC driver interface
class.
It only provide the USB_ETHERNET_PROTOCOL. The UNDI protocol is
easy to control by NetworkCommon driver.
I have convert the USB_ETHERNET_PROTOCOL to
EDKII_USB_ETHERNET_PROTOCOL, will send the patch next.
UsbRndisDevice->Signature = USB_RNDIS_SIGNATURE;
UsbRndisDevice->NumOfInterface = Interface.InterfaceNumber;
UsbRndisDevice->UsbRndisHandle = RndisHandle;
UsbRndisDevice->UsbCdcDataHandle = 0;
UsbRndisDevice->UsbIo = UsbIo;
// ------ Below is for USB_ETHERNET_PROTOCOL
UsbRndisDevice->UsbEth.UsbEthReceive = RndisUndiReceive;
UsbRndisDevice->UsbEth.UsbEthTransmit = RndisUndiTransmit;
UsbRndisDevice->UsbEth.UsbEthInterrupt = UsbRndisInterrupt;
UsbRndisDevice->UsbEth.UsbEthMacAddress = GetUsbEthMacAddress;
UsbRndisDevice->UsbEth.UsbEthMaxBulkSize = UsbEthBulkSize;
UsbRndisDevice->UsbEth.UsbHeaderFunDescriptor =
GetUsbHeaderFunDescriptor;
UsbRndisDevice->UsbEth.UsbUnionFunDescriptor =
GetUsbUnionFunDescriptor;
UsbRndisDevice->UsbEth.UsbEthFunDescriptor =
GetUsbRndisFunDescriptor;
UsbRndisDevice->UsbEth.SetUsbEthMcastFilter = SetUsbRndisMcastFilter;
UsbRndisDevice->UsbEth.SetUsbEthPowerPatternFilter =
SetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.GetUsbEthPoewrPatternFilter =
GetUsbRndisPowerFilter;
UsbRndisDevice->UsbEth.SetUsbEthPacketFilter = SetUsbRndisPacketFilter;
UsbRndisDevice->UsbEth.GetUsbEthStatistic = GetRndisStatistic;
// ------- Below is for UNDI Hook
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetState =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStart = RndisUndiStart;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStop = RndisUndiStop;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetInitInfo =
RndisUndiGetInitInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetConfigInfo =
RndisUndiGetConfigInfo;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInitialize =
RndisUndiInitialize;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReset = RndisUndiReset;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiShutdown =
RndisUndiShutdown;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiInterruptEnable =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceiveFilter =
RndisUndiReceiveFilter;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStationAddress =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiStatistics = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiMcastIp2Mac =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiNvData =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiGetStatus =
RndisUndiGetStatus;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiFillHeader =
RndisDummyReturn;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiTransmit = NULL;
UsbRndisDevice->UsbEth.UsbEthUndi.UsbEthUndiReceive = NULL;
UndiStart (
IN PXE_CDB *Cdb,
IN NIC_DATA *Nic
)
{
PXE_CPB_START_31 *Cpb;
EFI_STATUS Status;
if ((Cdb->OpCode != PXE_OPCODE_START) ||
(Cdb->StatCode != PXE_STATCODE_INITIALIZE) ||
(Cdb->StatFlags != PXE_STATFLAGS_INITIALIZE) ||
(Cdb->IFnum >= (gPxe->IFcnt | gPxe->IFcntExt << 8)) ||
(Cdb->CPBsize != sizeof (PXE_CPB_START_31)) ||
(Cdb->DBsize != PXE_DBSIZE_NOT_USED) ||
(Cdb->DBaddr != PXE_DBADDR_NOT_USED) ||
(Cdb->OpFlags != PXE_OPFLAGS_NOT_USED))
{
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_INVALID_CDB;
return;
} else {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_COMPLETE;
Cdb->StatCode = PXE_STATCODE_SUCCESS;
}
if (Nic->State != PXE_STATFLAGS_GET_STATE_STOPPED) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_ALREADY_STARTED;
return;
}
Cpb = (PXE_CPB_START_31 *)(UINTN)Cdb->CPBaddr;
Nic->PxeStart.Delay = Cpb->Delay;
Nic->PxeStart.Virt2Phys = Cpb->Virt2Phys;
Nic->PxeStart.Block = Cpb->Block;
Nic->PxeStart.Map_Mem = 0;
Nic->PxeStart.UnMap_Mem = 0;
Nic->PxeStart.Sync_Mem = Cpb->Sync_Mem;
Nic->PxeStart.Unique_ID = Cpb->Unique_ID;
Nic->State = PXE_STATFLAGS_GET_STATE_STARTED;
if (Nic->UsbEth->UsbEthUndi.UsbEthUndiStart != NULL) {
Status = Nic->UsbEth->UsbEthUndi.UsbEthUndiStart (Cdb, Nic); <---
RndisUndiStart
if (EFI_ERROR (Status)) {
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
}
}
}
Thanks,
Richard
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
Abner via groups.io
Sent: 2022年9月12日 3:56 PM
To: RichardHo [何明忠] <RichardHo@...>; devel@edk2.groups.io;
quic_rcran@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney <michael.d.kinney@...>;
Michael Kubacki <michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>; TonyLo
[羅金松] <TonyLo@...>
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, thanks for the reply. My feedback in line below,
-----Original Message-----
From: RichardHo [何明忠] <RichardHo@...>
Sent: Monday, September 12, 2022 11:48 AM
To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io;
quic_rcran@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH] UsbNetworkPkg: add
USB network devices support
[CAUTION: External Email]
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. UsbNetworkPkg has the feature that overlaps with Bus\Usb and \NetworkPkg in
my opinion, that is the reason I think we can have these modules in the existing
package. Plus as I know from the edk2 community view point, we only create a
new package when necessary for a totally new feature. However, this is at the
discretion of edk2 stewards. You can confirm this with them.
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. My question was USB_ETHERNET_PROTOCOL is designed as an abstract
protocol, with this the underlying transport (UsbIo or PciIo) is agnostic to the
upper layer driver. Upper layer driver (e.g., USB_ETHERNET_PROTOCOL based
SNP driver) can check whether USB_ETHERNET_PROTOCOL is installed on the
given Controller handle at Supported() function. However, seems the upper layer
driver has no way to distinguish the USB CDC driver interface class if I read your
code right; then manages the right device (in the case of multiple USB CDC
devices are attached to the system). We can either attach USB Class Device Path
to the device path in Usb CDC driver or provide a function to retrieve the USB
CDC interface class/subclass/protocol via USB_ETHERNET_PROTOCOL. I would
prefer the former one.
Thanks
Abner
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@...>
Sent: 2022年9月9日 3:07 PM
To: devel@edk2.groups.io; quic_rcran@...; RichardHo [何明忠]
<RichardHo@...>
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
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@...
Cc: Andrew Fish <afish@...>; Leif Lindholm
<quic_llindhol@...>; Michael D Kinney
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Zhiguang Liu
<zhiguang.liu@...>; Liming Gao <gaoliming@...>;
TonyLo [羅金松]
<TonyLo@...>
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%2Fnam1
2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fnam
1
&data=05%7C01%7Cabner.chang%40amd.com%7Cf79f1cf3f8354d727467
08da9b
79d240%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799324477
7924364
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
TiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=21Xp09W656c14
EGfJms8Ov
TvUZ4xihlIep3ES9kAC0U%3D&reserved=0
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fnam
&
amp;data=05%7C01%7Cigork%40ami.com%7C2dbbb37cd5aa464372d408da949
44389%
7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C637985661785139437%
7CUnkn
own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
Wwi
LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6kJ7djzYAbWay75nEMfH2
eATKEWC2
j%2FoFYakFE2PuJw%3D&reserved=0
1
2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fn
am1
&data=05%7C01%7CAbner.Chang%40amd.com%7C92e0dc34ec2a4c9a9f
6908da94
71a29e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63798551306
1024727
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
CJBTiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=78%2FyX9SBP
4wrftOYrmnP
muF5e%2F3NsmqY7JVw4atcoGM%3D&reserved=0
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fgi
th
&data=05%7C01%7Crichardho%40ami.com%7C1ffd2a8dbf6f4cda9da908
da9231
e426%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C63798304023249
3161%7
CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
BTiI6Ik1
haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XK3dw8vGMtln2
Ig6Rcieyw6l
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@...>
Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <michael.kubacki@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Liming Gao <gaoliming@...>
Reviewed-by: Tony Lo <tonylo@...>
---
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.
-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.
|
|
|
Rebecca Cran <quic_rcran@...>
On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote: diff --git a/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc b/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
new file mode 100644
index 0000000000..74b5a3c1b3
--- /dev/null
+++ b/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
@@ -0,0 +1,20 @@
+## @file
+# List of Core Components.
+#
+# Copyright (c) 2022, American Megatrends International LLC. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+ UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbCdcEcmSupport
+ UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf
+!endif
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbCdcNcmSupport
+ UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf
+!endif
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbRndisSupport
+ UsbNetworkPkg/UsbRndis/UsbRndis.inf
+!endif Since some of the functions have the same name and aren't static, should there be code to avoid allowing more than one protocol to be enabled at the same time? diff --git a/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h b/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h
new file mode 100644
index 0000000000..1c1a450920
--- /dev/null
+++ b/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h +// Request Type Codes for USB Ethernet
+#define USB_ETHERNET_GET_REQ_TYPE 0xA1
+#define USB_ETHRTNET_SET_REQ_TYPE 0x21 Typo. +// The USB_ETHERNET_PROTOCOL provides some basic USB Ethernet device relevant
+// descriptor and specific requests.
+struct _USB_ETHERNET_PROTOCOL {
+ USB_ETHERNET_UNDI UsbEthUndi;
+ // for calling the UNDI child functions
+ USB_ETHERNET_INITIALIZE UsbEthInitialize;
+ USB_ETHERNET_STATISTICS UsbEthStatistics;
+ USB_ETHERNET_RECEIVE UsbEthReceive;
+ USB_ETHERNET_TRANSMIT UsbEthTransmit;
+ USB_ETHERNET_INTERRUPT UsbEthInterrupt;
+ USB_GET_ETH_MAC_ADDRESS UsbEthMacAddress;
+ USB_ETH_MAX_BULK_SIZE UsbEthMaxBulkSize;
+ USB_HEADER_FUNCTIONAL_DESCRIPTOR UsbHeaderFunDescriptor;
+ USB_UNION_FUNCTIONAL_DESCRIPTOR UsbUnionFunDescriptor;
+ USB_ETHERNET_FUNCTIONAL_DESCRIPTOR UsbEthFunDescriptor;
+ USB_ETHERNET_SET_ETH_MULTICAST_FILTERS SetUsbEthMcastFilter;
+ USB_ETHERNET_SET_ETH_POWER_MANAGE_PATTERN_FILTER SetUsbEthPowerPatternFilter;
+ USB_ETHERNET_GET_ETH_POWER_MANAGE_PATTERN_FILTER GetUsbEthPoewrPatternFilter; Typo. diff --git a/UsbNetworkPkg/NetworkCommon/PxeFunction.c b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
new file mode 100644
index 0000000000..f6505f7018
--- /dev/null
+++ b/UsbNetworkPkg/NetworkCommon/PxeFunction.c +/**
+ Set PXE receive filter.
+
+ @param[in] Nic A pointer to the Network interface controller data.
+ @param[in] SetFilter PXE receive filter
+ @param[in] CpbAddr Command Parameter Block Address
+ @param[in] CpbSize Command Parameter Block Size
+
+**/
+UINT16
+Setfilter (
+ IN NIC_DATA *Nic,
+ IN UINT16 SetFilter,
+ IN UINT64 CpbAddr,
+ IN UINT32 CpbSize
+ ) Following naming conventions, this should probably be "SetFilter", with an uppercase 'F'? +**/
+UINT16
+Receive (
+ IN PXE_CDB *Cdb,
+ IN OUT NIC_DATA *Nic,
+ IN UINT64 CpbAddr,
+ IN OUT UINT64 DbAddr
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ UINT16 StatCode = PXE_STATCODE_NO_DATA;
+ PXE_FRAME_TYPE FrameType = PXE_FRAME_TYPE_NONE;
+ PXE_CPB_RECEIVE *Cpb;
+ PXE_DB_RECEIVE *Db;
+ UINT8 *BulkInData;
+ UINTN DataLength = (UINTN)Nic->MaxSegmentSize;
+ EthernetHeader *Header;
+
+ Cpb = (PXE_CPB_RECEIVE *)(UINTN)CpbAddr;
+ Db = (PXE_DB_RECEIVE *)(UINTN)DbAddr;
+
+ Status = gBS->AllocatePool (EfiBootServicesData, DataLength, (VOID **)&BulkInData); Allocating and freeing memory on every poll (i.e. 10ms) doesn't seem like a good idea. Could we allocate it during initialize instead? -- Rebecca Cran
|
|
|
Hi Rebecca,
Thanks for your response.
#1. The NCM, ECM, and RNDIS feature could enable at the same time.
#2. UsbEthernetProtocol.h, PxeFunction.c the TYPO will be fixed next.
#3 Receive buffer will be allocate during driver initialize.
Thanks,
Richard
toggle quoted message
Show quoted text
-----Original Message----- From: Rebecca Cran <rebecca@...> Sent: 2022年11月27日 2:56 AM To: devel@edk2.groups.io; Richard Ho (何明忠) <RichardHo@...> Cc: Andrew Fish <afish@...>; Leif Lindholm <quic_llindhol@...>; Michael D Kinney <michael.d.kinney@...>; Michael Kubacki <michael.kubacki@...>; Zhiguang Liu <zhiguang.liu@...>; Liming Gao <gaoliming@...>; Tony Lo (羅金松) <TonyLo@...> 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.** On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote: diff --git a/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
b/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
new file mode 100644
index 0000000000..74b5a3c1b3
--- /dev/null
+++ b/UsbNetworkPkg/Config/UsbNetworkPkgComponentsDxe.inc.dsc
@@ -0,0 +1,20 @@
+## @file
+# List of Core Components.
+#
+# Copyright (c) 2022, American Megatrends International LLC. All
+rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+ UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbCdcEcmSupport
+ UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf
+!endif
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbCdcNcmSupport
+ UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf
+!endif
+
+!if gUsbNetworkPkgTokenSpaceGuid.UsbRndisSupport
+ UsbNetworkPkg/UsbRndis/UsbRndis.inf
+!endif Since some of the functions have the same name and aren't static, should there be code to avoid allowing more than one protocol to be enabled at the same time? diff --git a/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h
b/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h
new file mode 100644
index 0000000000..1c1a450920
--- /dev/null
+++ b/UsbNetworkPkg/Include/Protocol/UsbEthernetProtocol.h +// Request Type Codes for USB Ethernet #define
+USB_ETHERNET_GET_REQ_TYPE 0xA1 #define USB_ETHRTNET_SET_REQ_TYPE
+0x21 Typo. +// The USB_ETHERNET_PROTOCOL provides some basic USB Ethernet device
+relevant // descriptor and specific requests.
+struct _USB_ETHERNET_PROTOCOL {
+ USB_ETHERNET_UNDI UsbEthUndi;
+ // for calling the UNDI child functions
+ USB_ETHERNET_INITIALIZE UsbEthInitialize;
+ USB_ETHERNET_STATISTICS UsbEthStatistics;
+ USB_ETHERNET_RECEIVE UsbEthReceive;
+ USB_ETHERNET_TRANSMIT UsbEthTransmit;
+ USB_ETHERNET_INTERRUPT UsbEthInterrupt;
+ USB_GET_ETH_MAC_ADDRESS UsbEthMacAddress;
+ USB_ETH_MAX_BULK_SIZE UsbEthMaxBulkSize;
+ USB_HEADER_FUNCTIONAL_DESCRIPTOR UsbHeaderFunDescriptor;
+ USB_UNION_FUNCTIONAL_DESCRIPTOR UsbUnionFunDescriptor;
+ USB_ETHERNET_FUNCTIONAL_DESCRIPTOR UsbEthFunDescriptor;
+ USB_ETHERNET_SET_ETH_MULTICAST_FILTERS SetUsbEthMcastFilter;
+ USB_ETHERNET_SET_ETH_POWER_MANAGE_PATTERN_FILTER SetUsbEthPowerPatternFilter;
+ USB_ETHERNET_GET_ETH_POWER_MANAGE_PATTERN_FILTER GetUsbEthPoewrPatternFilter; Typo. diff --git a/UsbNetworkPkg/NetworkCommon/PxeFunction.c
b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
new file mode 100644
index 0000000000..f6505f7018
--- /dev/null
+++ b/UsbNetworkPkg/NetworkCommon/PxeFunction.c +/**
+ Set PXE receive filter.
+
+ @param[in] Nic A pointer to the Network interface controller data.
+ @param[in] SetFilter PXE receive filter
+ @param[in] CpbAddr Command Parameter Block Address
+ @param[in] CpbSize Command Parameter Block Size
+
+**/
+UINT16
+Setfilter (
+ IN NIC_DATA *Nic,
+ IN UINT16 SetFilter,
+ IN UINT64 CpbAddr,
+ IN UINT32 CpbSize
+ ) Following naming conventions, this should probably be "SetFilter", with an uppercase 'F'? +**/
+UINT16
+Receive (
+ IN PXE_CDB *Cdb,
+ IN OUT NIC_DATA *Nic,
+ IN UINT64 CpbAddr,
+ IN OUT UINT64 DbAddr
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ UINT16 StatCode = PXE_STATCODE_NO_DATA;
+ PXE_FRAME_TYPE FrameType = PXE_FRAME_TYPE_NONE;
+ PXE_CPB_RECEIVE *Cpb;
+ PXE_DB_RECEIVE *Db;
+ UINT8 *BulkInData;
+ UINTN DataLength = (UINTN)Nic->MaxSegmentSize;
+ EthernetHeader *Header;
+
+ Cpb = (PXE_CPB_RECEIVE *)(UINTN)CpbAddr; Db = (PXE_DB_RECEIVE
+ *)(UINTN)DbAddr;
+
+ Status = gBS->AllocatePool (EfiBootServicesData, DataLength, (VOID
+ **)&BulkInData); Allocating and freeing memory on every poll (i.e. 10ms) doesn't seem like a good idea. Could we allocate it during initialize instead? -- Rebecca Cran -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.
|
|
|
Rebecca Cran <quic_rcran@...>
On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote: + DataLength = (UINTN)(Cpb->DataLen + (UINT32)Cpb->MediaheaderLen);
+
+ while (1) {
+ if (Counter >= 3) {
+ StatCode = PXE_STATCODE_BUSY;
+ break;
+ }
+
+ Status = Nic->UsbEth->UsbEthTransmit (Cdb, Nic->UsbEth, (VOID *)(UINTN)BulkOutData, &DataLength);
+ if (EFI_ERROR (Status)) {
+ StatCode = PXE_STATFLAGS_COMMAND_FAILED;
+ }
+
+ if (Status == EFI_INVALID_PARAMETER) {
+ StatCode = PXE_STATCODE_INVALID_PARAMETER;
+ break;
+ }
+
+ if (Status == EFI_DEVICE_ERROR) {
+ StatCode = PXE_STATCODE_DEVICE_FAILURE;
+ break;
+ }
+
+ if (!EFI_ERROR (Status)) {
+ Nic->TxFrame++;
+ StatCode = PXE_STATCODE_SUCCESS;
+ break;
+ }
+
+ Counter++;
+ } You need to set DataLength inside the while loop, otherwise on subsequent iterations DataLength will be whatever value UsbEthTransmit just set it to, which will likely be 0 since an error occurred. -- Rebecca Cran
|
|
|
Hi Rebecca,
Thanks for your response. I will fixed it next.
Thanks,
Richard
toggle quoted message
Show quoted text
-----Original Message----- From: Rebecca Cran <rebecca@...> Sent: 2022年12月5日 11:15 AM To: devel@edk2.groups.io; Richard Ho (何明忠) <RichardHo@...> Cc: Andrew Fish <afish@...>; Leif Lindholm <quic_llindhol@...>; Michael D Kinney <michael.d.kinney@...>; Michael Kubacki <michael.kubacki@...>; Zhiguang Liu <zhiguang.liu@...>; Liming Gao <gaoliming@...>; Tony Lo (羅金松) <TonyLo@...> 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.** On 9/1/22 23:24, RichardHo [何明忠] via groups.io wrote: + DataLength = (UINTN)(Cpb->DataLen + (UINT32)Cpb->MediaheaderLen);
+
+ while (1) {
+ if (Counter >= 3) {
+ StatCode = PXE_STATCODE_BUSY;
+ break;
+ }
+
+ Status = Nic->UsbEth->UsbEthTransmit (Cdb, Nic->UsbEth, (VOID *)(UINTN)BulkOutData, &DataLength);
+ if (EFI_ERROR (Status)) {
+ StatCode = PXE_STATFLAGS_COMMAND_FAILED;
+ }
+
+ if (Status == EFI_INVALID_PARAMETER) {
+ StatCode = PXE_STATCODE_INVALID_PARAMETER;
+ break;
+ }
+
+ if (Status == EFI_DEVICE_ERROR) {
+ StatCode = PXE_STATCODE_DEVICE_FAILURE;
+ break;
+ }
+
+ if (!EFI_ERROR (Status)) {
+ Nic->TxFrame++;
+ StatCode = PXE_STATCODE_SUCCESS;
+ break;
+ }
+
+ Counter++;
+ } You need to set DataLength inside the while loop, otherwise on subsequent iterations DataLength will be whatever value UsbEthTransmit just set it to, which will likely be 0 since an error occurred. -- Rebecca Cran -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.
|
|
|
