2021-06-09 17:40:04

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v2 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver

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.

Another thing that must be mentioned is that this driver uses IO
in physical address 0x001e160000. IO_SPACE_LIMIT for MIPS is 0xffff
so some generic PCI functions (like of_pci_range_to_resource) won't
work and the resource ranges part for IO is set manually.

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'.

NOTE: Greg, I have maintained your Acked-by from previous series in
delete driver commit and added in the one which is moving code here
and delete the remaining stuff. If you are not ok with this, just
let me now and I'll drop it and resend.

Thanks in advance for your time.

Best regards,
Sergio Paracuellos

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 | 2 +-
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, 158 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


2021-06-09 18:46:20

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs

Add device tree binding documentation for PCIe in MT7621 SoCs.

Signed-off-by: Sergio Paracuellos <[email protected]>
---
.../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..716b77d6c830
--- /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 <[email protected]>
+
+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 0x00000000 0x60000000 0 0x10000000>, /* pci memory */
+ <0x01000000 0 0x00000000 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

2021-06-09 18:46:52

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v2 3/3] MAINTAINERS: add myself as maintainer of the MT7621 PCI controller driver

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

Signed-off-by: Sergio Paracuellos <[email protected]>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c55fdcc1514..2e58fba01289 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11574,6 +11574,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 <[email protected]>
+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 <[email protected]>
S: Maintained
--
2.25.1

2021-06-17 06:01:46

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver

Hi,

On Wed, Jun 9, 2021 at 4:02 PM Sergio Paracuellos
<[email protected]> 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
>
> .-------.
> | |
> | 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.
>
> Another thing that must be mentioned is that this driver uses IO
> in physical address 0x001e160000. IO_SPACE_LIMIT for MIPS is 0xffff
> so some generic PCI functions (like of_pci_range_to_resource) won't
> work and the resource ranges part for IO is set manually.

This has been fixed and now there is no need to set io resources manually.
See [0].

>
> 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'.

I forgot to comment that all of these changes are rebased on the top
of staging-next.
Since this is a 'git mv' as I was told to do by Bjorn, last version of
the code is available
here [1] with the following added changes to the ones listed above
from previously submitted v1 series:

- Define PCI_IOBASE for mips (spaces.h) to avoid parsing io resources manually
so 'mt7621_pci_parse_request_of_pci_ranges' is not needed anymore.
- Don't store resources in driver private data but just get them to properly
set io window register and mips iocu memory regions.

Thanks in advance for your time.

Best regards,
Sergio Paracuellos

[0]: https://lore.kernel.org/linux-staging/[email protected]/T/#t
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/mt7621-pci/pci-mt7621.c?h=staging-next

>
> NOTE: Greg, I have maintained your Acked-by from previous series in
> delete driver commit and added in the one which is moving code here
> and delete the remaining stuff. If you are not ok with this, just
> let me now and I'll drop it and resend.
>
> Thanks in advance for your time.
>
> Best regards,
> Sergio Paracuellos
>
> 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 | 2 +-
> 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, 158 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
>

2021-06-19 06:45:54

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: mt7621-pci: PCIe binding documentation for MT7621 SoCs

On Wed, 09 Jun 2021 16:01:57 +0200, Sergio Paracuellos wrote:
> Add device tree binding documentation for PCIe in MT7621 SoCs.
>
> Signed-off-by: Sergio Paracuellos <[email protected]>
> ---
> .../bindings/pci/mediatek,mt7621-pci.yaml | 142 ++++++++++++++++++
> 1 file changed, 142 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/mediatek,mt7621-pci.yaml
>

Reviewed-by: Rob Herring <[email protected]>