On 2016/11/12 6:09, Bjorn Helgaas wrote: > On Tue, Oct 18, 2016 at 09:41:27AM +0800, Shawn Lin wrote: >> The default value of common clock configuration is >> zero indicating Rockchip's RC is using asynchronous >> clock architecture but actually we are using common >> clock. This will confuses some EP drivers if they >> need some different settings referring to this value. >> So let's fix it. >> >> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com> >> >> --- >> >> Changes in v4: >> - rebase on the next branch >> >> Changes in v3: >> - rebase the code since it isn't cleanly applied again >> >> Changes in v2: >> - rebase the code since it isn't cleanly applied after Bjorn's cleanup >> >> drivers/pci/host/pcie-rockchip.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c >> index 3ede865..8e260d2 100644 >> --- a/drivers/pci/host/pcie-rockchip.c >> +++ b/drivers/pci/host/pcie-rockchip.c >> @@ -141,6 +141,7 @@ >> #define PCIE_RC_CONFIG_DCR_CPLS_SHIFT 26 >> #define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0) >> #define PCIE_RC_CONFIG_LCS_RETRAIN_LINK BIT(5) >> +#define PCIE_RC_CONFIG_LCS_CCC BIT(6) >> #define PCIE_RC_CONFIG_LCS_LBMIE BIT(10) >> #define PCIE_RC_CONFIG_LCS_LABIE BIT(11) >> #define PCIE_RC_CONFIG_LCS_LBMS BIT(30) > > These look an awful lot like these generic PCIe definitions: > > #define PCI_EXP_LNKCTL 16 /* Link Control */ > #define PCI_EXP_LNKCTL_ASPMC 0x0003 /* ASPM Control */ > #define PCI_EXP_LNKCTL_ASPM_L0S 0x0001 /* L0s Enable */ > #define PCI_EXP_LNKCTL_ASPM_L1 0x0002 /* L1 Enable */ > #define PCI_EXP_LNKCTL_RCB 0x0008 /* Read Completion Boundary */ > #define PCI_EXP_LNKCTL_LD 0x0010 /* Link Disable */ > #define PCI_EXP_LNKCTL_RL 0x0020 /* Retrain Link */ > #define PCI_EXP_LNKCTL_CCC 0x0040 /* Common Clock Configuration */ > #define PCI_EXP_LNKCTL_ES 0x0080 /* Extended Synch */ > #define PCI_EXP_LNKCTL_CLKREQ_EN 0x0100 /* Enable clkreq */ > #define PCI_EXP_LNKCTL_HAWD 0x0200 /* Hardware Autonomous Width Disable */ > #define PCI_EXP_LNKCTL_LBMIE 0x0400 /* Link Bandwidth Management Interrupt Enable */ > #define PCI_EXP_LNKCTL_LABIE 0x0800 /* Link Autonomous Bandwidth Interrupt Enable */ > > If these are indeed the same, I'd really like it if you could use the > generic definitions, e.g., by something like: > Ahh, thanks for sharing this. PCIE_RC_CONFIG_LCS is short for link control and status register for RC. So the register layout looks the same for that of PCI_EXP_LNKCTL after checking the TRM again. I will come up a cleanup patch to use the existed PCI_EXP_LNKCTL_XXX instead. :) > #define PCIE_RC_PCIE_CAP_BASE (PCIE_RC_CONFIG_BASE + 0xc0) > > status = rockchip_pcie_read(rockchip, PCIE_RC_PCIE_CAP_BASE + PCI_EXP_LNKCTL); > status |= PCI_EXP_LNKCTL_CCC; > > That way grep has a chance to find related pieces. > > If they only *look* similar but are not really the same, then > disregard this, of course. > >> @@ -536,6 +537,11 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) >> >> rockchip_pcie_set_power_limit(rockchip); >> >> + /* Set RC's clock architecture as common clock */ >> + status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS); >> + status |= PCIE_RC_CONFIG_LCS_CCC; >> + rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS); >> + >> /* Enable Gen1 training */ >> rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE, >> PCIE_CLIENT_CONFIG); >> -- >> 2.3.7 >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-pci" in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Best Regards Shawn Lin
