I totally agree with you that putting function which would change h/w status 
into ASSERT is not good practice.

For the case, it's only used to read a register once. It's just a double 
confirmation and real judgment is done by XhcResetHC().

Thanks
Feng
From: Sergey Isakov [mailto:isakov...@bk.ru]
Sent: Wednesday, September 18, 2013 13:52
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] XHCI Issues

Hi Feng,

I just searching for possible mistakes so I will agree with you. One thought, 
what if the bit will clear after a timeout?
Anyway I think it is not good practice to place hardware dependent codes into 
ASSERT that works differently in Debug or Release build.
I will test your final codes as soon as you commit them.
Best regards,
Sergey

On 18.09.2013, at 7:34, Tian, Feng wrote:


Hi, Sergey

Please be aware of there are two types of reset. One is HCRST by software, the 
other is Chip Hardware Reset. A Chip Hardware Reset means either a PCI reset 
input or an optional power-on reset input to the xHC.

CNR bit of USB STATUS register is only related with Chip Hardware Reset, and it 
should always be 0 after power-on. It's why we only add it to ASSERT.

Thanks
Feng

From: Sergey Isakov [mailto:isakov...@bk.ru]
Sent: Tuesday, September 17, 2013 18:45
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] XHCI Issues

OK,
Please consider one more possible mistake
Xhci.c, line 180.
ASSERT (!(XHC_REG_BIT_IS_SET (Xhc, XHC_USBSTS_OFFSET, XHC_USBSTS_CNR)));
In release build this macro will not be executed. But on real hardware reading 
Status will clear it.
Sergey.

On 17.09.2013, at 13:15, Tian, Feng wrote:



Hi, Sergey

The Assert() in XhcReadOpReg() makes sense as it would never be false if the 
XHCI controller follows XHCI spec.


For your 2nd concern, yes, we didn't care the return data for such cases.

Thanks
Feng

From: Sergey Isakov [mailto:isakov...@bk.ru]
Sent: Tuesday, September 17, 2013 16:13
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] XHCI Issues

One more issue here. Look into XhcReadRuntimeReg ()
------
  if (EFI_ERROR (Status)) {
    DEBUG ((EFI_D_ERROR, "XhcReadRuntimeReg: Pci Io Read error - %r at %d\n", 
Status, Offset));
    Data = 0xFFFFFFFF;
  }

  return Data;
}
------
But usage of the procedure do not propose wrong return. For example
---------
  Data  = XhcReadRuntimeReg (Xhc, Offset);
  Data |= Bit;
  XhcWriteRuntimeReg (Xhc, Offset, Data);
-----------

Yours,
Sergey

On 17.09.2013, at 11:20, Sergey Isakov wrote:




Hi Feng,
I am also thinking about different ASSERTs in the driver.
If debug then ASSERT cause a hang/stop/break.
If release then ASSERT do nothing causes next operator to crash (null pointer 
etc).
For example
------------------
UINT32
XhcReadOpReg (
  IN  USB_XHCI_INSTANCE   *Xhc,
  IN  UINT32              Offset
  )
{
  UINT32                  Data;
  EFI_STATUS              Status;

  ASSERT (Xhc->CapLength != 0);
------------------
Is it true for all controllers? May be we can make
-----------------
  if (!Xhc->CapLength) {
    return 0xFFFFFFFF;
  }
----------------- ?
The same for door bell and others.

I am not sure there is a trouble but I don't like ASSERT.

Sergey.


On 17.09.2013, at 7:23, Sergey Isakov wrote:




Hi Feng,
Yes, it is good. I was also thinking that 0 may be right value. (not in my 
case).
Sergey.


On 17.09.2013, at 7:00, "Tian, Feng" 
<feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote:




Hi, Sergey

Please help review the patch.

The patch has a little different with yours as value 0 may be the correct 
return value for some XHCI cards which supports USB Legacy Support Capability.

Thanks
Feng

From: Tian, Feng
Sent: Monday, September 16, 2013 15:57
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Cc: Tian, Feng
Subject: RE: [edk2] XHCI Issues

Yes, you are right. Usb Legacy Support Capability is optional, we should ignore 
it if the XHCI controller doesn't support this feature.

I will follow up it as soon as possible.

From: Sergey Isakov [mailto:isakov...@bk.ru]
Sent: Monday, September 16, 2013 15:33
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] XHCI Issues

Dear sirs,
I found new issue with XhciDxe driver.
I have USB3 PCIe adapter with chip VIA VL800-Q8. It works but the code
--------
Xhc->UsbLegSupOffset = XhcGetLegSupCapAddr (Xhc);
--------
returns Zero.
And then in procedure
------------------
VOID
XhcSetBiosOwnership (
  IN USB_XHCI_INSTANCE    *Xhc
  )
{
  UINT32                    Buffer;

  DEBUG ((EFI_D_INFO, "XhcSetBiosOwnership: called to set BIOS ownership\n"));

  Buffer = XhcReadExtCapReg (Xhc, Xhc->UsbLegSupOffset);
  Buffer = ((Buffer & (~USBLEGSP_OS_SEMAPHORE)) | USBLEGSP_BIOS_SEMAPHORE);
  XhcWriteExtCapReg (Xhc, Xhc->UsbLegSupOffset, Buffer);
}
------------------
I have a hang.
It will be good to add check for zero
------------------
VOID
XhcSetBiosOwnership (
  IN USB_XHCI_INSTANCE    *Xhc
  )
{
  UINT32                    Buffer;
  if (!Xhc->UsbLegSupOffset) {
    DEBUG ((EFI_D_INFO, "XhcSetBiosOwnership: not set\n"));
    return;
  }
------------------
The same for ClearBiosOwnership

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sergey Isakov <isakov...@bk.ru<mailto:isakov...@bk.ru>>




On 12.08.2013, at 12:55, Li, Elvin wrote:

Eugene,
                The XHCI DMA update has been checked in to edk2 14546. If you 
find any problems, please contact me.

Thanks
Elvin
From: Cohen, Eugene [mailto:eug...@hp.com]
Sent: Thursday, July 25, 2013 4:29 AM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: [edk2] XHCI DMA Buffer Issues

Dear XhciDxe Maintainer,

I'm currently reviewing the XhciDxe driver and I'm trying to figure out how DMA 
buffers are allocated.  I see a number of pool and page allocations but I do 
not see any called to PCI_IO Map()/Unmap() or to AllocateBuffer()/FreeBuffer().

This appears to be violating the rules for PCI DMA buffers since they are not 
being mapped (and if common buffers are desired then they are also not being 
allocated with AllocateBuffer as required).

Can someone more familiar with XHCI help me determine which buffers need to be 
mapped and how (BusMasterRead, BusMasterWrite, CommonBuffer)?

It would be useful if we had a test environment that could catch driver DMA 
buffer mapping issues earlier.  I think one way to do this would be set up the 
MMU in a non-identity mode so that anyone trying to use a processor virtual 
address for DMA would see a failure.

Thanks,

Eugene

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead.
Download for free and get started troubleshooting in minutes.
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

<XhciReg.c.patch>------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13. 
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to