| Message ID | 163178241612.65790.3191991741031171590.stgit@localhost |
|---|---|
| State | Accepted |
| Commit | 9845b924369cc71457a21e78b5f9e6f7af43532d |
| Headers | show |
| Series | efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest | expand |
On 9/16/21 10:53 AM, Masami Hiramatsu wrote: > Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet > receiving loop. This depends on the implementation and not > related to whether the packet can be received or not. Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a package needs to be processed? Best regards Heinrich > > Whether the received packets are available or not is ensured > by wait_for_packet, and that is already done in the loop. > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > --- > lib/efi_selftest/efi_selftest_snp.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c > index cb0db7eea2..c5366c872c 100644 > --- a/lib/efi_selftest/efi_selftest_snp.c > +++ b/lib/efi_selftest/efi_selftest_snp.c > @@ -340,8 +340,6 @@ static int execute(void) > events[0] = timer; > events[1] = net->wait_for_packet; > for (;;) { > - u32 int_status; > - > /* > * Wait for packet to be received or timer event. > */ > @@ -367,15 +365,6 @@ static int execute(void) > * Receive packet > */ > buffer_size = sizeof(buffer); > - ret = net->get_status(net, &int_status, NULL); > - if (ret != EFI_SUCCESS) { > - efi_st_error("Failed to get status"); > - return EFI_ST_FAILURE; > - } > - if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) { > - efi_st_error("RX interrupt not set"); > - return EFI_ST_FAILURE; > - } > ret = net->receive(net, NULL, &buffer_size, &buffer, > &srcaddr, &destaddr, NULL); > if (ret != EFI_SUCCESS) { >
Hi Heinrich, 2021年9月16日(木) 18:30 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > > > On 9/16/21 10:53 AM, Masami Hiramatsu wrote: > > Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet > > receiving loop. This depends on the implementation and not > > related to whether the packet can be received or not. > > Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a > package needs to be processed? No, that is not promised as far as I can read the specification. It is just a bit flag of the device (Spec says "the interrupt status will be read from the device."). Not ensure the received packet buffer status. However, the WaitForPacket ensures the packet is received. "Event used with EFI_BOOT_SERVICES.WaitForEvent() to wait for a packet to be received." So I think it is enough to check the WaitForPacket. Thank you, > > Best regards > > Heinrich > > > > > Whether the received packets are available or not is ensured > > by wait_for_packet, and that is already done in the loop. > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > --- > > lib/efi_selftest/efi_selftest_snp.c | 11 ----------- > > 1 file changed, 11 deletions(-) > > > > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c > > index cb0db7eea2..c5366c872c 100644 > > --- a/lib/efi_selftest/efi_selftest_snp.c > > +++ b/lib/efi_selftest/efi_selftest_snp.c > > @@ -340,8 +340,6 @@ static int execute(void) > > events[0] = timer; > > events[1] = net->wait_for_packet; > > for (;;) { > > - u32 int_status; > > - > > /* > > * Wait for packet to be received or timer event. > > */ > > @@ -367,15 +365,6 @@ static int execute(void) > > * Receive packet > > */ > > buffer_size = sizeof(buffer); > > - ret = net->get_status(net, &int_status, NULL); > > - if (ret != EFI_SUCCESS) { > > - efi_st_error("Failed to get status"); > > - return EFI_ST_FAILURE; > > - } > > - if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) { > > - efi_st_error("RX interrupt not set"); > > - return EFI_ST_FAILURE; > > - } > > ret = net->receive(net, NULL, &buffer_size, &buffer, > > &srcaddr, &destaddr, NULL); > > if (ret != EFI_SUCCESS) { > > -- Masami Hiramatsu
diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c index cb0db7eea2..c5366c872c 100644 --- a/lib/efi_selftest/efi_selftest_snp.c +++ b/lib/efi_selftest/efi_selftest_snp.c @@ -340,8 +340,6 @@ static int execute(void) events[0] = timer; events[1] = net->wait_for_packet; for (;;) { - u32 int_status; - /* * Wait for packet to be received or timer event. */ @@ -367,15 +365,6 @@ static int execute(void) * Receive packet */ buffer_size = sizeof(buffer); - ret = net->get_status(net, &int_status, NULL); - if (ret != EFI_SUCCESS) { - efi_st_error("Failed to get status"); - return EFI_ST_FAILURE; - } - if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) { - efi_st_error("RX interrupt not set"); - return EFI_ST_FAILURE; - } ret = net->receive(net, NULL, &buffer_size, &buffer, &srcaddr, &destaddr, NULL); if (ret != EFI_SUCCESS) {
Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet receiving loop. This depends on the implementation and not related to whether the packet can be received or not. Whether the received packets are available or not is ensured by wait_for_packet, and that is already done in the loop. Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- lib/efi_selftest/efi_selftest_snp.c | 11 ----------- 1 file changed, 11 deletions(-)