Hi Greg, On Mon, May 27, 2019 at 6:37 AM Greg Ungerer <gerg@xxxxxxxxxx> wrote: > > Hi Sergio, > > On 24/5/19 3:35 pm, Sergio Paracuellos wrote: > > On Fri, May 24, 2019 at 2:35 AM Greg Ungerer <gerg@xxxxxxxxxx> wrote: > >> On 23/5/19 3:26 pm, Sergio Paracuellos wrote: > >>> On Thu, May 23, 2019 at 4:11 AM Greg Ungerer <gerg@xxxxxxxxxx> wrote: > >>>> On 22/5/19 4:27 pm, Sergio Paracuellos wrote: > >>>> [snip] > >>>>> There are some big changes between 4.20 and 5.x. One is the use of PERST_N > >>>>> instead of using gpio. This PERT_N stuff is used now on enable ports > >>>>> assuming the > >>>>> link of PCI is properly detected after enabling the phy. And it seems > >>>>> it is not according to > >>>>> your dmesg traces. The previous 4.20 code used gpio before this was done. > >>>>> > >>>>> This code is the one I am referring: > >>>>> > >>>>> /* Use GPIO control instead of PERST_N */ > >>>>> *(unsigned int *)(0xbe000620) |= BIT(19) | BIT(8) | BIT(7); // set DATA > >>>>> mdelay(1000); > >>>> > >>>> I have been looking closely at those, wondering why the old code > >>>> drove that PERST line as a GPIO instead of using the built-in behavior. > >>>> (I have ignored bits 7 and 8 here since they are control of UART 3) > >>> > >>> Yes, this was also at first one of my big concerns so I tried to change into > >>> to use builtin behaviour (which is much more cleaner) and when the > >>> code was tested > >>> it worked. It seems it is not valid for every board. > >>> > >>>> > >>>> > >>>>> I assume reset lines on your device tree are properly set up which is > >>>>> other of the big changes here: use > >>>>> reset lines instead of that hardcoding stuff. Also, the > >>>>> mt7621_reset_port routine is also using msleep(100) > >>>>> but maybe you can try a bigger value and change it into a mdelay, to > >>>>> see if that changes anything. > >>>> > >>>> I see the reset line configuration in the pcie section of mt7621.dtsi, > >>>> is there any others absolutely required here? I couldn't see the > >>>> gbpc1.dts devicetree do anything else with pcie - othe than enable it. > >>>> My device tree for the EX15 is similar in that regard. > >>>> > >>>> I tried a couple of things with interesting results. > >>>> > >>>> 1. I made sure that the PERST_N line is set for PCIe operation (not GPIO). > >>>> I forced it with: > >>>> > >>>> *(unsigned int *)(0xbe000060) &= ~(0x3 << 10); > >>>> > >>>> I checked bits 10 and 11 at kernel PCI init and they were 00 anyway. > >>>> So PERST_N was definitely in PCIe reset mode. No change in behavior, > >>>> > >>>> > >>>> 2. I forced a GPIO style reset of that PERST line (using GPIO19) and got > >>>> the following result on kernel boot: > >>>> > >>>> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 0 > >>>> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz > >>>> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 1 > >>>> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK) > >>>> mt7621-pci 1e140000.pcie: Initiating port 1 failed > >>>> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 2 > >>>> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz > >>>> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK) > >>>> mt7621-pci 1e140000.pcie: Initiating port 2 failed > >>>> mt7621-pci 1e140000.pcie: de-assert port 0 PERST_N > >>> > >>> This line seems to be the problem. When ports are init, (and with your > >>> changes seems the are > >>> init properly), the ports with pcie link are stored into a list to be > >>> enabled afterwards. This code is > >>> located into 'mt7621_pcie_enable_ports' which call simple > >>> 'mt7621_pcie_enable_port' to enable each port > >>> on the list. In this process it uses the PERS_N built-in register > >>> deasserting and asserting it. If enabling fails > >>> (and this is ypour case now) the port is removed from the list and it > >>> is not properly set up. You should try to > >>> comment this code: > >>> > >>> /* assert port PERST_N */ > >>> val = pcie_read(pcie, RALINK_PCI_PCICFG_ADDR); > >>> val |= PCIE_PORT_PERST(slot); > >>> pcie_write(pcie, val, RALINK_PCI_PCICFG_ADDR); > >>> > >>> /* de-assert port PERST_N */ > >>> val = pcie_read(pcie, RALINK_PCI_PCICFG_ADDR); > >>> val &= ~PCIE_PORT_PERST(slot); > >>> pcie_write(pcie, val, RALINK_PCI_PCICFG_ADDR); > >>> > >>> /* 100ms timeout value should be enough for Gen1 training */ > >>> err = readl_poll_timeout(port->base + RALINK_PCI_STATUS, > >>> val, !!(val & PCIE_PORT_LINKUP), > >>> 20, 100 * USEC_PER_MSEC); > >>> if (err) > >>> return -ETIMEDOUT; > >>> > >>> because on enabling, it seems it is getting ETIMEOUT and hence the > >>> message ' mt7621-pci 1e140000.pcie: de-assert port 0 PERST_N'. > >>> Commenting > >>> this code should end up into a properly configured pci? > >> > >> No, unfortunately it doesn't. It does show PCIE0 enabled now though: > > > > That is a surprise :( > > > >> > >> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 0 > >> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz > >> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 1 > >> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK) > >> mt7621-pci 1e140000.pcie: Initiating port 1 failed > >> mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 2 > >> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz > >> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK) > >> mt7621-pci 1e140000.pcie: Initiating port 2 failed > >> mt7621-pci 1e140000.pcie: PCIE0 enabled > >> mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002 > >> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00 > >> pci_bus 0000:00: root bus resource [io 0xffffffff] > >> pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff] > >> pci_bus 0000:00: root bus resource [bus 00-ff] > > > > > >> > >> And again no devices are found on the PCI bus. > >> (System did still boot too). > > > > Looking to your original trace of linux-4.20 working the init traces > > are pretty much the same... I don't really know what could be > > happening there. Root resources > > are correct, virtual bridge seems to be detected, the next should be > > to reconfigure resources of > > the bridge and this is done by the pci kernel apis. > > > > Can you check that "mt7621_pcie_init_virtual_bridges" is getting link > > up and configuring bridges > > correctly? > > Yes, it does get link there. It sees pcie_link_status as 0x1, so its getting > through that. > > I threw a bit of trace in to see where we end up losing the ability to > read correct config data from slot 0 (my only valid slot). It gets to > the "err_no_link_up:" label for port/slot 2 still being able to read config > space, but then after executing the phy_power_off() and phy_exit() > calls for that port/slot we can no longer read config for slot 0. Mmmm. I see. So phy instances for port 0 and 2 are different instances of the phy, so it should not have problems for the power_off function. Looking again to the version which is in the 5.0 linux (but not in the last changes of staging where no child nodes are being used) I can see the phy_exit function is disabling the clock using PCIE_PORT_CLK_EN which is defined as: #define PCIE_PORT_CLK_EN(x) BIT(24 + (x)) On probe function index is being set to 0 for the port 2 also, instead of 2 (which is the correct value). Just try to comment this line: rt_sysc_m32(PCIE_PORT_CLK_EN(instance->index), 0, RALINK_CLKCFG1); Does this enough to get the pci enumeration being done correctly? > > If I comment out the code in phy_power_off() and phy_exit() so they > return doing nothing then I get further: > > mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 0 > mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz > mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 1 > mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK) > mt7621-pci 1e140000.pcie: Initiating port 1 failed > mt7621-pci 1e140000.pcie: Port 454043648 N_FTS = 2 > mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz > mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK) > mt7621-pci 1e140000.pcie: Initiating port 2 failed > mt7621-pci 1e140000.pcie: PCIE0 enabled > mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002 > mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00 > pci_bus 0000:00: root bus resource [io 0xffffffff] > pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff] > pci_bus 0000:00: root bus resource [bus 00-ff] > pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring > pci 0000:00:00.0: PCI bridge to [bus 01-ff] > pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000] > pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000] > pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff] > pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref] > pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff] > pci 0000:00:00.0: BAR 7: no space for [io size 0x1000] > pci 0000:00:00.0: BAR 7: failed to assign [io size 0x1000] > pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit] > pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref] > pci 0000:00:00.0: PCI bridge to [bus 01] > pci 0000:00:00.0: bridge window [mem 0x60000000-0x601fffff] > pci 0000:00:00.0: bridge window [mem 0x60200000-0x602fffff pref] > pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22 > pcieport 0000:00:00.0: enabling device (0004 -> 0006) > > > So devices found, but interrupt setup failed for some reason. > I have an atheros PCIe WIFI device on that bus which is detected, but > the interrupt failure means it still doesn't actually work. Nothing has changed about interrupts from linux 4.20 version to this. It is returning -EINVAL for some reason. Irq is set using "of_irq_parse_and_map_pci" function. > > Regards > Greg Best regards, Sergio Paracuellos > > > >> I'll keep digging. > > > > Thanks, really appreciate it. > > > >> > >> Thanks > >> Greg > > > > Best regards, > > Sergio Paracuellos > > > >> > >> > >>> > >>>> mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002 > >>>> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00 > >>>> pci_bus 0000:00: root bus resource [io 0xffffffff] > >>>> pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff] > >>>> pci_bus 0000:00: root bus resource [bus 00-ff] > >>>> > >>>> And the system continued on to fully boot. So it looks like it thinks > >>>> pcie0 is active. Better, but the PCI bus probe didn't find any of the > >>>> devices it should have. > >>> > >>> Yes, that seems what is happening because of my explanation above. > >>> > >>>> > >>>> I inserted the quick hack code to do this at the top of mt7621_pcie_init_ports() > >>>> and it looked like this: > >>>> > >>>> /* Force PERST PCIe line into GPIO mode */ > >>>> *(unsigned int *)(0xbe000060) &= ~(0x3 << 10); > >>>> *(unsigned int *)(0xbe000060) |= BIT(10); > >>>> mdelay(100); > >>>> > >>>> *(unsigned int *)(0xbe000600) |= BIT(19); // use GPIO19 (PERST_N/) > >>>> mdelay(100); > >>>> *(unsigned int *)(0xbe000620) &= ~(BIT(19)); // clear DATA > >>>> mdelay(1000); > >>>> > >>>> /* Use GPIO control instead of PERST_N */ > >>>> *(unsigned int *)(0xbe000620) |= BIT(19); // set DATA > >>>> mdelay(1000); > >>>> > >>>> > >>> > >>> I really hate the gpio way of doing this. If this works we have to > >>> think in how to achieve this in a cleaner way :)) > >>> > >>>> Regards > >>>> Greg > >>> > >>> Thanks for your effort in this. > >>> > >>> Best regards, > >>> Sergio Paracuellos > >>>> > >>>> > >>>>> Other big change is to use the new phy driver but I think the problem > >>>>> seems to be related with resets. > >>>>> > >>>>>> > >>>>>> Regards > >>>>>> Greg > >>>>> > >>>>> Please, don't hesitate to ask me whatever you need to. > >>>>> > >>>>> Hope this helps. > >>>>> > >>>>> Best regards, > >>>>> Sergio Paracuellos > >>>>> > >>> > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel