From: Chuanjia Liu <hidden> Date: 2021-07-19 07:35:54
There are two independent PCIe controllers in MT2712 and MT7622 platform.
Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root bridge,
and all of the devices will share the same MSI domain.Hence that,
the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
change note:
v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
description in binding file. No code change.
v10:Rebase for 5.13-rc1, no code change.
v9:fix kernel-ci bot warning. In the scene of using new dts format,
when mtk_pcie_parse_port fails, of_node_put don't need to be called.
v8:remove slot node and fix yaml warning.
v7:dt-bindings file was modified as suggested by Rob, other file no
change.
v6:Fix yaml error. make sure driver compatible with old and
new DTS format.
v5:rebase for 5.9-rc1, no code change.
v4:change commit message due to bayes statistical bogofilter
considers this series patch SPAM.
v3:rebase for 5.8-rc1. Only collect ack of Ryder, No code change.
v2:change the allocation of MT2712 PCIe MMIO space due to the
allocation size is not right in v1.
Chuanjia Liu (4):
dt-bindings: PCI: mediatek: Update the Device tree bindings
PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622
ARM: dts: mediatek: Update MT7629 PCIe node for new format
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
arch/arm/boot/dts/mt7629-rfb.dts | 3 +-
arch/arm/boot/dts/mt7629.dtsi | 45 ++--
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 97 +++++----
.../dts/mediatek/mt7622-bananapi-bpi-r64.dts | 16 +-
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 6 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 112 +++++-----
drivers/pci/controller/pcie-mediatek.c | 52 +++--
9 files changed, 330 insertions(+), 246 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
--
2.18.0
From: Chuanjia Liu <hidden> Date: 2021-07-19 07:36:12
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
2 files changed, 150 insertions(+), 95 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
@@ -8,7 +8,7 @@ Required properties: "mediatek,mt7623-pcie" "mediatek,mt7629-pcie" - device_type: Must be "pci"-- reg: Base addresses and lengths of the PCIe subsys and root ports.+- reg: Base addresses and lengths of the root ports. - reg-names: Names of the above areas to use during resource lookup. - #address-cells: Address representation for root ports (must be 3) - #size-cells: Size representation for root ports (must be 2)
@@ -47,9 +47,12 @@ Required properties for MT7623/MT2701: - reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the number of root ports.-Required properties for MT2712/MT7622:+Required properties for MT2712/MT7622/MT7629: -interrupts: A list of interrupt outputs of the controller, must have one entry for each PCIe port+- interrupt-names: Must include the following entries:+ - "pcie_irq": The interrupt that is asserted when an MSI/INTX is received+- linux,pci-domain: PCI domain ID. Should be unique for each host controller In addition, the device tree node must have sub-nodes describing each PCIe port interface, having the following mandatory properties:
From: Rob Herring <robh@kernel.org> Date: 2021-07-20 00:18:13
On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
2 files changed, 150 insertions(+), 95 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
From: Chuanjia Liu <hidden> Date: 2021-07-20 02:11:27
On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
quoted
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
2 files changed, 150 insertions(+), 95 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
Hi,Rob
I have described in the cover letter:
v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
description in binding file. No code change.
if you still ok for this, I will add R-b in next version.
Best regards
From: Rob Herring <robh@kernel.org> Date: 2021-07-20 16:32:14
On Mon, Jul 19, 2021 at 8:07 PM Chuanjia Liu [off-list ref] wrote:
On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
quoted
On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
quoted
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
2 files changed, 150 insertions(+), 95 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
Hi,Rob
I have described in the cover letter:
v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
description in binding file. No code change.
if you still ok for this, I will add R-b in next version.
Yes, it's fine.
In the future, put the changelog for a patch in the patch.
Rob
From: Chuanjia Liu <hidden> Date: 2021-07-23 07:17:33
On Tue, 2021-07-20 at 10:26 -0600, Rob Herring wrote:
On Mon, Jul 19, 2021 at 8:07 PM Chuanjia Liu [off-list ref] wrote:
quoted
On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
quoted
On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
quoted
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 39 ++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
2 files changed, 150 insertions(+), 95 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
Hi,Rob
I have described in the cover letter:
v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
description in binding file. No code change.
if you still ok for this, I will add R-b in next version.
Yes, it's fine.
In the future, put the changelog for a patch in the patch.
Hi, Rob
Thanks for you suggestion, I will do this in future versions.
Best regards
From: Chuanjia Liu <hidden> Date: 2021-07-19 07:36:37
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;}err=mtk_pcie_subsys_powerup(pcie);
From: Chuanjia Liu <hidden> Date: 2021-07-20 03:02:59
On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
quoted hunk
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;
Hi，Rob
I changed this in the v9 version:
When the new dts format is used, of_node_get() is not called.
So when mtk_pcie_parse_port fails, of_node_put don't need to be called.
if you still ok for this, I will add R-b in next version.
Best Regards
From: Rob Herring <robh+dt@kernel.org> Date: 2021-08-03 22:18:26
On Mon, Jul 19, 2021 at 8:59 PM Chuanjia Liu [off-list ref] wrote:
On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
quoted
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;
Hi，Rob
I changed this in the v9 version:
When the new dts format is used, of_node_get() is not called.
So when mtk_pcie_parse_port fails, of_node_put don't need to be called.
if you still ok for this, I will add R-b in next version.
Yes, and that's small enough change to keep my R-b.
Rob
From: Chuanjia Liu (柳传嘉) <hidden> Date: 2021-08-06 07:37:52
On Tue, 2021-08-03 at 16:18 -0600, Rob Herring wrote:
On Mon, Jul 19, 2021 at 8:59 PM Chuanjia Liu <
chuanjia.liu@mediatek.com> wrote:
quoted
On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
quoted
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-
------
1 file changed, 39 insertions(+), 13 deletions(-)
Hi，Rob
I changed this in the v9 version:
When the new dts format is used, of_node_get() is not called.
So when mtk_pcie_parse_port fails, of_node_put don't need to be
called.
if you still ok for this, I will add R-b in next version.
Yes, and that's small enough change to keep my R-b.
Rob
Hi, Rob
Thanks for your review.
Hi lorenzo
If there are no more comments, can this series be merged?
Best Regards
From: Chuanjia Liu <hidden> Date: 2021-08-02 07:07:30
On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
Gently ping...
quoted hunk
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;}err=mtk_pcie_subsys_powerup(pcie);
On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
This commit log doesn't seem to really cover what's going on here. It
looks like:
- You added a check for "mediatek,generic-pciecfg" (I guess this is
the "shared pcie-cfg base address" part). Probably could have
been its own patch.
- You added checks for "interrupt-names" and "pcie_irq". Not
explained in commit log; probably could have been its own patch,
too.
- You now look for "linux,pci-domain" (via of_get_pci_domain_nr()).
If present, you parse only one port instead of looking for all the
children of the node.
That's sort of weird behavior -- why should the presence of
"linux,pci-domain" determine whether the node can have children?
Is that really what you intend?
Should be explained in the commit log and could have been its own
patch, too.
quoted hunk
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;}err=mtk_pcie_subsys_powerup(pcie);
From: Lorenzo Pieralisi <hidden> Date: 2021-08-13 15:23:14
On Tue, Aug 10, 2021 at 02:42:50PM -0500, Bjorn Helgaas wrote:
On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
quoted
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
This commit log doesn't seem to really cover what's going on here. It
looks like:
- You added a check for "mediatek,generic-pciecfg" (I guess this is
the "shared pcie-cfg base address" part). Probably could have
been its own patch.
- You added checks for "interrupt-names" and "pcie_irq". Not
explained in commit log; probably could have been its own patch,
too.
- You now look for "linux,pci-domain" (via of_get_pci_domain_nr()).
If present, you parse only one port instead of looking for all the
children of the node.
That's sort of weird behavior -- why should the presence of
"linux,pci-domain" determine whether the node can have children?
Is that really what you intend?
Should be explained in the commit log and could have been its own
patch, too.
I agree with Bjorn, this patch should be split (and commit logs
rewritten). I will drop it from my tree, waiting for a v12.
Lorenzo
quoted
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
@@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,returnerr;}-port->irq=platform_get_irq(pdev,port->slot);+if(of_find_property(dev->of_node,"interrupt-names",NULL))+port->irq=platform_get_irq_byname(pdev,"pcie_irq");+else+port->irq=platform_get_irq(pdev,port->slot);+if(port->irq<0)returnport->irq;
@@ -682,6 +690,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)val|=PCIE_CSR_LTSSM_EN(port->slot)|PCIE_CSR_ASPM_L1_EN(port->slot);writel(val,pcie->base+PCIE_SYS_CFG_V2);+}elseif(pcie->cfg){+val=PCIE_CSR_LTSSM_EN(port->slot)|+PCIE_CSR_ASPM_L1_EN(port->slot);+regmap_update_bits(pcie->cfg,PCIE_SYS_CFG_V2,val,val);}/* Assert all reset signals */
@@ -985,6 +997,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structplatform_device*pdev=to_platform_device(dev);structresource*regs;+structdevice_node*cfg_node;interr;/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)returnPTR_ERR(pcie->base);}+cfg_node=of_find_compatible_node(NULL,NULL,+"mediatek,generic-pciecfg");+if(cfg_node){+pcie->cfg=syscon_node_to_regmap(cfg_node);+if(IS_ERR(pcie->cfg))+returnPTR_ERR(pcie->cfg);+}+pcie->free_ck=devm_clk_get(dev,"free_ck");if(IS_ERR(pcie->free_ck)){if(PTR_ERR(pcie->free_ck)==-EPROBE_DEFER)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)structdevice*dev=pcie->dev;structdevice_node*node=dev->of_node,*child;structmtk_pcie_port*port,*tmp;-interr;+interr,slot;++slot=of_get_pci_domain_nr(dev->of_node);+if(slot<0){+for_each_available_child_of_node(node,child){+err=of_pci_get_devfn(child);+if(err<0){+dev_err(dev,"failed to get devfn: %d\n",err);+gotoerror_put_node;+}-for_each_available_child_of_node(node,child){-intslot;+slot=PCI_SLOT(err);-err=of_pci_get_devfn(child);-if(err<0){-dev_err(dev,"failed to parse devfn: %d\n",err);-gotoerror_put_node;+err=mtk_pcie_parse_port(pcie,child,slot);+if(err)+gotoerror_put_node;}--slot=PCI_SLOT(err);--err=mtk_pcie_parse_port(pcie,child,slot);+}else{+err=mtk_pcie_parse_port(pcie,node,slot);if(err)-gotoerror_put_node;+returnerr;}err=mtk_pcie_subsys_powerup(pcie);
From: Chuanjia Liu (柳传嘉) <hidden> Date: 2021-08-17 11:19:20
On Fri, 2021-08-13 at 16:22 +0100, Lorenzo Pieralisi wrote:
On Tue, Aug 10, 2021 at 02:42:50PM -0500, Bjorn Helgaas wrote:
quoted
On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
quoted
For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.
This commit log doesn't seem to really cover what's going on
here. It
looks like:
- You added a check for "mediatek,generic-pciecfg" (I guess this
is
the "shared pcie-cfg base address" part). Probably could have
been its own patch.
- You added checks for "interrupt-names" and "pcie_irq". Not
explained in commit log; probably could have been its own
patch,
too.
- You now look for "linux,pci-domain" (via
of_get_pci_domain_nr()).
If present, you parse only one port instead of looking for all
the
children of the node.
That's sort of weird behavior -- why should the presence of
"linux,pci-domain" determine whether the node can have
children?
Is that really what you intend?
Should be explained in the commit log and could have been its
own
patch, too.
hi, Bjorn,
Yes, this is my intention,"Linux,pci-domain" has two purposes
1)Distinguish the old and new dts formats, then parse them
2)Determine the current pcie slot num in new dts format, because the
offset of some regs needs to be based on slot num
I will split this patch into three patches, and then explain them in
commit log, thanks for your comment.
I agree with Bjorn, this patch should be split (and commit logs
rewritten). I will drop it from my tree, waiting for a v12.
Lorenzo
hi, Lorenzo
Thanks for your confirmation, I will send the V12 version as soon as
possible.
Chuanjia
From: Chuanjia Liu <hidden> Date: 2021-07-19 07:36:42
There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712 and MT7622 platform to comply with
the hardware design and fix MSI issue.
Signed-off-by: Chuanjia Liu <redacted>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 97 +++++++--------
.../dts/mediatek/mt7622-bananapi-bpi-r64.dts | 16 ++-
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 6 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 112 ++++++++++--------
4 files changed, 118 insertions(+), 113 deletions(-)
From: Lorenzo Pieralisi <hidden> Date: 2021-08-06 09:40:12
On Mon, 19 Jul 2021 15:34:52 +0800, Chuanjia Liu wrote:
There are two independent PCIe controllers in MT2712 and MT7622 platform.
Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root bridge,
and all of the devices will share the same MSI domain.Hence that,
the PCIe devices will not work properly if the irq number
which required is more than 32.
[...]
From: Chuanjia Liu (柳传嘉) <hidden> Date: 2021-08-08 04:51:19
On Fri, 2021-08-06 at 10:39 +0100, Lorenzo Pieralisi wrote:
On Mon, 19 Jul 2021 15:34:52 +0800, Chuanjia Liu wrote:
quoted
There are two independent PCIe controllers in MT2712 and MT7622
platform.
Each of them should contain an independent MSI domain.
In old dts architecture, MSI domain will be inherited from the root
bridge,
and all of the devices will share the same MSI domain.Hence that,
the PCIe devices will not work properly if the irq number
which required is more than 32.
[...]