staging: mt7621-pci: factor out 'mt7621_pcie_enable_port' function
Sergio Paracuellos
sergio.paracuellos at gmail.com
Fri May 24 05:35:33 UTC 2019
Hi Greg,
On Fri, May 24, 2019 at 2:35 AM Greg Ungerer <gerg at kernel.org> wrote:
>
> Hi Sergio,
>
> On 23/5/19 3:26 pm, Sergio Paracuellos wrote:
> > On Thu, May 23, 2019 at 4:11 AM Greg Ungerer <gerg at kernel.org> 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?
>
> 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
> >>>
> >
More information about the devel
mailing list