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

From: Dilip Kota
Date: Tue Nov 19 2019 - 04:20:05 EST



On 11/19/2019 11:54 AM, Jingoo Han wrote:

ïOn 11/18/19, 10:15 PM, Dilip Kota wrote:
On 11/19/2019 10:33 AM, Jingoo Han wrote:
On 11/18/19, 8:26 PM, Dilip Kota wrote:
On 11/19/2019 12:40 AM, Jingoo Han wrote:
ïOn 11/18/19, 2:58 AM, Dilip Kota wrote:

On 11/16/2019 4:40 AM, Jingoo Han wrote:
On 11/14/19, 9:31 PM, Dilip Kota wrote:

Add support to PCIe RC controller on Intel Gateway SoCs.
PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training
Sequence and link speed configurations. 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.

Signed-off-by: Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>
Reviewed-by: Andrew Murray <andrew.murray@xxxxxxx>
Acked-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
---
[.....]

drivers/pci/controller/dwc/Kconfig | 10 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-designware.c | 57 +++
drivers/pci/controller/dwc/pcie-designware.h | 12 +
drivers/pci/controller/dwc/pcie-intel-gw.c | 542 +++++++++++++++++++++++++++
include/uapi/linux/pci_regs.h | 1 +
6 files changed, 623 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 0ba988b5b5bc..fb6d474477df 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
order to enable device-specific features PCI_DW_PLAT_EP must be
selected.
+config PCIE_INTEL_GW
+ bool "Intel Gateway PCIe host controller support"
+ depends on OF && (X86 || COMPILE_TEST)
+ select PCIE_DW_HOST
+ help
+ Say 'Y' here to enable PCIe Host controller support on Intel
+ Gateway SoCs.
+ The PCIe controller uses the DesignWare core plus Intel-specific
+ hardware wrappers.
+
Please add this config alphabetical order!
So, this config should be after 'config PCI_IMX6'.
There is no reason to put this config at the first place.

config PCI_EXYNOS
bool "Samsung Exynos PCIe controller"
depends on SOC_EXYNOS5440 || COMPILE_TEST
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index b30336181d46..99db83cd2f35 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
+obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
Ditto.
PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
Hey, although some of them are not in order, you don't have a right to do so.
If some people do not follow the law, it does not mean that you can break the law.
Anyway, if you don't follow an alphabetical order, my answer is NACK.
Also, other people or I will send a patch to fix the order of other drivers.
I am not against following the order. I kept PCIE_INTEL_GW after
PCIE_DW* by checking the best possible order.
As per the alphabetical order, i see all CONFIG_PCIE_* comes first and
CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW
after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after
PCI_IMX6).
Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW
comes after PCIE_DW_PLAT( and PCIE_HISI_STB).
Are you kidding me?

Most PCIE_* drivers are located after PCI_*. Look at PCIE_QCOM, PCIE_ARMADA_8K,
PCIE_ARTPEC6, PCIE_KIRIN, PCIE_HISI_STB, PCIE_TEGRA194, PCIE_UNIPHIER, PCIE_AL.
Ok, So the understanding is PCIE_DW* will be at top as they are
framework and then comes CONFIG_PCI_*, CONFIG_PCIE_*.
Yes, right.
Ah, I will explain the order of Kconfig options in the dwc directory.
The order is PCIE_DW, PCI_*, and PCIE_* as below.

1. Common Frameworks:
These options are used by other controller drivers.
e.g., PCIE_DW, PCIE_DW_HOST, PCIE_DW_EP.

2. PCI_* controller drivers:
PCI_* was used earlier than PCIE_*. If a chip vendor's controllers provide
both conventional PCI and PCI Express, or only conventional PCI, PCI_* can
be used. For example, the reason PCI_EXYNOS was chosen instead of
PCIE_EXYNOS, is that EXYNOS had controllers to support either conventional
PCI or PCI Express.

3. PCIE_* controller drivers
If a controller can support only PCI Express, not conventional PCI,
PCIE_* is the proper naming.

Then, within PCI_* or PCIE_* categories, each controller option should be
in an alphabetical order for the readability. Also, other person or I will
send a patch to fix orders. (e.g., PCIE_QCOM, PCIE_AL, etc.)

Thanks Jingoo Han for explaining in detail.

Regards,
Dilip

Put PCIE_INTEL_GW between PCIE_ARTPEC6_EP and PCIE_KIRIN.
Ok. I will update in the next patch version.
Ok, thank you.

Best regards,
Jingoo Han