Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver

From: Dilip Kota
Date: Tue Oct 29 2019 - 03:46:04 EST

On 10/22/2019 9:09 PM, Bjorn Helgaas wrote:
On Tue, Oct 22, 2019 at 05:07:47PM +0800, Dilip Kota wrote:
On 10/22/2019 1:17 AM, Bjorn Helgaas wrote:
On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
Add support to PCIe RC controller on Intel Gateway SoCs.
PCIe controller is based of Synopsys DesignWare pci core.

Intel PCIe driver requires Upconfig support, fast training
sequence configuration and link speed change. So adding the
respective helper functions in the pcie DesignWare framework.
It also programs hardware autonomous speed during speed
configuration so defining it in pci_regs.h.

+static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
+ u32 val;
+ val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
+ lpp->max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
+ lpp->max_width = FIELD_GET(PCI_EXP_LNKCAP_MLW, val);
+ val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
PCI_EXP_LNKCTL_CCC is RW. But doesn't it depend on the components on
both ends of the link? Do you know what device is at the other end?
I would have assumed that you'd have to start with CCC==0, which
should be most conservative, then set CCC=1 only if you know both ends
have a common clock.
PCIe RC and endpoint device are having the common clock so set the CCC=1.
How do you know what the endpoint device is? Is this driver only for
a specific embedded configuration where the endpoint is always
soldered down? There's no possibility of this RC being used with a

Shouldn't this be either discoverable or configurable via DT or
something? pcie_aspm_configure_common_clock() seems to do something
similar, but I can't really vouch for its correctness.

(sorry for the late reply, i am back today from sick leave)

I see pcie_aspm_configure_common_clock() is getting called during pcie root bus bridge scanning and programming the CCC.

So, CCC configuration can be removed here in intel_pcie_link_setup().