From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=s/e4oTV6; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.222.179, mailfrom: mw@semihalf.com) Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) by groups.io with SMTP; Fri, 02 Aug 2019 00:11:51 -0700 Received: by mail-qk1-f179.google.com with SMTP id r4so53955932qkm.13 for ; Fri, 02 Aug 2019 00:11:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=G4I9TkEoUK56s660jgJgMjpcMdbEyg6GWg6DwMSBUGI=; b=s/e4oTV6TRzoTG5B1lq2/lOLhac1LbqVALddm/KKmlAJ1dkdpa3GlZ2l0liaIDyKu8 2YpQqQ5plIRXBKei4qKmqh5cUciL4LR4JPEIKIVN/UekYi+uxiNfCMHijAjS1kzUYHT8 vknYaTlKc5m0QCQDNaltDzSanbU9E1V7mNJWbzzlFV0I7gbtxhB5AIy8LjzKxe3l8Lpd DZkWGQoGvCc28joCfXM4cAoHRqQ9nyko+JJIqcXwk9RoPu9dBe1Q4jUuWqLl2yV/oNdz EsUfuDnp7F6pEwjupuArLy9uQFcQYkYHcU/Gt66WcTmeDKJrDSQQDgUHc6iPye5/PL5O 9fuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=G4I9TkEoUK56s660jgJgMjpcMdbEyg6GWg6DwMSBUGI=; b=Niq5VXynyFazLuxeHNUV5AO4Msl4r1yNfmVKbJe53udKpqXMBg90+1W/u4Tx5SvUVY S4Hn1arVSy9iIcxJrBrQn09THVbypk1gtadU7jLEfVeuUGKMnjoPrEfyuanjJ3c+ViYT 0QLObj2/ESCRmsJ8qT+ctANxwY5a/9yav8xSJlDOBtvhAKaiP5VoRIsiq+C3SQvwDV1x ZxytOwianvqJZmT9XyBkSfROz+X0LRCzDP2lKBJ21JohXk0hn3MKDoD1tP7UMbWoQKpp w2fIS8vTYPAeoOQr7qCwvn1rxR4SyVC6bogWlmfPLI9WFeBd2SAdUXClIEIfA5XRFyUP Du/Q== X-Gm-Message-State: APjAAAUpY+JyY01l2lLWy/9BqKvQjQMfK2SgNZrVuSHO9/EluAqql74V oh4ox3TJMa0FDAo+gMeSfNtHCC4kIOJlwhwxdaU0b0rH X-Google-Smtp-Source: APXvYqwmvx989V/FuRW9DJrqJK6Pfi6yMfAd6kGI4FX2zRMupJgblcx1LPLavAC3Dp5Ar4NowewN9lEjbps0kXFqHes= X-Received: by 2002:a37:454:: with SMTP id 81mr87037946qke.153.1564729910530; Fri, 02 Aug 2019 00:11:50 -0700 (PDT) MIME-Version: 1.0 References: <1564554319-26810-1-git-send-email-mw@semihalf.com> <734D49CCEBEEF84792F5B80ED585239D5C265FC4@SHSMSX104.ccr.corp.intel.com> In-Reply-To: From: "Marcin Wojtas" Date: Fri, 2 Aug 2019 09:11:39 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present To: edk2-devel-groups-io , "Wu, Hao A" Cc: "Ni, Ray" , "leif.lindholm@linaro.org" , "ard.biesheuvel@linaro.org" , "jsd@semihalf.com" , "jaz@semihalf.com" , "Kinney, Michael D" , "Gao, Liming" , "lersek@redhat.com" Content-Type: multipart/alternative; boundary="0000000000007cac3d058f1d123a" --0000000000007cac3d058f1d123a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Hao, pt., 2 sie 2019 o 03:04 Wu, Hao A napisa=C5=82(a): > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Ni, > > Ray > > Sent: Thursday, August 01, 2019 12:51 AM > > To: devel@edk2.groups.io; mw@semihalf.com > > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; > jsd@semihalf.com; > > jaz@semihalf.com; Tian, Feng; Kinney, Michael D; Gao, Liming; > > lersek@redhat.com > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > > when the device is not present > > > > Marcin, > > What does the failure look like without your fix? > > > From my observation of the code: > > A. Changes are made in UsbEnumerateNewDev(); > B. Caller of UsbEnumerateNewDev(): UsbEnumeratePort(); > C. UsbEnumeratePort() directly returns the status from UsbEnumerateNewDe= v() > as its own return value; > D. Callers of UsbEnumeratePort(): UsbHubEnumeration() and > UsbRootHubEnumeration(); > E. Both of them do not care about the return status from > UsbEnumeratePort(). > > I think there is no direct failure without the proposed fix. > However, let us wait the confirmation from Marcin. > > Indeed, the code path guarantees that nothing is done with the UsbEnumeratePort() Status. The patch is not a fix for the upstream code, however current behavior was problematic for a solution I was implementing on top of it (i.e. disabling USB hot-plug feature for a certain platform). Best regards, Marcin > Also, I think the proposed patch is a good refinement. If future updates= to > the driver requires checking the return status during the above code pat= h, > then proper status will be returned. > > Best Regards, > Hao Wu > > > > > > Thanks, > > Ray > > > > > -----Original Message----- > > > From: devel@edk2.groups.io On Behalf Of > > Marcin Wojtas > > > Sent: Wednesday, July 31, 2019 2:25 PM > > > To: devel@edk2.groups.io > > > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; > > mw@semihalf.com; jsd@semihalf.com; jaz@semihalf.com; Tian, Feng > > > ; Kinney, Michael D ; > > Gao, Liming ; > > > lersek@redhat.com > > > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > > when the device is not present > > > > > > Until now, during the USB device enumeration when its PortState > > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not > > informed > > > that the device is not present. Fix that by returning appropriate > > > error code. > > > > > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > > > Signed-off-by: Marcin Wojtas > > > --- > > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > index be9d9bd..ab1db15 100644 > > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > > > > > if (!USB_BIT_IS_SET (PortState.PortStatus, > > USB_PORT_STAT_CONNECTION)) { > > > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at > > port %d\n", Port)); > > > + Status =3D EFI_NOT_FOUND; > > > goto ON_ERROR; > > > } else if (USB_BIT_IS_SET (PortState.PortStatus, > > USB_PORT_STAT_SUPER_SPEED)){ > > > Child->Speed =3D EFI_USB_SPEED_SUPER; > > > -- > > > 2.7.4 > > > > > > > > > > > > > > > > > >=20 > > --0000000000007cac3d058f1d123a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Hao,

pt., 2 sie 2019 o 03:04=C2=A0Wu, H= ao A <hao.a.wu@intel.com> n= apisa=C5=82(a):
= > -----Original Message-----
> From: devel= @edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni,
> Ray
> Sent: Thursday, August 01, 2019 12:51 AM
> To: devel@e= dk2.groups.io; mw@= semihalf.com
> Cc: lei= f.lindholm@linaro.org; ard.biesheuvel@linaro.org; jsd@semihalf.com;
> jaz@semihalf.co= m; Tian, Feng; Kinney, Michael D; Gao, Liming;
> lersek@redhat.= com
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return erro= r
> when the device is not present
>
> Marcin,
> What does the failure look like without your fix?


>>From my observation of the code:

A. Changes are made in UsbEnumerateNewDev();
B. Caller of UsbEnumerateNewDev(): UsbEnumeratePort();
C. UsbEnumeratePort() directly returns the status from UsbEnumerateNewDev(= )
=C2=A0 =C2=A0as its own return value;
D. Callers of UsbEnumeratePort(): UsbHubEnumeration() and UsbRootHubEnumer= ation();
E. Both of them do not care about the return status from UsbEnumeratePort(= ).

I think there is no direct failure without the proposed fix.
However, let us wait the confirmation from Marcin.


Indeed, the code path guarantees that= nothing is done with the UsbEnumeratePort() Status. The patch is not a fix= for the upstream code, however current behavior was problematic for a solu= tion I was implementing on top of it (i.e. disabling USB hot-plug feature f= or a certain platform).

Best regards,
Ma= rcin
=C2=A0
= devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Marcin Wojtas
> > Sent: Wednesday, July 31, 2019 2:25 PM
> > To: de= vel@edk2.groups.io
> > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org;
> mw@semihalf.com<= /a>; jsd@semihalf.com= ; jaz@semihalf.co= m; Tian, Feng
> > <fen= g.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; > Gao, Liming <liming.gao@intel.com>;
> > lersek@re= dhat.com
> > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return err= or
> when the device is not present
> >
> > Until now, during the USB device enumeration when its PortState<= br> > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not
> informed
> > that the device is not present. Fix that by returning appropriat= e
> > error code.
> >
> > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> >=C2=A0 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 +
> >=C2=A0 1 file changed, 1 insertion(+)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> > index be9d9bd..ab1db15 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> > @@ -719,6 +719,7 @@ UsbEnumerateNewDev (
> >
> >=C2=A0 =C2=A0 if (!USB_BIT_IS_SET (PortState.PortStatus,
> USB_PORT_STAT_CONNECTION)) {
> >=C2=A0 =C2=A0 =C2=A0 DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDe= v: No device present at
> port %d\n", Port));
> > +=C2=A0 =C2=A0 Status =3D EFI_NOT_FOUND;
> >=C2=A0 =C2=A0 =C2=A0 goto ON_ERROR;
> >=C2=A0 =C2=A0 } else if (USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_SUPER_SPEED)){
> >=C2=A0 =C2=A0 =C2=A0 Child->Speed=C2=A0 =C2=A0 =C2=A0 =3D EFI_= USB_SPEED_SUPER;
> > --
> > 2.7.4
> >
> >
> >
>
>
>




--0000000000007cac3d058f1d123a--