cherry-embedded / CherryUSB

CherryUSB is a tiny, beautiful and portable USB host and device stack for embedded system with USB IP
https://cherryusb.readthedocs.io/
Apache License 2.0
1.21k stars 256 forks source link

BL616 reports "port 1 does not enable power" #222

Closed harbaum closed 1 month ago

harbaum commented 1 month ago

I just updated my bl616 setup to the latest cherryusb 1.3.1 and since then the bl616 runs into this error:

[I/USB] EHCI HCIVERSION:0x0100
[I/USB] EHCI HCSPARAMS:0x000001
[I/USB] EHCI HCCPARAMS:0x0006
[I/USB] EHCI ppc:0, n_ports:1, n_cc:0, n_pcc:0
[I/USB] EHCI uses tt for ls/fs device
[W/usbh_hub] Port 1 does not enable power

The attached device is not accessed after that. Still unplugging is detected:

[I/usbh_hub] Device on Bus 0, Hub 1, Port 1 disconnected

Replugging it gives the same problem: [W/usbh_hub] Port 1 does not enable power

sakumisu commented 1 month ago

I see your log, ppc is not 1, ehci must be 1.

sakumisu commented 1 month ago

image

sakumisu commented 1 month ago

So this is the bug of bl616, it means bl616 ehci ip is not standard, you need modify g_ehci_hcd[bus->hcd.hcd_id].ppc=1 to fix

harbaum commented 1 month ago

Changed like so:

#if defined(BL616)
    g_ehci_hcd[bus->hcd.hcd_id].ppc = true;
#else
    g_ehci_hcd[bus->hcd.hcd_id].ppc = (EHCI_HCCR->hcsparams & EHCI_HCSPARAMS_PPC) ? true : false;
#endif

The result has ppc set. But that does not solve the problem:

[I/USB] EHCI HCIVERSION:0x0100
[I/USB] EHCI HCSPARAMS:0x000001
[I/USB] EHCI HCCPARAMS:0x0006
[I/USB] EHCI ppc:1, n_ports:1, n_cc:0, n_pcc:0
[I/USB] EHCI uses tt for ls/fs device
[W/usbh_hub] Port 1 does not enable power
sakumisu commented 1 month ago

remove if in

                if (temp & EHCI_PORTSC_PP) {
                    status |= (1 << HUB_PORT_FEATURE_POWER);
                }
sakumisu commented 1 month ago

Because bl616 ehci does not have ppc, so temp will not match EHCI_PORTSC_PP

harbaum commented 1 month ago

That fixes it! Thanks.

#if !defined(BL616)
                if (temp & EHCI_PORTSC_PP)
#endif
            {
                    status |= (1 << HUB_PORT_FEATURE_POWER);
                }
sakumisu commented 1 month ago

I hate those off-standard ips because we need to do many patches for them.

harbaum commented 1 month ago

What about:

                if (temp & EHCI_PORTSC_PP || !(EHCI_HCCR->hcsparams & EHCI_HCSPARAMS_PPC) ) {
                    status |= (1 << HUB_PORT_FEATURE_POWER);
                }

?

sakumisu commented 1 month ago

Just always enable this bit i think.

harbaum commented 1 month ago

The code above doesn't change your code logic on other platforms but fixes the bl616 without any ifdef. Works for me ...

sakumisu commented 1 month ago

You are right, can you push mr for this patch?

sakumisu commented 1 month ago

This feature is not in v1.3.1 but in master branch, for stable, you can checkout release tag.