> From: Martin Pieuchot
> Sent: Thursday, December 7, 2017 3:18 AM
>
> Which issue are you having?
Sorry, there was more context in an earlier thread. Basically, I have a pc
engines APU3 board which has AMD Hudson-2 EHCI USB ports on it. If devices are
plugged in when the system boots and the ports are initialized, the operating
system sees they are there. However, if you hot plug a device after it is
booted, or remove a device that was plugged in at boot, the system does not
notice the change in state. Also, the Sierra wireless LTE modem I was trying to
use does not function, the driver sends an open message over the USB bus to the
device and then nothing ever comes back. Once the system is booted, the
interrupt account for the ECHI ports from 'vmstat -I' never increases. The xHCI
USB ports on the same system seemed to work fine, detecting hot plug/remove
events, and properly initializing the wireless modem.
> What makes you think that the quirks below
> will help? What do you mean with 'work fine on those systems'? If they
> work fine, which issues are you having?
The same board when booted up under either Linux or FreeBSD appears to have
fully functional EHCI USB ports; they detect hot plug/remove events, and under
linux the wireless modem is initialized and can successfully pass traffic
(FreeBSD doesn't have a driver for it, so I was unable to test it there).
I honestly don't know if these specific quirks will resolve the issue under
OpenBSD, all I know is that there is something Linux/FreeBSD is doing different
regarding the USB hardware, and porting these quirks seemed a good place to
start. I'm not really a low level hardware/device driver guy, so I'm flying a
bit blind. Someone told me there are known issues with amd USB ports in
general, such as ath based USB wireless cards not working, so these might help
that problem even if it doesn't fix mine.
> It depends how the controller is connected to the host. If you look at
> the PCI glue driver, dev/pci/ehci_pci.c you'll see
>
> 115: /* Map I/O registers */
> 116: if (pci_mapreg_map(pa, PCI_CBMEM, PCI_MAPREG_TYPE_MEM, 0,
> 117: &sc->sc.iot, &sc->sc.ioh, NULL, &sc->sc.sc_size, 0))
>
> Then in the EHCI driver, dev/usb/ehci.c, these are accessed via the
> EREAD/EWRITE/EOREAD/EOWRITE macros.
Ah, ok; it appears that pci_mapreg_map defines the start of the region as:
ex = pa->pa_ioex;
if (ex != NULL) {
start = max(PCI_IO_START, ex->ex_start);
with:
#define PCI_IO_START 0
So the starting memory address is either 0 or pa->pa_ioex from the struct
pci_attach_args that was passed into ehci_pci_attach.
Given the existing reads:
sc->sc.sc_offs = EREAD1(&sc->sc, EHCI_CAPLENGTH);
define EHCI_CAPLENGTH 0x00
I'm pretty sure it's not zero, so it must be the one from pa_ioex.
> But maybe you just want to use pci_conf_read()/pci_conf_write()?
Hmm, given my lack of detailed knowledge of this area I can't say for sure.
However, there are two different things being done in the linux code I am
referring to:
outb_p(0xe0, 0xcd6);
This I believe writes the byte 0xe0 to the I/O port at address 0xcd6, where as
this:
pci_write_config_dword(amd_chipset.nb_dev, 0xe4, val);
Writes the contents of the variable val to the PCI configuration register
located at 0xe4. Those are two different operations, right?
So wherever the linux code writes to an absolute I/O port for the USB device,
such as 0xcd6, I can subtract the beginning of the mapped region as stored in
pa_ioex from it to arrive at the appropriate offset value to use with
EREAD/WRITE?
> Is low power mode enabled on OpenBSD?
Based on the comment in the linux code:
"The hardware normally enables the A-link power management feature, which
lets the system lower the power consumption in idle states."
I believe that is the default behavior of the hardware unless you explicitly do
something otherwise with it.
> The other quirk involves never having an empty frame list; I have
> > implemented the logic to detect when that is required, but haven't even
> > come close to wrapping my head around actually implementing the quirk
> > itself.
>
> For which transfer type is this quirk required, isochronous only?
The explanation of this quirk is:
"EHCI controller on AMD SB700/SB800/Hudson-2/3 platforms may
read/write memory space which does not belong to it when
there is NULL pointer with T-bit set to 1 in the frame list
table. To avoid the issue, the frame list link pointer
should always contain a valid pointer to a inactive qh."
I am also unfortunately not that expert in the underlying USB hardware level
protocol, but the quirk is referenced in two functions, scan_isoc:
if (!ehci->use_dummy_qh ||
q.itd->hw_next != EHCI_LIST_END(ehci))
*hw_p = q.itd->hw_next;
else
*hw_p = cpu_to_hc32(ehci, ehci->dummy->qh_dma);
and periodic_unlink:
if (!ehci->use_dummy_qh ||
*shadow_next_periodic(ehci, &here, Q_NEXT_TYPE(ehci, *hw_p))
!= EHCI_LIST_END(ehci))
*hw_p = *shadow_next_periodic(ehci, &here,
Q_NEXT_TYPE(ehci, *hw_p));
else
*hw_p = cpu_to_hc32(ehci, ehci->dummy->qh_dma);
Based solely on the name of the first function, it sounds like it might be.
> Thanks for sending a diff. Don't hesitate to send them to tech@ next
> time.
Okay, thanks; I wasn't sure what the etiquette was for posting to that list :).
> > + /* AMD errata indicates 8111 chipset EHCI is broken */
> > + if (PCI_PRODUCT(pa->pa_id) ==
> PCI_PRODUCT_AMD_8111_EHCI) {
> > + printf("%s: AMD 8111 EHCI broken, skipping",
> devname);
> > + goto disestablish_ret;
>
> If it is broken we should not match it. That means ehci_pci_match()
> should contain a check for this model an return 0 instead of 1.
Okay, I will move this check there.
> This could be a diff on its own, but please make sure to give a
> reference for where you found the piece of information. A vendor
> document is always the best, does it exist an errata for this chip?
> At least the Linux commit hash such that it makes reviewer life easy ;)
I'll try to track down a good reference for the information.
> > + if (pci_find_device(&amd_pa, ehci_amd_pll_quirk_match)) {
> > + sc->sc.amd_chipset.rev =
> PCI_REVISION(amd_pa.pa_class);
> > + if (PCI_PRODUCT(amd_pa.pa_id) ==
> PCI_PRODUCT_ATI_SBX00_SMB) {
> > + if (sc->sc.amd_chipset.rev >= 0x10 &&
> > + sc->sc.amd_chipset.rev <= 0x1f)
> > + sc->sc.amd_chipset.gen =
> AMD_CHIPSET_SB600;
>
> You don't need to reset the value, just read the code in this file, you
> can use the PCI_REVISION() and PCI_PRODUCT() macro ;)
Hmm, I'm not sure I follow you? I am using the the PCI_REVISION() and
PCI_PRODUCT() macros. The variable that is being set is actually a new one, the
linux code does different stuff for a given USB controller depending on the
revision of the northbridge chipset. This code reviews the combinations of
PCI_PRODUCT and ranges of PCI_REVISION so it will know what to do later.
Otherwise, every time it needed to bang the hardware to change the power mode
it would have to figure it out again.
Thanks much for the info and advice, I will take another pass at the code and
come back :).
