linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
@ 2021-09-22  5:00 Sergio Paracuellos
  2021-09-22  5:00 ` [PATCH v3 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs Sergio Paracuellos
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-09-22  5:00 UTC (permalink / raw)
  To: linux-pci
  Cc: lorenzo.pieralisi, john, devicetree, tsbogend, bhelgaas,
	matthias.bgg, gregkh, linux-mips, linux-staging, neil,
	linux-kernel

MediaTek MT7621 PCIe subsys supports single Root complex (RC)
with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link.
Topology is as follows:


                          MT7621 PCIe HOST Topology

                                   .-------.
                                   |       |
                                   |  CPU  |
                                   |       |
                                   '-------'
                                       |
                                       |
                                       |
                                       v
                              .------------------.
                  .-----------|  HOST/PCI Bridge |------------.
                  |           '------------------'            |     Type1 
         BUS0     |                     |                     |    Access 
                  v                     v                     v    On Bus0
          .-------------.        .-------------.       .-------------.
          | VIRTUAL P2P |        | VIRTUAL P2P |       | VIRTUAL P2P |
          |    BUS0     |        |    BUS0     |       |    BUS0     |
          |    DEV0     |        |    DEV1     |       |    DEV2     |
          '-------------'        '-------------'       '-------------'
    Type0        |          Type0       |         Type0       |
   Access   BUS1 |         Access   BUS2|        Access   BUS3|
   On Bus1       v         On Bus2      v        On Bus3      v
           .----------.           .----------.          .----------.
           | Device 0 |           | Device 0 |          | Device 0 |
           |  Func 0  |           |  Func 0  |          |  Func 0  |
           '----------'           '----------'          '----------'

This driver has been very long time in staging and I have been cleaning
it from its first versions where there was code kaos and PCI_LEGACY support.
Original code came probably from openWRT based on mediatek's SDK code. There
is no documentation at all about the mt7621 PCI subsystem.
I have been cleaning it targeting mt7621 SoC which is the one I use in
my GNUBee PC1 board and HiLink HLK-MT7621A evaluation board.

Now I think is clean enough to be moved into 'drivers/pci/controller'.
This driver is mips/ralink architecture and need 'mips_cps_numiocu()'
to properly configure iocu regions for mips.

This driver also uses already mainlined pci phy driver located in
'drivers/phy/ralink/phy-mt7621-pci.c'. There are two instances of
the phy being the first one dual ported for pci0 and pci1, and the
second one not dual ported dedicated to pci2. Because of writing twice
some phy registers of the dual-ported one sometimes become in not
confident boot cycles we have to take care of this when device link
is checked here in controller driver. We power on the dual ported-phy
if there is something connected in pcie0 or pcie1. In the same manner
we have to properly disable it only if nothing is connected in of both
pcie0 and pcie1 slots.

This changes are rebased on the top of staging-next branch of staging
tree for a clean git mv since last changes are always in that tree.
Since this a git mv as I was told to do, include link to the last code
here [0].

Changes in v3:
 - Add Rob's Reviewed-by for the bindings.
 - Avoid custom 'of_pci_range_to_resource' in driver side since
   PCI core APIs has been changed to properly support this architecture.
 - Kconfig:
   - Change from 'bool' to 'tristate'.
   - Add phy's selection 'select PHY_MT7621_PCI'.
   - Move PCI_DRIVERS_GENERIC selection to 'arch/mips' since its mips
     internal stuff (change requested by Lorenzo in v2 review of this series).

Changes in v2:
 - Make one commit moving driver directly from staging into
  'drivers/pci/controllers' instead of two commits making
  one add and a later remove.
 - Update binding documentation moving 'clocks', 'resets' and
   'phys' properties to child root bridge nodes. 
 - Update code to properly be able to use new bindings.
 - Kconfig: add || (MIPS && COMPILE_TEST).
 - Use {read/write}_relaxed versions.
 - Use 'PCI_BASE_ADDRESS_0' instead of a custom definition.
 - Avoid to set 'PCI_COMMAND_MASTER' and re-do functions
   'mt7621_pcie_enable_ports' and 'mt7621_pcie_enable_port'.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos

[0]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/mt7621-pci/pci-mt7621.c?h=staging-testing
Sergio Paracuellos (3):
  dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
  PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  MAINTAINERS: add myself as maintainer of the MT7621 PCI controller
    driver

 .../bindings/pci/mediatek,mt7621-pci.yaml     | 142 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 arch/mips/ralink/Kconfig                      |   3 +-
 drivers/pci/controller/Kconfig                |   8 +
 drivers/pci/controller/Makefile               |   1 +
 .../controller}/pci-mt7621.c                  |   0
 drivers/staging/Kconfig                       |   2 -
 drivers/staging/Makefile                      |   1 -
 drivers/staging/mt7621-pci/Kconfig            |   8 -
 drivers/staging/mt7621-pci/Makefile           |   2 -
 drivers/staging/mt7621-pci/TODO               |   4 -
 .../mt7621-pci/mediatek,mt7621-pci.txt        | 104 -------------
 12 files changed, 159 insertions(+), 122 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
 rename drivers/{staging/mt7621-pci => pci/controller}/pci-mt7621.c (100%)
 delete mode 100644 drivers/staging/mt7621-pci/Kconfig
 delete mode 100644 drivers/staging/mt7621-pci/Makefile
 delete mode 100644 drivers/staging/mt7621-pci/TODO
 delete mode 100644 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt

-- 
2.25.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
  2021-09-22  5:00 [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
@ 2021-09-22  5:00 ` Sergio Paracuellos
  2021-09-22  5:00 ` [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-09-22  5:00 UTC (permalink / raw)
  To: linux-pci
  Cc: lorenzo.pieralisi, john, devicetree, tsbogend, bhelgaas,
	matthias.bgg, gregkh, linux-mips, linux-staging, neil,
	linux-kernel, Rob Herring

Add device tree binding documentation for PCIe in MT7621 SoCs.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/pci/mediatek,mt7621-pci.yaml     | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
new file mode 100644
index 000000000000..d7359d0ee935
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
@@ -0,0 +1,142 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/mediatek,mt7621-pci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT7621 PCIe controller
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |+
+  MediaTek MT7621 PCIe subsys supports single Root complex (RC)
+  with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    const: mediatek,mt7621-pci
+
+  reg:
+    items:
+      - description: host-pci bridge registers
+      - description: pcie port 0 RC control registers
+      - description: pcie port 1 RC control registers
+      - description: pcie port 2 RC control registers
+
+  ranges:
+    maxItems: 2
+
+patternProperties:
+  'pcie@[0-2],0':
+    type: object
+    $ref: /schemas/pci/pci-bus.yaml#
+
+    properties:
+      resets:
+        maxItems: 1
+
+      clocks:
+        maxItems: 1
+
+      phys:
+        maxItems: 1
+
+    required:
+      - "#interrupt-cells"
+      - interrupt-map-mask
+      - interrupt-map
+      - resets
+      - clocks
+      - phys
+      - phy-names
+      - ranges
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+
+    pcie: pcie@1e140000 {
+        compatible = "mediatek,mt7621-pci";
+        reg = <0x1e140000 0x100>,
+              <0x1e142000 0x100>,
+              <0x1e143000 0x100>,
+              <0x1e144000 0x100>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pcie_pins>;
+        device_type = "pci";
+        ranges = <0x02000000 0 0x60000000 0x60000000 0 0x10000000>,  /* pci memory */
+                 <0x01000000 0 0x1e160000 0x1e160000 0 0x00010000>;  /* io space */
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0xF800 0 0 0>;
+        interrupt-map = <0x0000 0 0 0 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0800 0 0 0 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x1000 0 0 0 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
+        reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>;
+
+        pcie@0,0 {
+            reg = <0x0000 0 0 0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            device_type = "pci";
+            #interrupt-cells = <1>;
+            interrupt-map-mask = <0 0 0 0>;
+            interrupt-map = <0 0 0 0 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>;
+            resets = <&rstctrl 24>;
+            clocks = <&clkctrl 24>;
+            phys = <&pcie0_phy 1>;
+            phy-names = "pcie-phy0";
+            ranges;
+        };
+
+        pcie@1,0 {
+            reg = <0x0800 0 0 0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            device_type = "pci";
+            #interrupt-cells = <1>;
+            interrupt-map-mask = <0 0 0 0>;
+            interrupt-map = <0 0 0 0 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>;
+            resets = <&rstctrl 25>;
+            clocks = <&clkctrl 25>;
+            phys = <&pcie0_phy 1>;
+            phy-names = "pcie-phy1";
+            ranges;
+        };
+
+        pcie@2,0 {
+            reg = <0x1000 0 0 0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            device_type = "pci";
+            #interrupt-cells = <1>;
+            interrupt-map-mask = <0 0 0 0>;
+            interrupt-map = <0 0 0 0 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
+            resets = <&rstctrl 26>;
+            clocks = <&clkctrl 26>;
+            phys = <&pcie2_phy 0>;
+            phy-names = "pcie-phy2";
+            ranges;
+        };
+    };
+...
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-09-22  5:00 [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
  2021-09-22  5:00 ` [PATCH v3 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs Sergio Paracuellos
@ 2021-09-22  5:00 ` Sergio Paracuellos
  2021-10-13 13:05   ` Lorenzo Pieralisi
  2021-09-22  5:00 ` [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI " Sergio Paracuellos
  2021-10-20 14:23 ` [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host " Lorenzo Pieralisi
  3 siblings, 1 reply; 18+ messages in thread
From: Sergio Paracuellos @ 2021-09-22  5:00 UTC (permalink / raw)
  To: linux-pci
  Cc: lorenzo.pieralisi, john, devicetree, tsbogend, bhelgaas,
	matthias.bgg, gregkh, linux-mips, linux-staging, neil,
	linux-kernel

Add driver for the PCIe controller of the MT7621 SoC.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/ralink/Kconfig                      |   3 +-
 drivers/pci/controller/Kconfig                |   8 ++
 drivers/pci/controller/Makefile               |   1 +
 .../controller}/pci-mt7621.c                  |   0
 drivers/staging/Kconfig                       |   2 -
 drivers/staging/Makefile                      |   1 -
 drivers/staging/mt7621-pci/Kconfig            |   8 --
 drivers/staging/mt7621-pci/Makefile           |   2 -
 drivers/staging/mt7621-pci/TODO               |   4 -
 .../mt7621-pci/mediatek,mt7621-pci.txt        | 104 ------------------
 10 files changed, 11 insertions(+), 122 deletions(-)
 rename drivers/{staging/mt7621-pci => pci/controller}/pci-mt7621.c (100%)
 delete mode 100644 drivers/staging/mt7621-pci/Kconfig
 delete mode 100644 drivers/staging/mt7621-pci/Makefile
 delete mode 100644 drivers/staging/mt7621-pci/TODO
 delete mode 100644 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt

diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
index c800bf5559b5..120adad51d6a 100644
--- a/arch/mips/ralink/Kconfig
+++ b/arch/mips/ralink/Kconfig
@@ -51,7 +51,8 @@ choice
 		select SYS_SUPPORTS_HIGHMEM
 		select MIPS_GIC
 		select CLKSRC_MIPS_GIC
-		select HAVE_PCI if PCI_MT7621
+		select HAVE_PCI
+		select PCI_DRIVERS_GENERIC
 		select SOC_BUS
 endchoice
 
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 326f7d13024f..b76404be0360 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -312,6 +312,14 @@ config PCIE_HISI_ERR
 	  Say Y here if you want error handling support
 	  for the PCIe controller's errors on HiSilicon HIP SoCs
 
+config PCI_MT7621
+	tristate "MediaTek MT7621 PCI Controller"
+	depends on (RALINK && SOC_MT7621) || (MIPS && COMPILE_TEST)
+	select PHY_MT7621_PCI
+	default SOC_MT7621
+	help
+	  This selects a driver for the MediaTek MT7621 PCI Controller.
+
 source "drivers/pci/controller/dwc/Kconfig"
 source "drivers/pci/controller/mobiveil/Kconfig"
 source "drivers/pci/controller/cadence/Kconfig"
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index aaf30b3dcc14..f42a566353cb 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_VMD) += vmd.o
 obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
 obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
 obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
+obj-$(CONFIG_PCI_MT7621) += pci-mt7621.o
 # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
 obj-y				+= dwc/
 obj-y				+= mobiveil/
diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/pci/controller/pci-mt7621.c
similarity index 100%
rename from drivers/staging/mt7621-pci/pci-mt7621.c
rename to drivers/pci/controller/pci-mt7621.c
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index e03627ad4460..59af251e7576 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -86,8 +86,6 @@ source "drivers/staging/vc04_services/Kconfig"
 
 source "drivers/staging/pi433/Kconfig"
 
-source "drivers/staging/mt7621-pci/Kconfig"
-
 source "drivers/staging/mt7621-dma/Kconfig"
 
 source "drivers/staging/ralink-gdma/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index c7f8d8d8dd11..76f413470bc8 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -33,7 +33,6 @@ obj-$(CONFIG_KS7010)		+= ks7010/
 obj-$(CONFIG_GREYBUS)		+= greybus/
 obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
 obj-$(CONFIG_PI433)		+= pi433/
-obj-$(CONFIG_PCI_MT7621)	+= mt7621-pci/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
 obj-$(CONFIG_DMA_RALINK)	+= ralink-gdma/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dts/
diff --git a/drivers/staging/mt7621-pci/Kconfig b/drivers/staging/mt7621-pci/Kconfig
deleted file mode 100644
index ce58042f2f21..000000000000
--- a/drivers/staging/mt7621-pci/Kconfig
+++ /dev/null
@@ -1,8 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-config PCI_MT7621
-	tristate "MediaTek MT7621 PCI Controller"
-	depends on RALINK
-	select PCI_DRIVERS_GENERIC
-	help
-	  This selects a driver for the MediaTek MT7621 PCI Controller.
-
diff --git a/drivers/staging/mt7621-pci/Makefile b/drivers/staging/mt7621-pci/Makefile
deleted file mode 100644
index f4e651cf7ce3..000000000000
--- a/drivers/staging/mt7621-pci/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_PCI_MT7621)       += pci-mt7621.o
diff --git a/drivers/staging/mt7621-pci/TODO b/drivers/staging/mt7621-pci/TODO
deleted file mode 100644
index d674a9ac85c1..000000000000
--- a/drivers/staging/mt7621-pci/TODO
+++ /dev/null
@@ -1,4 +0,0 @@
-
-- general code review and cleanup
-
-Cc: NeilBrown <neil@brown.name>
diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
deleted file mode 100644
index 327a68267309..000000000000
--- a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
+++ /dev/null
@@ -1,104 +0,0 @@
-MediaTek MT7621 PCIe controller
-
-Required properties:
-- compatible: "mediatek,mt7621-pci"
-- device_type: Must be "pci"
-- reg: Base addresses and lengths of the PCIe subsys and root ports.
-- bus-range: Range of bus numbers associated with this controller.
-- #address-cells: Address representation for root ports (must be 3)
-- pinctrl-names : The pin control state names.
-- pinctrl-0: The "default" pinctrl state.
-- #size-cells: Size representation for root ports (must be 2)
-- ranges: Ranges for the PCI memory and I/O regions.
-- #interrupt-cells: Must be 1
-- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties.
-  Please refer to the standard PCI bus binding document for a more detailed
-  explanation.
-- status: either "disabled" or "okay".
-- resets: Must contain an entry for each entry in reset-names.
-  See ../reset/reset.txt for details.
-- reset-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
-  root ports.
-- clocks: Must contain an entry for each entry in clock-names.
-  See ../clocks/clock-bindings.txt for details.
-- clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
-  root ports.
-- reset-gpios: GPIO specs for the reset pins.
-
-In addition, the device tree node must have sub-nodes describing each PCIe port
-interface, having the following mandatory properties:
-
-Required properties:
-- reg: Only the first four bytes are used to refer to the correct bus number
-      and device number.
-- #address-cells: Must be 3
-- #size-cells: Must be 2
-- ranges: Sub-ranges distributed from the PCIe controller node. An empty
-  property is sufficient.
-- bus-range: Range of bus numbers associated with this port.
-
-Example for MT7621:
-
-	pcie: pcie@1e140000 {
-		compatible = "mediatek,mt7621-pci";
-        reg = <0x1e140000 0x100    /* host-pci bridge registers */
-               0x1e142000 0x100    /* pcie port 0 RC control registers */
-               0x1e143000 0x100    /* pcie port 1 RC control registers */
-               0x1e144000 0x100>;  /* pcie port 2 RC control registers */
-
-		#address-cells = <3>;
-		#size-cells = <2>;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&pcie_pins>;
-
-		device_type = "pci";
-
-		bus-range = <0 255>;
-		ranges = <
-			0x02000000 0 0x00000000 0x60000000 0 0x10000000 /* pci memory */
-			0x01000000 0 0x00000000 0x1e160000 0 0x00010000 /* io space */
-		>;
-
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0xF0000 0 0 1>;
-		interrupt-map = <0x10000 0 0 1 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>,
-				<0x20000 0 0 1 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>,
-				<0x30000 0 0 1 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
-
-		status = "disabled";
-
-		resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
-		reset-names = "pcie0", "pcie1", "pcie2";
-		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
-		clock-names = "pcie0", "pcie1", "pcie2";
-
-		reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
-				<&gpio 8 GPIO_ACTIVE_LOW>,
-				<&gpio 7 GPIO_ACTIVE_LOW>;
-
-		pcie@0,0 {
-			reg = <0x0000 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-			ranges;
-			bus-range = <0x00 0xff>;
-		};
-
-		pcie@1,0 {
-			reg = <0x0800 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-			ranges;
-			bus-range = <0x00 0xff>;
-		};
-
-		pcie@2,0 {
-			reg = <0x1000 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-			ranges;
-			bus-range = <0x00 0xff>;
-		};
-	};
-
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
  2021-09-22  5:00 [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
  2021-09-22  5:00 ` [PATCH v3 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs Sergio Paracuellos
  2021-09-22  5:00 ` [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
@ 2021-09-22  5:00 ` Sergio Paracuellos
  2021-09-22  9:05   ` Sergei Shtylyov
  2021-10-20 14:23 ` [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host " Lorenzo Pieralisi
  3 siblings, 1 reply; 18+ messages in thread
From: Sergio Paracuellos @ 2021-09-22  5:00 UTC (permalink / raw)
  To: linux-pci
  Cc: lorenzo.pieralisi, john, devicetree, tsbogend, bhelgaas,
	matthias.bgg, gregkh, linux-mips, linux-staging, neil,
	linux-kernel

Add myself as maintainer of the PCie Controlller driver for
MT7621 SoCs.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eeb4c70b3d5b..a7f0cbad7ac5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11842,6 +11842,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i2c/i2c-mt7621.txt
 F:	drivers/i2c/busses/i2c-mt7621.c
 
+MEDIATEK MT7621 PCI CONTROLLER DRIVER
+M:	Sergio Paracuellos <sergio.paracuellos@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
+F:	drivers/pci/controller/pci-mt7621.c
+
 MEDIATEK MT7621 PHY PCI DRIVER
 M:	Sergio Paracuellos <sergio.paracuellos@gmail.com>
 S:	Maintained
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
  2021-09-22  5:00 ` [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI " Sergio Paracuellos
@ 2021-09-22  9:05   ` Sergei Shtylyov
  2021-10-03 16:55     ` Sergio Paracuellos
  0 siblings, 1 reply; 18+ messages in thread
From: Sergei Shtylyov @ 2021-09-22  9:05 UTC (permalink / raw)
  To: Sergio Paracuellos, linux-pci
  Cc: lorenzo.pieralisi, john, devicetree, tsbogend, bhelgaas,
	matthias.bgg, gregkh, linux-mips, linux-staging, neil,
	linux-kernel

On 22.09.2021 8:00, Sergio Paracuellos wrote:

> Add myself as maintainer of the PCie Controlller driver for

     PCIe Controller. :-)

> MT7621 SoCs.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
[...]

MBR, Sergei


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
  2021-09-22  9:05   ` Sergei Shtylyov
@ 2021-10-03 16:55     ` Sergio Paracuellos
  0 siblings, 0 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-03 16:55 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-pci, Lorenzo Pieralisi, John Crispin,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Bjorn Helgaas, Matthias Brugger, Greg KH,
	open list:MIPS, linux-staging, NeilBrown, linux-kernel

Hi Sergei,

On Wed, Sep 22, 2021 at 11:05 AM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
>
> On 22.09.2021 8:00, Sergio Paracuellos wrote:
>
> > Add myself as maintainer of the PCie Controlller driver for
>
>      PCIe Controller. :-)

Ups, true :) I will send v4 with this fixed if more changes are needed.

Best regards,
    Sergio Paracuellos

>
> > MT7621 SoCs.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> [...]
>
> MBR, Sergei
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-09-22  5:00 ` [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
@ 2021-10-13 13:05   ` Lorenzo Pieralisi
  2021-10-13 13:35     ` Sergio Paracuellos
  0 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2021-10-13 13:05 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-pci, john, devicetree, tsbogend, bhelgaas, matthias.bgg,
	gregkh, linux-mips, linux-staging, neil, linux-kernel

On Wed, Sep 22, 2021 at 07:00:34AM +0200, Sergio Paracuellos wrote:
> Add driver for the PCIe controller of the MT7621 SoC.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  arch/mips/ralink/Kconfig                      |   3 +-
>  drivers/pci/controller/Kconfig                |   8 ++
>  drivers/pci/controller/Makefile               |   1 +
>  .../controller}/pci-mt7621.c                  |   0
>  drivers/staging/Kconfig                       |   2 -
>  drivers/staging/Makefile                      |   1 -
>  drivers/staging/mt7621-pci/Kconfig            |   8 --
>  drivers/staging/mt7621-pci/Makefile           |   2 -
>  drivers/staging/mt7621-pci/TODO               |   4 -
>  .../mt7621-pci/mediatek,mt7621-pci.txt        | 104 ------------------
>  10 files changed, 11 insertions(+), 122 deletions(-)
>  rename drivers/{staging/mt7621-pci => pci/controller}/pci-mt7621.c (100%)
>  delete mode 100644 drivers/staging/mt7621-pci/Kconfig
>  delete mode 100644 drivers/staging/mt7621-pci/Makefile
>  delete mode 100644 drivers/staging/mt7621-pci/TODO
>  delete mode 100644 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> 
> diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
> index c800bf5559b5..120adad51d6a 100644
> --- a/arch/mips/ralink/Kconfig
> +++ b/arch/mips/ralink/Kconfig
> @@ -51,7 +51,8 @@ choice
>  		select SYS_SUPPORTS_HIGHMEM
>  		select MIPS_GIC
>  		select CLKSRC_MIPS_GIC
> -		select HAVE_PCI if PCI_MT7621
> +		select HAVE_PCI
> +		select PCI_DRIVERS_GENERIC
>  		select SOC_BUS
>  endchoice
>  
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 326f7d13024f..b76404be0360 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -312,6 +312,14 @@ config PCIE_HISI_ERR
>  	  Say Y here if you want error handling support
>  	  for the PCIe controller's errors on HiSilicon HIP SoCs
>  
> +config PCI_MT7621
> +	tristate "MediaTek MT7621 PCI Controller"
> +	depends on (RALINK && SOC_MT7621) || (MIPS && COMPILE_TEST)

- Is there a chance we can remove the MIPS dependency from the
  COMPILE_TEST conditional ?
- I am not a big fan of "SOC_XXX" config options dependencies, actually
  there is none in pci/controller. Is there a way to remove it ?

Lorenzo

> +	select PHY_MT7621_PCI
> +	default SOC_MT7621
> +	help
> +	  This selects a driver for the MediaTek MT7621 PCI Controller.
> +
>  source "drivers/pci/controller/dwc/Kconfig"
>  source "drivers/pci/controller/mobiveil/Kconfig"
>  source "drivers/pci/controller/cadence/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index aaf30b3dcc14..f42a566353cb 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_VMD) += vmd.o
>  obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
>  obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
>  obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> +obj-$(CONFIG_PCI_MT7621) += pci-mt7621.o
>  # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>  obj-y				+= dwc/
>  obj-y				+= mobiveil/
> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/pci/controller/pci-mt7621.c
> similarity index 100%
> rename from drivers/staging/mt7621-pci/pci-mt7621.c
> rename to drivers/pci/controller/pci-mt7621.c
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index e03627ad4460..59af251e7576 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -86,8 +86,6 @@ source "drivers/staging/vc04_services/Kconfig"
>  
>  source "drivers/staging/pi433/Kconfig"
>  
> -source "drivers/staging/mt7621-pci/Kconfig"
> -
>  source "drivers/staging/mt7621-dma/Kconfig"
>  
>  source "drivers/staging/ralink-gdma/Kconfig"
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index c7f8d8d8dd11..76f413470bc8 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -33,7 +33,6 @@ obj-$(CONFIG_KS7010)		+= ks7010/
>  obj-$(CONFIG_GREYBUS)		+= greybus/
>  obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
>  obj-$(CONFIG_PI433)		+= pi433/
> -obj-$(CONFIG_PCI_MT7621)	+= mt7621-pci/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
>  obj-$(CONFIG_DMA_RALINK)	+= ralink-gdma/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-dts/
> diff --git a/drivers/staging/mt7621-pci/Kconfig b/drivers/staging/mt7621-pci/Kconfig
> deleted file mode 100644
> index ce58042f2f21..000000000000
> --- a/drivers/staging/mt7621-pci/Kconfig
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0
> -config PCI_MT7621
> -	tristate "MediaTek MT7621 PCI Controller"
> -	depends on RALINK
> -	select PCI_DRIVERS_GENERIC
> -	help
> -	  This selects a driver for the MediaTek MT7621 PCI Controller.
> -
> diff --git a/drivers/staging/mt7621-pci/Makefile b/drivers/staging/mt7621-pci/Makefile
> deleted file mode 100644
> index f4e651cf7ce3..000000000000
> --- a/drivers/staging/mt7621-pci/Makefile
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_PCI_MT7621)       += pci-mt7621.o
> diff --git a/drivers/staging/mt7621-pci/TODO b/drivers/staging/mt7621-pci/TODO
> deleted file mode 100644
> index d674a9ac85c1..000000000000
> --- a/drivers/staging/mt7621-pci/TODO
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -
> -- general code review and cleanup
> -
> -Cc: NeilBrown <neil@brown.name>
> diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> deleted file mode 100644
> index 327a68267309..000000000000
> --- a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> +++ /dev/null
> @@ -1,104 +0,0 @@
> -MediaTek MT7621 PCIe controller
> -
> -Required properties:
> -- compatible: "mediatek,mt7621-pci"
> -- device_type: Must be "pci"
> -- reg: Base addresses and lengths of the PCIe subsys and root ports.
> -- bus-range: Range of bus numbers associated with this controller.
> -- #address-cells: Address representation for root ports (must be 3)
> -- pinctrl-names : The pin control state names.
> -- pinctrl-0: The "default" pinctrl state.
> -- #size-cells: Size representation for root ports (must be 2)
> -- ranges: Ranges for the PCI memory and I/O regions.
> -- #interrupt-cells: Must be 1
> -- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties.
> -  Please refer to the standard PCI bus binding document for a more detailed
> -  explanation.
> -- status: either "disabled" or "okay".
> -- resets: Must contain an entry for each entry in reset-names.
> -  See ../reset/reset.txt for details.
> -- reset-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
> -  root ports.
> -- clocks: Must contain an entry for each entry in clock-names.
> -  See ../clocks/clock-bindings.txt for details.
> -- clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
> -  root ports.
> -- reset-gpios: GPIO specs for the reset pins.
> -
> -In addition, the device tree node must have sub-nodes describing each PCIe port
> -interface, having the following mandatory properties:
> -
> -Required properties:
> -- reg: Only the first four bytes are used to refer to the correct bus number
> -      and device number.
> -- #address-cells: Must be 3
> -- #size-cells: Must be 2
> -- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> -  property is sufficient.
> -- bus-range: Range of bus numbers associated with this port.
> -
> -Example for MT7621:
> -
> -	pcie: pcie@1e140000 {
> -		compatible = "mediatek,mt7621-pci";
> -        reg = <0x1e140000 0x100    /* host-pci bridge registers */
> -               0x1e142000 0x100    /* pcie port 0 RC control registers */
> -               0x1e143000 0x100    /* pcie port 1 RC control registers */
> -               0x1e144000 0x100>;  /* pcie port 2 RC control registers */
> -
> -		#address-cells = <3>;
> -		#size-cells = <2>;
> -
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&pcie_pins>;
> -
> -		device_type = "pci";
> -
> -		bus-range = <0 255>;
> -		ranges = <
> -			0x02000000 0 0x00000000 0x60000000 0 0x10000000 /* pci memory */
> -			0x01000000 0 0x00000000 0x1e160000 0 0x00010000 /* io space */
> -		>;
> -
> -		#interrupt-cells = <1>;
> -		interrupt-map-mask = <0xF0000 0 0 1>;
> -		interrupt-map = <0x10000 0 0 1 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>,
> -				<0x20000 0 0 1 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>,
> -				<0x30000 0 0 1 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
> -
> -		status = "disabled";
> -
> -		resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
> -		reset-names = "pcie0", "pcie1", "pcie2";
> -		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> -		clock-names = "pcie0", "pcie1", "pcie2";
> -
> -		reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
> -				<&gpio 8 GPIO_ACTIVE_LOW>,
> -				<&gpio 7 GPIO_ACTIVE_LOW>;
> -
> -		pcie@0,0 {
> -			reg = <0x0000 0 0 0 0>;
> -			#address-cells = <3>;
> -			#size-cells = <2>;
> -			ranges;
> -			bus-range = <0x00 0xff>;
> -		};
> -
> -		pcie@1,0 {
> -			reg = <0x0800 0 0 0 0>;
> -			#address-cells = <3>;
> -			#size-cells = <2>;
> -			ranges;
> -			bus-range = <0x00 0xff>;
> -		};
> -
> -		pcie@2,0 {
> -			reg = <0x1000 0 0 0 0>;
> -			#address-cells = <3>;
> -			#size-cells = <2>;
> -			ranges;
> -			bus-range = <0x00 0xff>;
> -		};
> -	};
> -
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-13 13:05   ` Lorenzo Pieralisi
@ 2021-10-13 13:35     ` Sergio Paracuellos
  0 siblings, 0 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-13 13:35 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-pci, John Crispin,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Bjorn Helgaas, Matthias Brugger, Greg KH,
	open list:MIPS, linux-staging, NeilBrown, linux-kernel

Hi Lorenzo,

Thanks for the review. See my comments below.

On Wed, Oct 13, 2021 at 3:05 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Wed, Sep 22, 2021 at 07:00:34AM +0200, Sergio Paracuellos wrote:
> > Add driver for the PCIe controller of the MT7621 SoC.
> >
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  arch/mips/ralink/Kconfig                      |   3 +-
> >  drivers/pci/controller/Kconfig                |   8 ++
> >  drivers/pci/controller/Makefile               |   1 +
> >  .../controller}/pci-mt7621.c                  |   0
> >  drivers/staging/Kconfig                       |   2 -
> >  drivers/staging/Makefile                      |   1 -
> >  drivers/staging/mt7621-pci/Kconfig            |   8 --
> >  drivers/staging/mt7621-pci/Makefile           |   2 -
> >  drivers/staging/mt7621-pci/TODO               |   4 -
> >  .../mt7621-pci/mediatek,mt7621-pci.txt        | 104 ------------------
> >  10 files changed, 11 insertions(+), 122 deletions(-)
> >  rename drivers/{staging/mt7621-pci => pci/controller}/pci-mt7621.c (100%)
> >  delete mode 100644 drivers/staging/mt7621-pci/Kconfig
> >  delete mode 100644 drivers/staging/mt7621-pci/Makefile
> >  delete mode 100644 drivers/staging/mt7621-pci/TODO
> >  delete mode 100644 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> >
> > diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
> > index c800bf5559b5..120adad51d6a 100644
> > --- a/arch/mips/ralink/Kconfig
> > +++ b/arch/mips/ralink/Kconfig
> > @@ -51,7 +51,8 @@ choice
> >               select SYS_SUPPORTS_HIGHMEM
> >               select MIPS_GIC
> >               select CLKSRC_MIPS_GIC
> > -             select HAVE_PCI if PCI_MT7621
> > +             select HAVE_PCI
> > +             select PCI_DRIVERS_GENERIC
> >               select SOC_BUS
> >  endchoice
> >
> > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > index 326f7d13024f..b76404be0360 100644
> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -312,6 +312,14 @@ config PCIE_HISI_ERR
> >         Say Y here if you want error handling support
> >         for the PCIe controller's errors on HiSilicon HIP SoCs
> >
> > +config PCI_MT7621
> > +     tristate "MediaTek MT7621 PCI Controller"
> > +     depends on (RALINK && SOC_MT7621) || (MIPS && COMPILE_TEST)
>
> - Is there a chance we can remove the MIPS dependency from the
>   COMPILE_TEST conditional ?

Driver make use of the following functions to properly configure MIPS
IO coherency regions for used pci addresses:
- 'mips_cps_numiocu()'
- 'write_gcr_reg1_base()'
- 'write_gcr_reg1_mask()'

Without configuring this, the PCI subsystem won't work.
These three are a MIPS thing and we want at the very least to make
COMPILE_TEST available for MIPS. To avoid this I guess we will need
stubs for all the other architectures and I am not really sure it is
really worthly and makes sense.

> - I am not a big fan of "SOC_XXX" config options dependencies, actually
>   there is none in pci/controller. Is there a way to remove it ?

I am not a Kconfig expert, so I am not sure :). This PCI driver needs
only to be available for MIPS RALINK architecture for MT7621 SoCs and
ideally enabled by default for SOC_MT7621. So I don't know if just
doing the following is enough:

config PCI_MT7621
    tristate "MediaTek MT7621 PCI Controller"
    depends on RALINK || (MIPS && COMPILE_TEST)
    select PHY_MT7621_PCI
    default SOC_MT7621
    help
        This selects a driver for the MediaTek MT7621 PCI Controller.

Thanks in advance for clarification.

Best regards,
    Sergio Paracuellos

>
> Lorenzo
>
> > +     select PHY_MT7621_PCI
> > +     default SOC_MT7621
> > +     help
> > +       This selects a driver for the MediaTek MT7621 PCI Controller.
> > +
> >  source "drivers/pci/controller/dwc/Kconfig"
> >  source "drivers/pci/controller/mobiveil/Kconfig"
> >  source "drivers/pci/controller/cadence/Kconfig"
> > diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> > index aaf30b3dcc14..f42a566353cb 100644
> > --- a/drivers/pci/controller/Makefile
> > +++ b/drivers/pci/controller/Makefile
> > @@ -37,6 +37,7 @@ obj-$(CONFIG_VMD) += vmd.o
> >  obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
> >  obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> >  obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> > +obj-$(CONFIG_PCI_MT7621) += pci-mt7621.o
> >  # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> >  obj-y                                += dwc/
> >  obj-y                                += mobiveil/
> > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/pci/controller/pci-mt7621.c
> > similarity index 100%
> > rename from drivers/staging/mt7621-pci/pci-mt7621.c
> > rename to drivers/pci/controller/pci-mt7621.c
> > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> > index e03627ad4460..59af251e7576 100644
> > --- a/drivers/staging/Kconfig
> > +++ b/drivers/staging/Kconfig
> > @@ -86,8 +86,6 @@ source "drivers/staging/vc04_services/Kconfig"
> >
> >  source "drivers/staging/pi433/Kconfig"
> >
> > -source "drivers/staging/mt7621-pci/Kconfig"
> > -
> >  source "drivers/staging/mt7621-dma/Kconfig"
> >
> >  source "drivers/staging/ralink-gdma/Kconfig"
> > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> > index c7f8d8d8dd11..76f413470bc8 100644
> > --- a/drivers/staging/Makefile
> > +++ b/drivers/staging/Makefile
> > @@ -33,7 +33,6 @@ obj-$(CONFIG_KS7010)                += ks7010/
> >  obj-$(CONFIG_GREYBUS)                += greybus/
> >  obj-$(CONFIG_BCM2835_VCHIQ)  += vc04_services/
> >  obj-$(CONFIG_PI433)          += pi433/
> > -obj-$(CONFIG_PCI_MT7621)     += mt7621-pci/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-dma/
> >  obj-$(CONFIG_DMA_RALINK)     += ralink-gdma/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-dts/
> > diff --git a/drivers/staging/mt7621-pci/Kconfig b/drivers/staging/mt7621-pci/Kconfig
> > deleted file mode 100644
> > index ce58042f2f21..000000000000
> > --- a/drivers/staging/mt7621-pci/Kconfig
> > +++ /dev/null
> > @@ -1,8 +0,0 @@
> > -# SPDX-License-Identifier: GPL-2.0
> > -config PCI_MT7621
> > -     tristate "MediaTek MT7621 PCI Controller"
> > -     depends on RALINK
> > -     select PCI_DRIVERS_GENERIC
> > -     help
> > -       This selects a driver for the MediaTek MT7621 PCI Controller.
> > -
> > diff --git a/drivers/staging/mt7621-pci/Makefile b/drivers/staging/mt7621-pci/Makefile
> > deleted file mode 100644
> > index f4e651cf7ce3..000000000000
> > --- a/drivers/staging/mt7621-pci/Makefile
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -# SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_PCI_MT7621)       += pci-mt7621.o
> > diff --git a/drivers/staging/mt7621-pci/TODO b/drivers/staging/mt7621-pci/TODO
> > deleted file mode 100644
> > index d674a9ac85c1..000000000000
> > --- a/drivers/staging/mt7621-pci/TODO
> > +++ /dev/null
> > @@ -1,4 +0,0 @@
> > -
> > -- general code review and cleanup
> > -
> > -Cc: NeilBrown <neil@brown.name>
> > diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> > deleted file mode 100644
> > index 327a68267309..000000000000
> > --- a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
> > +++ /dev/null
> > @@ -1,104 +0,0 @@
> > -MediaTek MT7621 PCIe controller
> > -
> > -Required properties:
> > -- compatible: "mediatek,mt7621-pci"
> > -- device_type: Must be "pci"
> > -- reg: Base addresses and lengths of the PCIe subsys and root ports.
> > -- bus-range: Range of bus numbers associated with this controller.
> > -- #address-cells: Address representation for root ports (must be 3)
> > -- pinctrl-names : The pin control state names.
> > -- pinctrl-0: The "default" pinctrl state.
> > -- #size-cells: Size representation for root ports (must be 2)
> > -- ranges: Ranges for the PCI memory and I/O regions.
> > -- #interrupt-cells: Must be 1
> > -- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties.
> > -  Please refer to the standard PCI bus binding document for a more detailed
> > -  explanation.
> > -- status: either "disabled" or "okay".
> > -- resets: Must contain an entry for each entry in reset-names.
> > -  See ../reset/reset.txt for details.
> > -- reset-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
> > -  root ports.
> > -- clocks: Must contain an entry for each entry in clock-names.
> > -  See ../clocks/clock-bindings.txt for details.
> > -- clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
> > -  root ports.
> > -- reset-gpios: GPIO specs for the reset pins.
> > -
> > -In addition, the device tree node must have sub-nodes describing each PCIe port
> > -interface, having the following mandatory properties:
> > -
> > -Required properties:
> > -- reg: Only the first four bytes are used to refer to the correct bus number
> > -      and device number.
> > -- #address-cells: Must be 3
> > -- #size-cells: Must be 2
> > -- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > -  property is sufficient.
> > -- bus-range: Range of bus numbers associated with this port.
> > -
> > -Example for MT7621:
> > -
> > -     pcie: pcie@1e140000 {
> > -             compatible = "mediatek,mt7621-pci";
> > -        reg = <0x1e140000 0x100    /* host-pci bridge registers */
> > -               0x1e142000 0x100    /* pcie port 0 RC control registers */
> > -               0x1e143000 0x100    /* pcie port 1 RC control registers */
> > -               0x1e144000 0x100>;  /* pcie port 2 RC control registers */
> > -
> > -             #address-cells = <3>;
> > -             #size-cells = <2>;
> > -
> > -             pinctrl-names = "default";
> > -             pinctrl-0 = <&pcie_pins>;
> > -
> > -             device_type = "pci";
> > -
> > -             bus-range = <0 255>;
> > -             ranges = <
> > -                     0x02000000 0 0x00000000 0x60000000 0 0x10000000 /* pci memory */
> > -                     0x01000000 0 0x00000000 0x1e160000 0 0x00010000 /* io space */
> > -             >;
> > -
> > -             #interrupt-cells = <1>;
> > -             interrupt-map-mask = <0xF0000 0 0 1>;
> > -             interrupt-map = <0x10000 0 0 1 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>,
> > -                             <0x20000 0 0 1 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>,
> > -                             <0x30000 0 0 1 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
> > -
> > -             status = "disabled";
> > -
> > -             resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
> > -             reset-names = "pcie0", "pcie1", "pcie2";
> > -             clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> > -             clock-names = "pcie0", "pcie1", "pcie2";
> > -
> > -             reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
> > -                             <&gpio 8 GPIO_ACTIVE_LOW>,
> > -                             <&gpio 7 GPIO_ACTIVE_LOW>;
> > -
> > -             pcie@0,0 {
> > -                     reg = <0x0000 0 0 0 0>;
> > -                     #address-cells = <3>;
> > -                     #size-cells = <2>;
> > -                     ranges;
> > -                     bus-range = <0x00 0xff>;
> > -             };
> > -
> > -             pcie@1,0 {
> > -                     reg = <0x0800 0 0 0 0>;
> > -                     #address-cells = <3>;
> > -                     #size-cells = <2>;
> > -                     ranges;
> > -                     bus-range = <0x00 0xff>;
> > -             };
> > -
> > -             pcie@2,0 {
> > -                     reg = <0x1000 0 0 0 0>;
> > -                     #address-cells = <3>;
> > -                     #size-cells = <2>;
> > -                     ranges;
> > -                     bus-range = <0x00 0xff>;
> > -             };
> > -     };
> > -
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-09-22  5:00 [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
                   ` (2 preceding siblings ...)
  2021-09-22  5:00 ` [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI " Sergio Paracuellos
@ 2021-10-20 14:23 ` Lorenzo Pieralisi
  2021-10-20 14:56   ` Sergio Paracuellos
  2021-10-21 15:52   ` Bjorn Helgaas
  3 siblings, 2 replies; 18+ messages in thread
From: Lorenzo Pieralisi @ 2021-10-20 14:23 UTC (permalink / raw)
  To: linux-pci, Sergio Paracuellos
  Cc: Lorenzo Pieralisi, linux-mips, linux-kernel, john, neil, gregkh,
	linux-staging, matthias.bgg, tsbogend, devicetree, bhelgaas

On Wed, 22 Sep 2021 07:00:32 +0200, Sergio Paracuellos wrote:
> MediaTek MT7621 PCIe subsys supports single Root complex (RC)
> with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link.
> Topology is as follows:
> 
> 
>                           MT7621 PCIe HOST Topology
> 
> [...]

Applied to pci/mt7621, thanks!

[1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
      https://git.kernel.org/lpieralisi/pci/c/e5bc5605e7
[2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
      https://git.kernel.org/lpieralisi/pci/c/5797a2b2bc
[3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
      https://git.kernel.org/lpieralisi/pci/c/eb1d7d438c

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-20 14:23 ` [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host " Lorenzo Pieralisi
@ 2021-10-20 14:56   ` Sergio Paracuellos
  2021-10-21 15:52   ` Bjorn Helgaas
  1 sibling, 0 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-20 14:56 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-pci, open list:MIPS, linux-kernel, John Crispin, NeilBrown,
	Greg KH, linux-staging, Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Wed, Oct 20, 2021 at 4:23 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Wed, 22 Sep 2021 07:00:32 +0200, Sergio Paracuellos wrote:
> > MediaTek MT7621 PCIe subsys supports single Root complex (RC)
> > with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link.
> > Topology is as follows:
> >
> >
> >                           MT7621 PCIe HOST Topology
> >
> > [...]
>
> Applied to pci/mt7621, thanks!
>
> [1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
>       https://git.kernel.org/lpieralisi/pci/c/e5bc5605e7
> [2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
>       https://git.kernel.org/lpieralisi/pci/c/5797a2b2bc
> [3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
>       https://git.kernel.org/lpieralisi/pci/c/eb1d7d438c
>
> Thanks,
> Lorenzo

Thanks, Lorenzo.

Best regards,
     Sergio Paracuellos

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-20 14:23 ` [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host " Lorenzo Pieralisi
  2021-10-20 14:56   ` Sergio Paracuellos
@ 2021-10-21 15:52   ` Bjorn Helgaas
  2021-10-21 17:27     ` Sergio Paracuellos
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2021-10-21 15:52 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-pci, Sergio Paracuellos, linux-mips, linux-kernel, john,
	neil, gregkh, linux-staging, matthias.bgg, tsbogend, devicetree,
	bhelgaas

On Wed, Oct 20, 2021 at 03:23:45PM +0100, Lorenzo Pieralisi wrote:
> On Wed, 22 Sep 2021 07:00:32 +0200, Sergio Paracuellos wrote:
> > MediaTek MT7621 PCIe subsys supports single Root complex (RC)
> > with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link.
> > Topology is as follows:
> > 
> > 
> >                           MT7621 PCIe HOST Topology
> > 
> > [...]
> 
> Applied to pci/mt7621, thanks!
> 
> [1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
>       https://git.kernel.org/lpieralisi/pci/c/e5bc5605e7
> [2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
>       https://git.kernel.org/lpieralisi/pci/c/5797a2b2bc
> [3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
>       https://git.kernel.org/lpieralisi/pci/c/eb1d7d438c

Since this is a PCIe (not conventional PCI) controller, I vote for
renaming these from:

  PCI_MT7621
  Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
  drivers/pci/controller/pci-mt7621.c

to:

  PCIE_MT7621
  Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
  drivers/pci/controller/pcie-mt7621.c

We have a mix of these, with many of the early PCIe drivers being
named "pci", but I think that was my mistake and there's no reason to
continue it.

I can do this locally unless somebody objects.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-21 15:52   ` Bjorn Helgaas
@ 2021-10-21 17:27     ` Sergio Paracuellos
  2021-10-21 18:11       ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-21 17:27 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, linux-pci, open list:MIPS, linux-kernel,
	John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

Hi Bjorn,

On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Oct 20, 2021 at 03:23:45PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, 22 Sep 2021 07:00:32 +0200, Sergio Paracuellos wrote:
> > > MediaTek MT7621 PCIe subsys supports single Root complex (RC)
> > > with 3 Root Ports. Each Root Ports supports a Gen1 1-lane Link.
> > > Topology is as follows:
> > >
> > >
> > >                           MT7621 PCIe HOST Topology
> > >
> > > [...]
> >
> > Applied to pci/mt7621, thanks!
> >
> > [1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs
> >       https://git.kernel.org/lpieralisi/pci/c/e5bc5605e7
> > [2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
> >       https://git.kernel.org/lpieralisi/pci/c/5797a2b2bc
> > [3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver
> >       https://git.kernel.org/lpieralisi/pci/c/eb1d7d438c
>
> Since this is a PCIe (not conventional PCI) controller, I vote for
> renaming these from:
>
>   PCI_MT7621
>   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
>   drivers/pci/controller/pci-mt7621.c
>
> to:
>
>   PCIE_MT7621
>   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
>   drivers/pci/controller/pcie-mt7621.c
>
> We have a mix of these, with many of the early PCIe drivers being
> named "pci", but I think that was my mistake and there's no reason to
> continue it.

I see.

>
> I can do this locally unless somebody objects.

I have no problem at all. Only one question. Do you mean to change
compatible string also, or only the name of the file? Let me know if I
have to do anything.

Thanks,
    Sergio Paracuellos

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-21 17:27     ` Sergio Paracuellos
@ 2021-10-21 18:11       ` Bjorn Helgaas
  2021-10-21 19:23         ` Sergio Paracuellos
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2021-10-21 18:11 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Lorenzo Pieralisi, linux-pci, open list:MIPS, linux-kernel,
	John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Since this is a PCIe (not conventional PCI) controller, I vote for
> > renaming these from:
> >
> >   PCI_MT7621
> >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> >   drivers/pci/controller/pci-mt7621.c
> >
> > to:
> >
> >   PCIE_MT7621
> >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> >   drivers/pci/controller/pcie-mt7621.c
> >
> > We have a mix of these, with many of the early PCIe drivers being
> > named "pci", but I think that was my mistake and there's no reason to
> > continue it.
> 
> I see.
> 
> >
> > I can do this locally unless somebody objects.
> 
> I have no problem at all. Only one question. Do you mean to change
> compatible string also, or only the name of the file? Let me know if I
> have to do anything.

I didn't change the compatible string, to avoid a DT incompatibility.
But I *did* change the Kconfig symbol to PCIE_MT7621, which could
require changes to out-of-tree .configs.  I'm open to suggestions
either way for both things.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-21 18:11       ` Bjorn Helgaas
@ 2021-10-21 19:23         ` Sergio Paracuellos
  2021-10-22  8:34           ` Lorenzo Pieralisi
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-21 19:23 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, linux-pci, open list:MIPS, linux-kernel,
	John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > Since this is a PCIe (not conventional PCI) controller, I vote for
> > > renaming these from:
> > >
> > >   PCI_MT7621
> > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > >   drivers/pci/controller/pci-mt7621.c
> > >
> > > to:
> > >
> > >   PCIE_MT7621
> > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > >   drivers/pci/controller/pcie-mt7621.c
> > >
> > > We have a mix of these, with many of the early PCIe drivers being
> > > named "pci", but I think that was my mistake and there's no reason to
> > > continue it.
> >
> > I see.
> >
> > >
> > > I can do this locally unless somebody objects.
> >
> > I have no problem at all. Only one question. Do you mean to change
> > compatible string also, or only the name of the file? Let me know if I
> > have to do anything.
>
> I didn't change the compatible string, to avoid a DT incompatibility.
> But I *did* change the Kconfig symbol to PCIE_MT7621, which could
> require changes to out-of-tree .configs.  I'm open to suggestions
> either way for both things.

IMHO, I do think we should not worry about out-of-tree stuff at all.
If the correct way to define the Kconfig symbol or the compatible
string is to change them, just do that. MT7621 SoC is extensively used
by openWRT community. As far as I have seen until now, the way of
doing things there is to take the latest long term kernel (now they
are using 5.4 as stable and 5.10 as testing kernel), apply a bunch of
patches they have and do a complete build of both kernel, device tree
and rootfs. So I guess it is not a big problem if we also change
compatible string since when an update is performed for a device all
of the stuff is just replaced. Maybe I am wrong and John has a
different opinion... John, any comments on this?

Best regards,
    Sergio Paracuellos

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-21 19:23         ` Sergio Paracuellos
@ 2021-10-22  8:34           ` Lorenzo Pieralisi
  2021-10-22  9:13             ` Sergio Paracuellos
  0 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2021-10-22  8:34 UTC (permalink / raw)
  To: Sergio Paracuellos, robh+dt
  Cc: Bjorn Helgaas, linux-pci, open list:MIPS, linux-kernel,
	John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:
> On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > Since this is a PCIe (not conventional PCI) controller, I vote for
> > > > renaming these from:
> > > >
> > > >   PCI_MT7621
> > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > > >   drivers/pci/controller/pci-mt7621.c
> > > >
> > > > to:
> > > >
> > > >   PCIE_MT7621
> > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > > >   drivers/pci/controller/pcie-mt7621.c
> > > >
> > > > We have a mix of these, with many of the early PCIe drivers being
> > > > named "pci", but I think that was my mistake and there's no reason to
> > > > continue it.
> > >
> > > I see.
> > >
> > > >
> > > > I can do this locally unless somebody objects.
> > >
> > > I have no problem at all. Only one question. Do you mean to change
> > > compatible string also, or only the name of the file? Let me know if I
> > > have to do anything.
> >
> > I didn't change the compatible string, to avoid a DT incompatibility.
> > But I *did* change the Kconfig symbol to PCIE_MT7621, which could
> > require changes to out-of-tree .configs.  I'm open to suggestions
> > either way for both things.
> 
> IMHO, I do think we should not worry about out-of-tree stuff at all.

For Kconfig I tend to agree. For DT I see some "bindings" in the staging
tree are being deleted and published as official DT bindings with this
patchset but I believe we still have to keep the compatible string
backward compatibility regardless because there may be firmware out
there using it.

Rob, what's the standard policy that should be used in this case ?

Thanks,
Lorenzo

> If the correct way to define the Kconfig symbol or the compatible
> string is to change them, just do that. MT7621 SoC is extensively used
> by openWRT community. As far as I have seen until now, the way of
> doing things there is to take the latest long term kernel (now they
> are using 5.4 as stable and 5.10 as testing kernel), apply a bunch of
> patches they have and do a complete build of both kernel, device tree
> and rootfs. So I guess it is not a big problem if we also change
> compatible string since when an update is performed for a device all
> of the stuff is just replaced. Maybe I am wrong and John has a
> different opinion... John, any comments on this?
> 
> Best regards,
>     Sergio Paracuellos

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-22  8:34           ` Lorenzo Pieralisi
@ 2021-10-22  9:13             ` Sergio Paracuellos
  2021-10-25 21:12               ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-22  9:13 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Rob Herring, Bjorn Helgaas, linux-pci, open list:MIPS,
	linux-kernel, John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Fri, Oct 22, 2021 at 10:35 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:
> > On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > > > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > Since this is a PCIe (not conventional PCI) controller, I vote for
> > > > > renaming these from:
> > > > >
> > > > >   PCI_MT7621
> > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > > > >   drivers/pci/controller/pci-mt7621.c
> > > > >
> > > > > to:
> > > > >
> > > > >   PCIE_MT7621
> > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > > > >   drivers/pci/controller/pcie-mt7621.c
> > > > >
> > > > > We have a mix of these, with many of the early PCIe drivers being
> > > > > named "pci", but I think that was my mistake and there's no reason to
> > > > > continue it.
> > > >
> > > > I see.
> > > >
> > > > >
> > > > > I can do this locally unless somebody objects.
> > > >
> > > > I have no problem at all. Only one question. Do you mean to change
> > > > compatible string also, or only the name of the file? Let me know if I
> > > > have to do anything.
> > >
> > > I didn't change the compatible string, to avoid a DT incompatibility.
> > > But I *did* change the Kconfig symbol to PCIE_MT7621, which could
> > > require changes to out-of-tree .configs.  I'm open to suggestions
> > > either way for both things.
> >
> > IMHO, I do think we should not worry about out-of-tree stuff at all.
>
> For Kconfig I tend to agree. For DT I see some "bindings" in the staging
> tree are being deleted and published as official DT bindings with this
> patchset but I believe we still have to keep the compatible string
> backward compatibility regardless because there may be firmware out
> there using it.

The bindings txt file removed in staging with this patchset was also
added by me three years ago[0], and has been changing until the YAML
bindings are reviewed by Rob and driver updated accordly in this
patchset.
OpenWRT maintains its own file[1] which I don't know is updated or not
according to the one in staging which I am pretending to properly
mainline for 5.17. But yes, I agree there might be firmware out there
using current compatible string.

[0]: Commit 5451e22618b8 ("staging: mt7621-pci: dt-bindings: add dt
bindings for mt7621 pcie controller")
[1]: https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi

Best regards,
    Sergio Paracuellos
>
> Rob, what's the standard policy that should be used in this case ?
>
> Thanks,
> Lorenzo
>
> > If the correct way to define the Kconfig symbol or the compatible
> > string is to change them, just do that. MT7621 SoC is extensively used
> > by openWRT community. As far as I have seen until now, the way of
> > doing things there is to take the latest long term kernel (now they
> > are using 5.4 as stable and 5.10 as testing kernel), apply a bunch of
> > patches they have and do a complete build of both kernel, device tree
> > and rootfs. So I guess it is not a big problem if we also change
> > compatible string since when an update is performed for a device all
> > of the stuff is just replaced. Maybe I am wrong and John has a
> > different opinion... John, any comments on this?
> >
> > Best regards,
> >     Sergio Paracuellos

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-22  9:13             ` Sergio Paracuellos
@ 2021-10-25 21:12               ` Bjorn Helgaas
  2021-10-26  5:49                 ` Sergio Paracuellos
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2021-10-25 21:12 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Lorenzo Pieralisi, Rob Herring, linux-pci, open list:MIPS,
	linux-kernel, John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Fri, Oct 22, 2021 at 11:13:39AM +0200, Sergio Paracuellos wrote:
> On Fri, Oct 22, 2021 at 10:35 AM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:
> > > On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > >
> > > > On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > > > > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > > Since this is a PCIe (not conventional PCI) controller, I
> > > > > > vote for renaming these from:
> > > > > >
> > > > > >   PCI_MT7621
> > > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > > > > >   drivers/pci/controller/pci-mt7621.c
> > > > > >
> > > > > > to:
> > > > > >
> > > > > >   PCIE_MT7621
> > > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > > > > >   drivers/pci/controller/pcie-mt7621.c
> > > > > >
> > > > > > We have a mix of these, with many of the early PCIe
> > > > > > drivers being named "pci", but I think that was my mistake
> > > > > > and there's no reason to continue it.
> > > > >
> > > > > I see.
> > > > >
> > > > > > I can do this locally unless somebody objects.
> > > > >
> > > > > I have no problem at all. Only one question. Do you mean to
> > > > > change compatible string also, or only the name of the file?
> > > > > Let me know if I have to do anything.
> > > >
> > > > I didn't change the compatible string, to avoid a DT
> > > > incompatibility.  But I *did* change the Kconfig symbol to
> > > > PCIE_MT7621, which could require changes to out-of-tree
> > > > .configs.  I'm open to suggestions either way for both things.
> > >
> > > IMHO, I do think we should not worry about out-of-tree stuff at
> > > all.
> >
> > For Kconfig I tend to agree. For DT I see some "bindings" in the
> > staging tree are being deleted and published as official DT
> > bindings with this patchset but I believe we still have to keep
> > the compatible string backward compatibility regardless because
> > there may be firmware out there using it.
> 
> The bindings txt file removed in staging with this patchset was also
> added by me three years ago[0], and has been changing until the YAML
> bindings are reviewed by Rob and driver updated accordly in this
> patchset.
>
> OpenWRT maintains its own file[1] which I don't know is updated or
> not according to the one in staging which I am pretending to
> properly mainline for 5.17. But yes, I agree there might be firmware
> out there using current compatible string.
> 
> [0]: Commit 5451e22618b8 ("staging: mt7621-pci: dt-bindings: add dt
> bindings for mt7621 pcie controller")
> [1]: https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi

OK, for now I left my rework as-is:

  - changed CONFIG_PCI_MT7621 to CONFIG_PCIE_MT7621
  - renamed mediatek,mt7621-pci.yaml to mediatek,mt7621-pcie.yaml
  - renamed pci-mt7621.c to pcie-mt7621.c
  - kept DT compatible string "mediatek,mt7621-pci" in .yaml and .c

I reason that the Kconfig and filename changes only affect people
building kernels or DTs, but a compatible string change would force a
DT update to be synchronized with a kernel update.

Happy to change this if necessary.

Bjorn

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver
  2021-10-25 21:12               ` Bjorn Helgaas
@ 2021-10-26  5:49                 ` Sergio Paracuellos
  0 siblings, 0 replies; 18+ messages in thread
From: Sergio Paracuellos @ 2021-10-26  5:49 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, Rob Herring, linux-pci, open list:MIPS,
	linux-kernel, John Crispin, NeilBrown, Greg KH, linux-staging,
	Matthias Brugger, Thomas Bogendoerfer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Bjorn Helgaas

On Mon, Oct 25, 2021 at 11:12 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Oct 22, 2021 at 11:13:39AM +0200, Sergio Paracuellos wrote:
> > On Fri, Oct 22, 2021 at 10:35 AM Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > >
> > > On Thu, Oct 21, 2021 at 09:23:35PM +0200, Sergio Paracuellos wrote:
> > > > On Thu, Oct 21, 2021 at 8:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > >
> > > > > On Thu, Oct 21, 2021 at 07:27:21PM +0200, Sergio Paracuellos wrote:
> > > > > > On Thu, Oct 21, 2021 at 5:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > > > Since this is a PCIe (not conventional PCI) controller, I
> > > > > > > vote for renaming these from:
> > > > > > >
> > > > > > >   PCI_MT7621
> > > > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
> > > > > > >   drivers/pci/controller/pci-mt7621.c
> > > > > > >
> > > > > > > to:
> > > > > > >
> > > > > > >   PCIE_MT7621
> > > > > > >   Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> > > > > > >   drivers/pci/controller/pcie-mt7621.c
> > > > > > >
> > > > > > > We have a mix of these, with many of the early PCIe
> > > > > > > drivers being named "pci", but I think that was my mistake
> > > > > > > and there's no reason to continue it.
> > > > > >
> > > > > > I see.
> > > > > >
> > > > > > > I can do this locally unless somebody objects.
> > > > > >
> > > > > > I have no problem at all. Only one question. Do you mean to
> > > > > > change compatible string also, or only the name of the file?
> > > > > > Let me know if I have to do anything.
> > > > >
> > > > > I didn't change the compatible string, to avoid a DT
> > > > > incompatibility.  But I *did* change the Kconfig symbol to
> > > > > PCIE_MT7621, which could require changes to out-of-tree
> > > > > .configs.  I'm open to suggestions either way for both things.
> > > >
> > > > IMHO, I do think we should not worry about out-of-tree stuff at
> > > > all.
> > >
> > > For Kconfig I tend to agree. For DT I see some "bindings" in the
> > > staging tree are being deleted and published as official DT
> > > bindings with this patchset but I believe we still have to keep
> > > the compatible string backward compatibility regardless because
> > > there may be firmware out there using it.
> >
> > The bindings txt file removed in staging with this patchset was also
> > added by me three years ago[0], and has been changing until the YAML
> > bindings are reviewed by Rob and driver updated accordly in this
> > patchset.
> >
> > OpenWRT maintains its own file[1] which I don't know is updated or
> > not according to the one in staging which I am pretending to
> > properly mainline for 5.17. But yes, I agree there might be firmware
> > out there using current compatible string.
> >
> > [0]: Commit 5451e22618b8 ("staging: mt7621-pci: dt-bindings: add dt
> > bindings for mt7621 pcie controller")
> > [1]: https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi
>
> OK, for now I left my rework as-is:
>
>   - changed CONFIG_PCI_MT7621 to CONFIG_PCIE_MT7621
>   - renamed mediatek,mt7621-pci.yaml to mediatek,mt7621-pcie.yaml
>   - renamed pci-mt7621.c to pcie-mt7621.c
>   - kept DT compatible string "mediatek,mt7621-pci" in .yaml and .c
>
> I reason that the Kconfig and filename changes only affect people
> building kernels or DTs, but a compatible string change would force a
> DT update to be synchronized with a kernel update.

This is all ok for me, Bjorn. Thanks for doing this. I guess even if
we don't force people to a DT update to synchronize things, since
bindings have been changed until they have been approved, I guess most
people must upgrade from early not approved DT early versions in any
case. But in any case I guess that maintaining the compatible string
is the safest thing to do.

>
> Happy to change this if necessary.

Best regards,
    Sergio Paracuellos

>
> Bjorn

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-26  5:49 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22  5:00 [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
2021-09-22  5:00 ` [PATCH v3 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs Sergio Paracuellos
2021-09-22  5:00 ` [PATCH v3 2/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver Sergio Paracuellos
2021-10-13 13:05   ` Lorenzo Pieralisi
2021-10-13 13:35     ` Sergio Paracuellos
2021-09-22  5:00 ` [PATCH v3 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI " Sergio Paracuellos
2021-09-22  9:05   ` Sergei Shtylyov
2021-10-03 16:55     ` Sergio Paracuellos
2021-10-20 14:23 ` [PATCH v3 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host " Lorenzo Pieralisi
2021-10-20 14:56   ` Sergio Paracuellos
2021-10-21 15:52   ` Bjorn Helgaas
2021-10-21 17:27     ` Sergio Paracuellos
2021-10-21 18:11       ` Bjorn Helgaas
2021-10-21 19:23         ` Sergio Paracuellos
2021-10-22  8:34           ` Lorenzo Pieralisi
2021-10-22  9:13             ` Sergio Paracuellos
2021-10-25 21:12               ` Bjorn Helgaas
2021-10-26  5:49                 ` Sergio Paracuellos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).