Conversation
|
Ive enabled and tested this driver across devices in my fork but intentionally left that out of this PR. I am planing to do individual follow-up PRs for that unless you'd rather one large PR. |
|
Wow, great stuff! Can you please move the first unrelated DP changes to a separate PR (could include the polling timeout fixes too) and perhaps squash some of the Fusb302 commits where the dev history isn't that relevant? |
|
Merged the first two commits in the other PR. Can drop them from this one and rebase. You could also add the platform enablement code via a single commit at the top here too, no need for another PR. I can test it on hardware a bit later. |
|
Rebased to master, added enablement commits. My branch rebased devicetrees to 7.2 and my testing was done on it against kernel 7.1.8.. USB-DP also requires kernel driver be enabled which it is not be default in Debian 13 anyway. Hold off on merge untill either update to 7.2 which i can also PR, or i will have to modify it for current. |
|
Platform enablement commits can be squashed into one, with a single README update commit at the end. PowerStation 6: Give combo PHY 2 a mode so USB 3 comes up is unrelated to this patchset and should be submitted later. But are we sure that the USB-A is wired to a ComboPHY rather than USBDP? |
Yeah it's better if we update the device trees before this. |
|
Hopefully to your liking, DTS PR sent also. PowerStation 6: Give combo PHY 2 a mode so USB 3 comes up is unrelated to this patchset and should be submitted later. But are we sure that the USB-A is wired to a ComboPHY rather than USBDP?
|
mariobalanica
left a comment
mariobalanica
left a comment
There was a problem hiding this comment.
At guick glance the code looks reasonable to me. Left one review.
Regarding higher-voltage negotiation, we kind of need to ensure that happens as early as possible, i.e. before host USB and PCIe ports get enabled, since power-hungry peripherals can cause brownouts depending on the board & PSU. It's not a blocker for this patchset, I can take a look at it myself once we get this merged.
Shared Type-C connectors only ever worked in one plug orientation, because the combo PHY took its lane mapping from a fixed board description that cannot know which way the cable went in. Add a driver for the ON Semiconductor FUSB302 that reports plug orientation from BC_LVL on both CC pins through a USB_TYPE_C_PORT_PROTOCOL. Detection probes as a sink first, where the source pulls the connected pin up and it reads higher than the open one, and falls back to source with Rp on both pins, where the comparison inverts: our own Rp holds an unconnected pin at the top of the range while the partner's Rd drags the connected one down. An electronically marked cable presents Ra lower still, so a pin at the top band identifies the open one, and where neither is there, the higher of the two is the one carrying Rd. On top of that the driver negotiates a USB Power Delivery contract as a sink, enters DisplayPort Alternate Mode and reports hot-plug detect, and sources power on a port whose board can switch the rail -- guarded so that sourcing starts only once the partner is confirmed a sink and VBUS is idle beforehand. Requests a partner makes in passing are answered rather than ignored, since a request left hanging strands it part way through an exchange. What the driver knows about the board arrives entirely through FUSB302_PLATFORM_DEVICE_PROTOCOL: where the controller sits, which PHY it belongs to, what it may source and will sink, and a callback to switch VBUS. Boards fill that in through PlatformGetTypeCPort() in RockchipPlatformLib, and Fusb302PlatformRkDxe publishes one instance per port -- the library is [LibraryClasses.common] with no constructor, so it has nowhere to install a protocol itself. The driver therefore has no PCDs and no GpioLib dependency, and a board switching VBUS through a GPIO expander can say so without the driver changing. Verified against a FUSB302B on a PowerStation 6, cross-checked against the Linux typec_fusb302 driver. Off unless a platform sets RK_FUSB302_ENABLE, which defaults to FALSE.
Orientation was only ever applied from DpPhyPowerOn(), so plain USB 3 on a shared connector still used the board default mapping and still worked in one orientation only. Apply it during rockchip_u3phy_init() dispatch instead, waiting on a protocol notify if the port controller has not bound yet. Pin assignments C and E give DisplayPort all four lanes while D splits two and two. A board can only describe one, so a partner that picked the other ran the wrong lane mux; re-derive the four-lane mapping from the orientation instead of only warning about the mismatch. Also throw the SBU switches to match the plug on boards that break them out through DC blocking switches -- AUX rides on SBU1/SBU2, which swap when the plug flips, and this driver never touched them before.
Two ordering problems kept the connector from getting a usable answer. The DP PHY is powered on from dw_dp_connector_init(), which runs before the port controller on I2C has said which way the plug is, so a flipped plug got the wrong lane assignment and AUX polarity and every AUX transaction timed out. Ask again and power the PHY a second time once alternate mode is up, by which point the answer is known. Display detection and I2cDxe's own EndOfDxe handler also run at the same TPL, with detection first, so the port controller had not been connected and the connector was written off before it was ever asked about. Connect the I2C masters the first time display detection goes looking; ConnectController is idempotent.
The driver landed with nothing turning it on. Enable it wherever a board's own device tree describes the chip: seventeen platforms, from the PowerStation 6 and Blade3 to the ROCK 5B/5B+ and the Orange Pi 5 and 5 Plus. Each names its controllers' buses and addresses and the PHY each one belongs to in its .dsc, and describes the ports themselves -- what they may source and will sink, and how to switch the rail -- through PlatformGetTypeCPort(). All of it is read from that board's own device tree. Fourteen of the seventeen also switch their own VBUS, wherever a board describes a dedicated switchable supply on an SoC GPIO the driver can reach -- Blade3 among them, with a separate callback per port. Three stay sink-only: the ITX-3588J and AIO-3588Q drive theirs through a PCA9555 expander, which the board hook could reach through PCA95XX_PROTOCOL but which there is no board here to test on, and the ROCK 5B does not fit the switch at all. Sixteen of the seventeen also get their SBU switches configured, so AUX follows the plug. The AIO-3588Q is the exception: its SBU DC blocking switches hang off the same PCA9555 as its VBUS enable, and PcdUsbDpPhy0SbuGpios names an SoC bank and pin. The ITX-3588J gains a DP0 lane mux, without which the driver assumes four lanes and leaves none for USB 3 on that connector. Edge2 and Orange Pi 5 Plus get DisplayPort Alt Mode added to their mainline device trees. Counts are platforms rather than board models: Station M3 builds from ROC-RK3588S-PC's .dsc.inc and ships its device tree, so the two are one design under two names. The ROCK 5B and 5B+ are the ones to watch. The board is powered through that port, which is why its controller was disabled in the device tree to begin with. Establishing the PD contract in firmware is what a USB-C-powered board needs, but it is untested here: if a board reboots during firmware startup, unset RK_FUSB302_ENABLE in its platform .dsc. Only the PowerStation 6 is verified on hardware. Everything else is read from each board's own device tree, with the guards the driver applies everywhere.
The table said the FUSB302 was not working, and the DisplayPort and USB 3 rows described the one-orientation limits it removes. Say what is true now. A board with a FUSB302 matches the plug orientation, so USB devices enumerate either way up and DisplayPort Alt Mode carries detection, and the rows name the boards that do not have one. The DisplayPort note keeps the caveat that EDID and link training need the Type-C sideband routed to the connector, since a board that does not route it gets a default timing rather than the display's own.
| **/ | ||
| STATIC | ||
| FUSB302_PLATFORM_DEVICE_PROTOCOL * | ||
| Fusb302MatchPlatformPort ( |
There was a problem hiding this comment.
I think we can just install the platform device protocol directly onto the relevant I2cIo handles, drop this and simplify the protocol interface too.
I'll pull the changes for some tests and also look into the negotiation ordering issue so we can have a better idea of what the bindings should look like.
There was a problem hiding this comment.
Makes sense — that drops the lookup, and DeviceGuid/DeviceIndex with it,
since pairing the two up is all they're for.
One ordering wrinkle: I2cBus installs EFI_I2C_IO_PROTOCOL from its
DriverBindingStart(), and our I2cDxe only connects the masters from its
EndOfDxe handler, so those handles don't exist when a TRUE-depex module's
entry point runs. The platform side would have to install as they appear,
and Fusb302Dxe's depex on the platform protocol would have to go with it —
otherwise the protocol first shows up at EndOfDxe, which is the same moment
the driver needs to already be dispatched.
I'll hold off until you've looked at the ordering, since that may move the
bindings anyway.
There was a problem hiding this comment.
Anything needed from me to get this moving?
There was a problem hiding this comment.
No, still need to test the changes and see about those bindings, hopefully this week.
There was a problem hiding this comment.
See https://github.com/edk2-porting/edk2-rk3588/tree/PR-USBC
I dropped the platform glue driver and PCDs. Binding on I2cIo isn't worth it, the current match logic is good enough.
I'm going to test negotiation and look into the power sequencing next.
There was a problem hiding this comment.
Power sequencing is also fixed.
Need to do some more tests as I recall seeing boot loops on ROCK 5B after reboot.
|
Looks like I managed to fry the FUSB302 chip on my Edge2. Worked fine for a while, but now I2C just times out. ...probably backfed 12V VBUS somewhere but looking at the schematic that doesn't actually seem possible. Anyway, PD negotiation seems to work well. The only problem I noticed was powering through a dock: on reboot, the PSU resets when the driver tries to renegotiate. Could just be this specific dock. DisplayPort doesn't work, seems like the connector doesn't get installed to begin with: |
2 participants
USB-C Enablement
Adds a driver for the FUSB302 Type-C port controller and points the USBDP
combo PHY at it, so a Type-C port works in whichever orientation the plug is
in rather than the one the board guessed at build time.
Fusb302Dxe— plug orientation, a PD contract as a sink, DisplayPortAlternate Mode with hot-plug detect, and sourcing where the board can
switch its own rail.
UsbDpPhyDxe— lane mapping and SBU switches follow the reportedorientation instead of a fixed board setting.
DisplayLib— asks the Type-C port once the answer exists.A board describes each port through
PlatformGetTypeCPort()inRockchipPlatformLib, andFusb302PlatformRkDxepublishes oneFUSB302_PLATFORM_DEVICE_PROTOCOLper port. The driver reads only that — noPCDs, no
GpioLib.Enabled on seventeen platforms, fourteen of which can also source.
Tested on a PowerStation 6: a dock negotiates, enters DP alternate mode on a
flipped plug and drives video; a drive in reverse enumerates with the lanes
remapped. All 23 platforms build from cold.
Depends on #285 (merged).