Hi Sergio,

On 27/5/19 6:02 pm, Sergio Paracuellos wrote:
On Mon, May 27, 2019 at 9:29 AM Greg Ungerer <g...@kernel.org> wrote:
On 27/5/19 4:35 pm, Sergio Paracuellos wrote:
On Mon, May 27, 2019 at 6:37 AM Greg Ungerer <g...@kernel.org> wrote:
On 24/5/19 3:35 pm, Sergio Paracuellos wrote:
On Fri, May 24, 2019 at 2:35 AM Greg Ungerer <g...@kernel.org> wrote:
On 23/5/19 3:26 pm, Sergio Paracuellos wrote:
On Thu, May 23, 2019 at 4:11 AM Greg Ungerer <g...@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?

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?

Yes, just that (and the gpio based reset hack) is enough to enumerate the bus.

Ok, so this is problem shouldn't be present in the current staging and
linux tree at master where the
device tree is not using child nodes, which is the way to go.

I cloned the staging tree from git.kernel.org and tried that.
It didn't work any better, I had to patch pci-mt7621.c and
pci-mt8721-phy.c in the same ways to get an almost working result.


I think we should add a gpio reset line in the device tree and use it
properly with
the gpio descriptor api. Maybe this is better for all the boards
instead of use the builtin stuff.

Would seem to be the best approach.


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.

Not sure either why this would be different.
I'll dig into that tomorrow too.

Thanks, let me know any advance on this, please.

I suspect that the bus or devices stop reading/writing valid data
at some point in this initialization process. What I see is that
dumping /sys/bus/pci/devices/0000:01:00.0/config on a running
system shows:

  00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
  00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
  00000020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
  ...

But if I replace the PCI code again with that from 4.20 then
I get a valid dump of the wifi card config space:

  00000000: 8c 16 3c 00 06 00 10 00 00 00 80 02 00 00 00 00     ..<.............
  00000010: 04 00 00 60 00 00 00 00 00 00 00 00 00 00 00 00     ...`............
  00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00     ................

Regards
Greg


Regards
Greg

Best regards,
     Sergio Paracuellos



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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to