Page MenuHomeFreeBSD

Add StarFive JH7110's PCIE controller driver
Needs ReviewPublic

Authored by jsihv_gmx.com on Dec 4 2024, 6:14 PM.
Tags
None
Referenced Files
F107786257: D47919.diff
Sat, Jan 18, 4:50 AM
Unknown Object (File)
Mon, Jan 6, 7:21 AM
Unknown Object (File)
Mon, Jan 6, 5:36 AM
Unknown Object (File)
Mon, Jan 6, 4:39 AM
Unknown Object (File)
Mon, Jan 6, 2:30 AM
Unknown Object (File)
Thu, Dec 19, 11:02 AM
Unknown Object (File)
Dec 18 2024, 6:04 AM
Unknown Object (File)
Dec 8 2024, 9:24 PM
Subscribers

Details

Reviewers
None
Group Reviewers
riscv
Summary

This driver requires jh7110_gpio.c and jh7110_clk_stg.c

JH7110 has two PCIE controller devices. First one is used by board's integrated USB which has no driver. Switching PHY to USB mode is not currently implemented. This functionality could be added in a form of a separate PCIE PHY driver if needed. PHY is on by default and there's no need to switch it on.

Pre/post_ithread and post_filter methods are not used for interrupt masking since they are meant for level-triggered interrupts whereas JH7110's MSI interrupts are edge triggered (and INTx interrupts do not use this irqsrc scheme at all). Pre_ithread method is nevertheless used for MSI bottom acking.

The driver has been tested with Kingston SNV2S NVME SSD
The functionality of INTx and MSI interrupts (as opposed to default MSIx) has been tested by forcing NVME to use them

There is a some kind of problem with memory allocation which happens in function pci_pci.c:pcib_probe_windows(). It seems the function and its PCI_PPBMEMBASE() macro are not capable of dealing with 64-bit addresses in cases when both 32-bit parts are used to form the address. This problem concerns the address 0x980000000 [0x9 0x80000000] which is used by the second PCIE device. The rejection of a formed faulty address eventually happens in subr_rman.c:rman_reserve_resource_bound(). Other pcie-drivers which fetch ranges (tegra_pcie.c, rk_pcie.c) seem to only use shorter one part addresses on their device trees. This problem leads to one failed memory allocation during the boot but seems not to stop the device from functioning.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Now adding this file to files.starfive

More about the memory allocation issue: It looks now that in pcib_probe_windows() a wrong value is read to "val" variable from register PCIR_PMBASEL_1. I don't suspect a bug in the PCI code anymore. One developer said it's probably related to U-Boot.

This is not a full review, just a couple of quick notes.

I pulled the change and will be testing it out :)

sys/riscv/starfive/files.starfive
7

You did not add device jh7110_pcie anywhere, and therefore did not enabled the driver.

Instead I suggest just making it optional on fdt pci, we don't need the new device IMO.

sys/riscv/starfive/jh7110_pcie.c
9โ€“10

We don't include this header anymore.

257

Preferred style for this type of mask check.

998โ€“1000

To follow recent bus API changes.

I agree with all the comments.

This update fixes small issues mentioned in previous comments and removes KVCO adjustments. I also report here about a U-Boot dependency.

I found out that I had done a careless mistake with KVCO adjustments. Registers used there were all plain wrong. It seems that KVCO adjustment should be done by using PCIE-PHY's registers (StarFive's own FDT is different in this respect, they have KVCO in their own kernel's pcie driver).

I'm not sure what the actual meaning of KVCO adjustments is (U-Boot doesn't have them) but if we want them, then we should implement a PCIE-PHY driver (or make some kind of hack here to reach another device's registers).

Otherwise PCIE-PHY driver could potentially contain just mode selection but FreeBSD's xhci driver seems not to use these modes and USB and NVME seem to work as one would expect without switching any modes, so it seems our PCIE-PHY driver would have nothing but this KVCO adjustment (two write calls). Would that be fine? It wouldn't take much if any time to implement that.

By building U-Boot without PCIE and/or USB, I found out that PCIB0 (the first PCIE device) is currently dependent on U-Boot's PCIE driver for JH7110. Despite of extensive debugging, I haven't yet found out the actual reason for this dependency. Currently it seems it's something in a function called starfive_pcie_init_port() in U-Boot. No matter how odd it may sound, while the second PCIe device works fine without U-Boot's PCIE driver, PCIB0's data link is not found.

Note that PCIE-PHY and USB-PHY drivers do not appear in U-Boot which uses xhci driver for USB like we do. I doesn't seem to me now that USB-PHY driver would do anything that we need.