Am 01.11.18 um 21:06 schrieb Sinan Kaya:
> On 11/1/2018 3:22 PM, Stefan Mätje wrote:
>> + b/drivers/pci/pcie/aspm.c
>> @@ -268,6 +268,15 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
>> /* Retrain link */
>> reg16 |= PCI_EXP_LNKCTL_RL;
>> pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
>> + if (0x12d8 == parent->vendor && 0xe111 == parent->device) {
>> + /*
>> + * Due to an erratum in the Pericom PI7C9X111SLB bridge in
>> + * reverse mode the retrain link bit needs to be cleared
>> + * again manually to allow the link training to succeed.
>> + */
>> + reg16 &= ~PCI_EXP_LNKCTL_RL;
>> + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
>> + }
>
> The typical model is to abstract quirk work into quirks.c and add some
> callbacks from the actual code.
Yes, I'm aware of the quirks.c code. But I don't believe the problem can be solved
by a quirk function that is run via pci_fixup_device() at certain points of the
PCI scan (i. e. pci_fixup_pass like pci_fixup_early / pci_fixup_header ...) after
pcie_aspm_cap_init() has run.
Let's have a look at the function pcie_aspm_cap_init() from where pcie_aspm_configure_common_clock()
is called (where the patch was included). Be aware of the fact that the PCI express link downstream
is broken after leaving that function without the patch. But looking in pcie_aspm_cap_init() you can
see that it is reloading the ASPM registers from the child device after returning from
pcie_aspm_configure_common_clock() and from this point on it is working on bogus ASPM register
contents.
Therefore I think the rest of pcie_aspm_cap_init() is doing nothing sensible for the downstream
PCIe tree. Also I think that pcie_aspm_configure_common_clock() must be fixed in a way that after
leaving that function the PCIe downstream link is still working. This is what my patch is good for.
Best regards,
Stefan
