* [PATCH v8 1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
@ 2025-08-28 7:34 ` Krishna Chaitanya Chundru
2025-08-28 7:34 ` [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping Krishna Chaitanya Chundru
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-28 7:34 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Krishna Chaitanya Chundru,
Konrad Dybcio, Manivannan Sadhasivam
PCIe ECAM(Enhanced Configuration Access Mechanism) feature requires
maximum of 256MB configuration space.
To enable this feature increase configuration space size to 256MB. If
the config space is increased, the BAR space needs to be truncated as
it resides in the same location. To avoid the bar space truncation move
config space, DBI, ELBI, iATU to upper PCIe region and use lower PCIe
iregion entirely for BAR region.
This depends on the commit: '10ba0854c5e6 ("PCI: qcom: Disable mirroring
of DBI and iATU register space in BAR region")'
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 64a2abd3010018e94eb50c534a509d6b4cf2473b..36afeb2e45937f8ad301c55caf296babdb499820 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2202,11 +2202,11 @@ wifi: wifi@17a10040 {
pcie1: pcie@1c08000 {
compatible = "qcom,pcie-sc7280";
- reg = <0 0x01c08000 0 0x3000>,
- <0 0x40000000 0 0xf1d>,
- <0 0x40000f20 0 0xa8>,
- <0 0x40001000 0 0x1000>,
- <0 0x40100000 0 0x100000>;
+ reg = <0x0 0x01c08000 0 0x3000>,
+ <0x4 0x10001000 0 0xf1d>,
+ <0x4 0x10001f20 0 0xa8>,
+ <0x4 0x10000000 0 0x1000>,
+ <0x4 0x00000000 0 0x10000000>;
reg-names = "parf", "dbi", "elbi", "atu", "config";
device_type = "pci";
@@ -2217,8 +2217,8 @@ pcie1: pcie@1c08000 {
#address-cells = <3>;
#size-cells = <2>;
- ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>,
- <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x1fd00000>;
+ ranges = <0x01000000 0x0 0x00000000 0x0 0x40000000 0x0 0x100000>,
+ <0x02000000 0x0 0x40100000 0x0 0x40100000 0x0 0x1ff00000>;
interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
2025-08-28 7:34 ` [PATCH v8 1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature Krishna Chaitanya Chundru
@ 2025-08-28 7:34 ` Krishna Chaitanya Chundru
2025-08-31 11:48 ` Manivannan Sadhasivam
2025-09-01 13:48 ` Manivannan Sadhasivam
2025-08-28 7:34 ` [PATCH v8 3/5] PCI: dwc: qcom: Switch to dwc " Krishna Chaitanya Chundru
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-28 7:34 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Krishna Chaitanya Chundru,
Manivannan Sadhasivam
External Local Bus Interface(ELBI) registers are optional registers in
DWC IPs having vendor specific registers.
Since ELBI register space is applicable for all DWC based controllers,
move the resource get code to DWC core and make it optional.
Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
}
}
+ if (!pci->elbi_base) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
+ if (res) {
+ pci->elbi_base = devm_ioremap_resource(pci->dev, res);
+ if (IS_ERR(pci->elbi_base))
+ return PTR_ERR(pci->elbi_base);
+ }
+ }
+
/* LLDD is supposed to manually switch the clocks and resets state */
if (dw_pcie_cap_is(pci, REQ_RES)) {
ret = dw_pcie_get_clocks(pci);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 00f52d472dcdd794013a865ad6c4c7cc251edb48..ceb022506c3191cd8fe580411526e20cc3758fed 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -492,6 +492,7 @@ struct dw_pcie {
resource_size_t dbi_phys_addr;
void __iomem *dbi_base2;
void __iomem *atu_base;
+ void __iomem *elbi_base;
resource_size_t atu_phys_addr;
size_t atu_size;
resource_size_t parent_bus_offset;
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-08-28 7:34 ` [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping Krishna Chaitanya Chundru
@ 2025-08-31 11:48 ` Manivannan Sadhasivam
2025-09-01 6:55 ` Krishna Chaitanya Chundru
2025-09-01 13:48 ` Manivannan Sadhasivam
1 sibling, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-31 11:48 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
> External Local Bus Interface(ELBI) registers are optional registers in
> DWC IPs having vendor specific registers.
>
> Since ELBI register space is applicable for all DWC based controllers,
> move the resource get code to DWC core and make it optional.
>
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> }
> }
>
> + if (!pci->elbi_base) {
Why this check is needed? Are we expecting any DWC glue drivers to supply
'dw_pcie::elbi_base' on their own?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-08-31 11:48 ` Manivannan Sadhasivam
@ 2025-09-01 6:55 ` Krishna Chaitanya Chundru
2025-09-01 9:07 ` Manivannan Sadhasivam
2025-09-01 13:44 ` Manivannan Sadhasivam
0 siblings, 2 replies; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-09-01 6:55 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On 8/31/2025 5:18 PM, Manivannan Sadhasivam wrote:
> On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
>> External Local Bus Interface(ELBI) registers are optional registers in
>> DWC IPs having vendor specific registers.
>>
>> Since ELBI register space is applicable for all DWC based controllers,
>> move the resource get code to DWC core and make it optional.
>>
>> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>> drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
>> drivers/pci/controller/dwc/pcie-designware.h | 1 +
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>> }
>> }
>>
>> + if (!pci->elbi_base) {
>
> Why this check is needed? Are we expecting any DWC glue drivers to supply
> 'dw_pcie::elbi_base' on their own?
>
I was following the same way that existed for for dbi_base, where we are
allowing DWC glue drivers to supply if they had any different approach
like ./pci-dra7xx.c driver.
- Krishna Chaitanya.
> - Mani
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-09-01 6:55 ` Krishna Chaitanya Chundru
@ 2025-09-01 9:07 ` Manivannan Sadhasivam
2025-09-01 13:44 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01 9:07 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On Mon, Sep 01, 2025 at 12:25:58PM GMT, Krishna Chaitanya Chundru wrote:
>
>
> On 8/31/2025 5:18 PM, Manivannan Sadhasivam wrote:
> > On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
> > > External Local Bus Interface(ELBI) registers are optional registers in
> > > DWC IPs having vendor specific registers.
> > >
> > > Since ELBI register space is applicable for all DWC based controllers,
> > > move the resource get code to DWC core and make it optional.
> > >
> > > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
> > > drivers/pci/controller/dwc/pcie-designware.h | 1 +
> > > 2 files changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> > > }
> > > }
> > > + if (!pci->elbi_base) {
> >
> > Why this check is needed? Are we expecting any DWC glue drivers to supply
> > 'dw_pcie::elbi_base' on their own?
> >
> I was following the same way that existed for for dbi_base, where we are
> allowing DWC glue drivers to supply if they had any different approach
> like ./pci-dra7xx.c driver.
>
That's because the glue drivers were using different dbi resource name in DT.
But for ELBI, we have so far seen only 'elbi'. So there is no need to allow
override.
I will remove this check while applying.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-09-01 6:55 ` Krishna Chaitanya Chundru
2025-09-01 9:07 ` Manivannan Sadhasivam
@ 2025-09-01 13:44 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01 13:44 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On Mon, Sep 01, 2025 at 12:25:58PM GMT, Krishna Chaitanya Chundru wrote:
>
>
> On 8/31/2025 5:18 PM, Manivannan Sadhasivam wrote:
> > On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
> > > External Local Bus Interface(ELBI) registers are optional registers in
> > > DWC IPs having vendor specific registers.
> > >
> > > Since ELBI register space is applicable for all DWC based controllers,
> > > move the resource get code to DWC core and make it optional.
> > >
> > > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
> > > drivers/pci/controller/dwc/pcie-designware.h | 1 +
> > > 2 files changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> > > }
> > > }
> > > + if (!pci->elbi_base) {
> >
> > Why this check is needed? Are we expecting any DWC glue drivers to supply
> > 'dw_pcie::elbi_base' on their own?
> >
> I was following the same way that existed for for dbi_base, where we are
> allowing DWC glue drivers to supply if they had any different approach
> like ./pci-dra7xx.c driver.
>
DBI is special because vendor glue drivers were using different name other than
'dbi' in DT. But for ELBI, all (both Exynos and Qcom EP) are using 'elbi' only.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-08-28 7:34 ` [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping Krishna Chaitanya Chundru
2025-08-31 11:48 ` Manivannan Sadhasivam
@ 2025-09-01 13:48 ` Manivannan Sadhasivam
2025-09-03 18:56 ` Bjorn Helgaas
1 sibling, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01 13:48 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
> External Local Bus Interface(ELBI) registers are optional registers in
> DWC IPs having vendor specific registers.
>
> Since ELBI register space is applicable for all DWC based controllers,
> move the resource get code to DWC core and make it optional.
>
As discussed offline, this changes also warrants switching the glue drivers to
use 'dw_pci::elbi' base instead of their own. So I've ammended this commit to
include those changes also while applying (which was straightforward).
- Mani
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> }
> }
>
> + if (!pci->elbi_base) {
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> + if (res) {
> + pci->elbi_base = devm_ioremap_resource(pci->dev, res);
> + if (IS_ERR(pci->elbi_base))
> + return PTR_ERR(pci->elbi_base);
> + }
> + }
> +
> /* LLDD is supposed to manually switch the clocks and resets state */
> if (dw_pcie_cap_is(pci, REQ_RES)) {
> ret = dw_pcie_get_clocks(pci);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 00f52d472dcdd794013a865ad6c4c7cc251edb48..ceb022506c3191cd8fe580411526e20cc3758fed 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -492,6 +492,7 @@ struct dw_pcie {
> resource_size_t dbi_phys_addr;
> void __iomem *dbi_base2;
> void __iomem *atu_base;
> + void __iomem *elbi_base;
> resource_size_t atu_phys_addr;
> size_t atu_size;
> resource_size_t parent_bus_offset;
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping
2025-09-01 13:48 ` Manivannan Sadhasivam
@ 2025-09-03 18:56 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 18:56 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Krishna Chaitanya Chundru, cros-qcom-dts-watchers,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
Bjorn Helgaas, Jingoo Han, linux-arm-msm, devicetree,
linux-kernel, linux-pci, quic_vbadigan, quic_mrana, quic_vpernami,
mmareddy
On Mon, Sep 01, 2025 at 07:18:17PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Aug 28, 2025 at 01:04:23PM GMT, Krishna Chaitanya Chundru wrote:
> > External Local Bus Interface(ELBI) registers are optional registers in
> > DWC IPs having vendor specific registers.
> >
> > Since ELBI register space is applicable for all DWC based controllers,
> > move the resource get code to DWC core and make it optional.
>
> As discussed offline, this changes also warrants switching the glue
> drivers to use 'dw_pci::elbi' base instead of their own. So I've
> ammended this commit to include those changes also while applying
> (which was straightforward).
I'm glad if we can do this in the DWC core instead of individual
drivers, but in the case of qcom, this changes the mapping from using
devm_pci_remap_cfgspace() to using devm_ioremap_resource():
qcom_pcie_ep_get_io_resources
platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi")
devm_pci_remap_cfg_resource
devm_pci_remap_cfgspace
pci_remap_cfgspace
ioremap_np # (except on arch/arm)
dw_pcie_get_resources
platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi")
devm_ioremap_resource
__devm_ioremap_resource(..., DEVM_IOREMAP)
__devm_ioremap
switch (type)
case DEVM_IOREMAP: ioremap
I assume this change from ioremap_np() to ioremap() is fine, but
please verify and update the commit log to mention this change and
explain why it's ok.
(I don't think the qcom changes were posted to the mailing list; you
can see them here:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc-ecam&id=d39e0103e38f9889271a77a837b6179b42d6730d)
> > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware.c | 9 +++++++++
> > drivers/pci/controller/dwc/pcie-designware.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 89aad5a08928cc29870ab258d33bee9ff8f83143..4684c671a81bee468f686a83cc992433b38af59d 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -167,6 +167,15 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> > }
> > }
> >
> > + if (!pci->elbi_base) {
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> > + if (res) {
> > + pci->elbi_base = devm_ioremap_resource(pci->dev, res);
> > + if (IS_ERR(pci->elbi_base))
> > + return PTR_ERR(pci->elbi_base);
> > + }
> > + }
> > +
> > /* LLDD is supposed to manually switch the clocks and resets state */
> > if (dw_pcie_cap_is(pci, REQ_RES)) {
> > ret = dw_pcie_get_clocks(pci);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 00f52d472dcdd794013a865ad6c4c7cc251edb48..ceb022506c3191cd8fe580411526e20cc3758fed 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -492,6 +492,7 @@ struct dw_pcie {
> > resource_size_t dbi_phys_addr;
> > void __iomem *dbi_base2;
> > void __iomem *atu_base;
> > + void __iomem *elbi_base;
> > resource_size_t atu_phys_addr;
> > size_t atu_size;
> > resource_size_t parent_bus_offset;
> >
> > --
> > 2.34.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 3/5] PCI: dwc: qcom: Switch to dwc ELBI resource mapping
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
2025-08-28 7:34 ` [PATCH v8 1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature Krishna Chaitanya Chundru
2025-08-28 7:34 ` [PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping Krishna Chaitanya Chundru
@ 2025-08-28 7:34 ` Krishna Chaitanya Chundru
2025-09-03 19:14 ` Bjorn Helgaas
2025-08-28 7:34 ` [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-28 7:34 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Krishna Chaitanya Chundru,
Konrad Dybcio, Manivannan Sadhasivam
Instead of using qcom ELBI resources mapping let the DWC core map it
ELBI is DWC specific.
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..5092752de23866ef95036bb3f8fae9bb06e8ea1e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -276,7 +276,6 @@ struct qcom_pcie_port {
struct qcom_pcie {
struct dw_pcie *pci;
void __iomem *parf; /* DT parf */
- void __iomem *elbi; /* DT elbi */
void __iomem *mhi;
union qcom_pcie_resources res;
struct phy *phy;
@@ -414,12 +413,17 @@ static void qcom_pcie_configure_dbi_atu_base(struct qcom_pcie *pcie)
static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie)
{
+ struct dw_pcie *pci = pcie->pci;
u32 val;
+ if (!pci->elbi_base) {
+ dev_err(pci->dev, "ELBI is not present\n");
+ return;
+ }
/* enable link training */
- val = readl(pcie->elbi + ELBI_SYS_CTRL);
+ val = readl(pci->elbi_base + ELBI_SYS_CTRL);
val |= ELBI_SYS_CTRL_LT_ENABLE;
- writel(val, pcie->elbi + ELBI_SYS_CTRL);
+ writel(val, pci->elbi_base + ELBI_SYS_CTRL);
}
static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
@@ -1861,12 +1865,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
goto err_pm_runtime_put;
}
- pcie->elbi = devm_platform_ioremap_resource_byname(pdev, "elbi");
- if (IS_ERR(pcie->elbi)) {
- ret = PTR_ERR(pcie->elbi);
- goto err_pm_runtime_put;
- }
-
/* MHI region is optional */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mhi");
if (res) {
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 3/5] PCI: dwc: qcom: Switch to dwc ELBI resource mapping
2025-08-28 7:34 ` [PATCH v8 3/5] PCI: dwc: qcom: Switch to dwc " Krishna Chaitanya Chundru
@ 2025-09-03 19:14 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 19:14 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han, linux-arm-msm, devicetree, linux-kernel, linux-pci,
quic_vbadigan, quic_mrana, quic_vpernami, mmareddy, Konrad Dybcio
On Thu, Aug 28, 2025 at 01:04:24PM +0530, Krishna Chaitanya Chundru wrote:
> Instead of using qcom ELBI resources mapping let the DWC core map it
> ELBI is DWC specific.
This seems like basically the same change you (Mani) added to the
"[PATCH v8 2/5] PCI: dwc: Add support for ELBI resource mapping"
patch? (The patch with Mani's additions is at
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc-ecam&id=d39e0103e38f9889271a77a837b6179b42d6730d)
If this qcom change is separated out, why can't the exynos and qcom-ep
changes from patch 2/5 be separated out? If we ever had to bisect
and/or revert parts of this, it seems like it would be simpler to do
them consistently like this:
- PCI: dwc: Add ELBI resource mapping
- PCI: exynos: Switch to dwc ELBI resource mapping
- PCI: qcom: Switch to dwc ELBI resource mapping
- PCI: qcom-ep: Switch to dwc ELBI resource mapping
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..5092752de23866ef95036bb3f8fae9bb06e8ea1e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -276,7 +276,6 @@ struct qcom_pcie_port {
> struct qcom_pcie {
> struct dw_pcie *pci;
> void __iomem *parf; /* DT parf */
> - void __iomem *elbi; /* DT elbi */
> void __iomem *mhi;
> union qcom_pcie_resources res;
> struct phy *phy;
> @@ -414,12 +413,17 @@ static void qcom_pcie_configure_dbi_atu_base(struct qcom_pcie *pcie)
>
> static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie)
> {
> + struct dw_pcie *pci = pcie->pci;
> u32 val;
>
> + if (!pci->elbi_base) {
> + dev_err(pci->dev, "ELBI is not present\n");
> + return;
> + }
> /* enable link training */
> - val = readl(pcie->elbi + ELBI_SYS_CTRL);
> + val = readl(pci->elbi_base + ELBI_SYS_CTRL);
> val |= ELBI_SYS_CTRL_LT_ENABLE;
> - writel(val, pcie->elbi + ELBI_SYS_CTRL);
> + writel(val, pci->elbi_base + ELBI_SYS_CTRL);
> }
>
> static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> @@ -1861,12 +1865,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> goto err_pm_runtime_put;
> }
>
> - pcie->elbi = devm_platform_ioremap_resource_byname(pdev, "elbi");
> - if (IS_ERR(pcie->elbi)) {
> - ret = PTR_ERR(pcie->elbi);
> - goto err_pm_runtime_put;
> - }
> -
> /* MHI region is optional */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mhi");
> if (res) {
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
` (2 preceding siblings ...)
2025-08-28 7:34 ` [PATCH v8 3/5] PCI: dwc: qcom: Switch to dwc " Krishna Chaitanya Chundru
@ 2025-08-28 7:34 ` Krishna Chaitanya Chundru
2025-09-01 13:52 ` Manivannan Sadhasivam
2025-09-03 20:10 ` Bjorn Helgaas
2025-08-28 7:34 ` [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature Krishna Chaitanya Chundru
2025-09-01 13:42 ` (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Manivannan Sadhasivam
5 siblings, 2 replies; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-28 7:34 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Krishna Chaitanya Chundru
The current implementation requires iATU for every configuration
space access which increases latency & cpu utilization.
Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
would be matched against the Base and Limit addresses) of the incoming
CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
Configuring iATU in config shift feature enables ECAM feature to access the
config space, which avoids iATU configuration for every config access.
Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift feature.
As DBI comes under config space, this avoids remapping of DBI space
separately. Instead, it uses the mapped config space address returned from
ECAM initialization. Change the order of dw_pcie_get_resources() execution
to achieve this.
Enable the ECAM feature if the config space size is equal to size required
to represent number of buses in the bus range property.
As per PCIe spec 6, sec 7.2.2 the memory should be aligned to 256MB for
ECAM. The synopsys iATU also uses bits [27:12] to form BDF, so the base
address must be 256MB aligned. Add a check to ensure the configuration
space base address is 256MB aligned before enabling ECAM.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/Kconfig | 1 +
drivers/pci/controller/dwc/pcie-designware-host.c | 145 +++++++++++++++++++---
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 5 +
4 files changed, 138 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index ff6b6d9e18ecfa44273e87931551f9e63fbe3cba..a0e7ad3fb5afec63b0f919732a50147229623186 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -20,6 +20,7 @@ config PCIE_DW_HOST
bool
select PCIE_DW
select IRQ_MSI_LIB
+ select PCI_HOST_COMMON
config PCIE_DW_EP
bool
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..eda7affcdcb2075d07ba6eeab70e41b6548a4b18 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -8,6 +8,7 @@
* Author: Jingoo Han <jg1.han@samsung.com>
*/
+#include <linux/align.h>
#include <linux/iopoll.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/irqchip/irq-msi-lib.h>
@@ -32,6 +33,8 @@ static struct pci_ops dw_child_pcie_ops;
MSI_FLAG_PCI_MSIX | \
MSI_GENERIC_FLAGS_MASK)
+#define IS_256MB_ALIGNED(x) IS_ALIGNED(x, SZ_256M)
+
static const struct msi_parent_ops dw_pcie_msi_parent_ops = {
.required_flags = DW_PCIE_MSI_FLAGS_REQUIRED,
.supported_flags = DW_PCIE_MSI_FLAGS_SUPPORTED,
@@ -413,6 +416,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
}
}
+static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct dw_pcie_ob_atu_cfg atu = {0};
+ resource_size_t bus_range_max;
+ struct resource_entry *bus;
+ int ret;
+
+ bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
+
+ /*
+ * Root bus under the host bridge doesn't require any iATU configuration
+ * as DBI region will be used to access root bus config space.
+ * Immediate bus under Root Bus, needs type 0 iATU configuration and
+ * remaining buses need type 1 iATU configuration.
+ */
+ atu.index = 0;
+ atu.type = PCIE_ATU_TYPE_CFG0;
+ atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
+ /* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
+ atu.size = SZ_1M;
+ atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
+ ret = dw_pcie_prog_outbound_atu(pci, &atu);
+ if (ret)
+ return ret;
+
+ bus_range_max = resource_size(bus->res);
+
+ if (bus_range_max < 2)
+ return 0;
+
+ /* Configure remaining buses in type 1 iATU configuration */
+ atu.index = 1;
+ atu.type = PCIE_ATU_TYPE_CFG1;
+ atu.parent_bus_addr = pp->cfg0_base + SZ_2M;
+ atu.size = (SZ_1M * bus_range_max) - SZ_2M;
+ atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
+
+ return dw_pcie_prog_outbound_atu(pci, &atu);
+}
+
+static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct device *dev = pci->dev;
+ struct resource_entry *bus;
+
+ bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
+ if (!bus)
+ return -ENODEV;
+
+ pp->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
+ if (IS_ERR(pp->cfg))
+ return PTR_ERR(pp->cfg);
+
+ pci->dbi_base = pp->cfg->win;
+ pci->dbi_phys_addr = res->start;
+
+ return 0;
+}
+
+static bool dw_pcie_ecam_enabled(struct dw_pcie_rp *pp, struct resource *config_res)
+{
+ struct resource *bus_range;
+ u64 nr_buses;
+
+ /*
+ * 256MB alignment is required for Enhanced Configuration Address Mapping (ECAM),
+ * as per PCIe Spec 6, Sec 7.2.2. It ensures proper mapping of memory addresses
+ * to Bus-Device-Function (BDF) fields in config TLPs.
+ *
+ * The synopsys iATU also uses bits [27:12] to form BDF, so the base address must
+ * be 256MB aligned.
+ */
+ if (!IS_256MB_ALIGNED(config_res->start))
+ return false;
+
+ bus_range = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res;
+ if (!bus_range)
+ return false;
+
+ nr_buses = resource_size(config_res) >> PCIE_ECAM_BUS_SHIFT;
+
+ return !!(nr_buses >= resource_size(bus_range));
+}
+
static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -422,10 +511,6 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
struct resource *res;
int ret;
- ret = dw_pcie_get_resources(pci);
- if (ret)
- return ret;
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
if (!res) {
dev_err(dev, "Missing \"config\" reg space\n");
@@ -435,9 +520,32 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
pp->cfg0_size = resource_size(res);
pp->cfg0_base = res->start;
- pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
- if (IS_ERR(pp->va_cfg0_base))
- return PTR_ERR(pp->va_cfg0_base);
+ pp->ecam_enabled = dw_pcie_ecam_enabled(pp, res);
+ if (pp->ecam_enabled) {
+ ret = dw_pcie_create_ecam_window(pp, res);
+ if (ret)
+ return ret;
+
+ pp->bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
+ pp->bridge->sysdata = pp->cfg;
+ pp->cfg->priv = pp;
+ } else {
+ pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
+ if (IS_ERR(pp->va_cfg0_base))
+ return PTR_ERR(pp->va_cfg0_base);
+
+ /* Set default bus ops */
+ pp->bridge->ops = &dw_pcie_ops;
+ pp->bridge->child_ops = &dw_child_pcie_ops;
+ pp->bridge->sysdata = pp;
+ }
+
+ ret = dw_pcie_get_resources(pci);
+ if (ret) {
+ if (pp->cfg)
+ pci_ecam_free(pp->cfg);
+ return ret;
+ }
/* Get the I/O range from DT */
win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_IO);
@@ -476,14 +584,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
if (ret)
return ret;
- /* Set default bus ops */
- bridge->ops = &dw_pcie_ops;
- bridge->child_ops = &dw_child_pcie_ops;
-
if (pp->ops->init) {
ret = pp->ops->init(pp);
if (ret)
- return ret;
+ goto err_free_ecam;
}
if (pci_msi_enabled()) {
@@ -525,6 +629,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
if (ret)
goto err_free_msi;
+ if (pp->ecam_enabled) {
+ ret = dw_pcie_config_ecam_iatu(pp);
+ if (ret) {
+ dev_err(dev, "Failed to configure iATU in ECAM mode\n");
+ goto err_free_msi;
+ }
+ }
+
/*
* Allocate the resource for MSG TLP before programming the iATU
* outbound window in dw_pcie_setup_rc(). Since the allocation depends
@@ -560,8 +672,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
/* Ignore errors, the link may come up later */
dw_pcie_wait_for_link(pci);
- bridge->sysdata = pp;
-
ret = pci_host_probe(bridge);
if (ret)
goto err_stop_link;
@@ -587,6 +697,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
if (pp->ops->deinit)
pp->ops->deinit(pp);
+err_free_ecam:
+ if (pp->cfg)
+ pci_ecam_free(pp->cfg);
+
return ret;
}
EXPORT_SYMBOL_GPL(dw_pcie_host_init);
@@ -609,6 +723,9 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
if (pp->ops->deinit)
pp->ops->deinit(pp);
+
+ if (pp->cfg)
+ pci_ecam_free(pp->cfg);
}
EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 4684c671a81bee468f686a83cc992433b38af59d..6826ddb9478d41227fa011018cffa8d2242336a9 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -576,7 +576,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
val = dw_pcie_enable_ecrc(val);
dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
- val = PCIE_ATU_ENABLE;
+ val = PCIE_ATU_ENABLE | atu->ctrl2;
if (atu->type == PCIE_ATU_TYPE_MSG) {
/* The data-less messages only for now */
val |= PCIE_ATU_INHIBIT_PAYLOAD | atu->code;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ceb022506c3191cd8fe580411526e20cc3758fed..f770e160ce7c538e0835e7cf80bae9ed099f906c 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -20,6 +20,7 @@
#include <linux/irq.h>
#include <linux/msi.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/reset.h>
#include <linux/pci-epc.h>
@@ -169,6 +170,7 @@
#define PCIE_ATU_REGION_CTRL2 0x004
#define PCIE_ATU_ENABLE BIT(31)
#define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
+#define PCIE_ATU_CFG_SHIFT_MODE_ENABLE BIT(28)
#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22)
#define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
#define PCIE_ATU_LOWER_BASE 0x008
@@ -387,6 +389,7 @@ struct dw_pcie_ob_atu_cfg {
u8 func_no;
u8 code;
u8 routing;
+ u32 ctrl2;
u64 parent_bus_addr;
u64 pci_addr;
u64 size;
@@ -425,6 +428,8 @@ struct dw_pcie_rp {
struct resource *msg_res;
bool use_linkup_irq;
struct pci_eq_presets presets;
+ bool ecam_enabled;
+ struct pci_config_window *cfg;
};
struct dw_pcie_ep_ops {
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration
2025-08-28 7:34 ` [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
@ 2025-09-01 13:52 ` Manivannan Sadhasivam
2025-09-03 20:10 ` Bjorn Helgaas
1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01 13:52 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy
On Thu, Aug 28, 2025 at 01:04:25PM GMT, Krishna Chaitanya Chundru wrote:
> The current implementation requires iATU for every configuration
> space access which increases latency & cpu utilization.
>
> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> would be matched against the Base and Limit addresses) of the incoming
> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>
> Configuring iATU in config shift feature enables ECAM feature to access the
> config space, which avoids iATU configuration for every config access.
>
> Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift feature.
>
> As DBI comes under config space, this avoids remapping of DBI space
> separately. Instead, it uses the mapped config space address returned from
> ECAM initialization. Change the order of dw_pcie_get_resources() execution
> to achieve this.
>
> Enable the ECAM feature if the config space size is equal to size required
> to represent number of buses in the bus range property.
>
> As per PCIe spec 6, sec 7.2.2 the memory should be aligned to 256MB for
> ECAM. The synopsys iATU also uses bits [27:12] to form BDF, so the base
> address must be 256MB aligned. Add a check to ensure the configuration
> space base address is 256MB aligned before enabling ECAM.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-designware-host.c | 145 +++++++++++++++++++---
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 5 +
> 4 files changed, 138 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index ff6b6d9e18ecfa44273e87931551f9e63fbe3cba..a0e7ad3fb5afec63b0f919732a50147229623186 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -20,6 +20,7 @@ config PCIE_DW_HOST
> bool
> select PCIE_DW
> select IRQ_MSI_LIB
> + select PCI_HOST_COMMON
>
> config PCIE_DW_EP
> bool
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..eda7affcdcb2075d07ba6eeab70e41b6548a4b18 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -8,6 +8,7 @@
> * Author: Jingoo Han <jg1.han@samsung.com>
> */
>
> +#include <linux/align.h>
> #include <linux/iopoll.h>
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqchip/irq-msi-lib.h>
> @@ -32,6 +33,8 @@ static struct pci_ops dw_child_pcie_ops;
> MSI_FLAG_PCI_MSIX | \
> MSI_GENERIC_FLAGS_MASK)
>
> +#define IS_256MB_ALIGNED(x) IS_ALIGNED(x, SZ_256M)
> +
> static const struct msi_parent_ops dw_pcie_msi_parent_ops = {
> .required_flags = DW_PCIE_MSI_FLAGS_REQUIRED,
> .supported_flags = DW_PCIE_MSI_FLAGS_SUPPORTED,
> @@ -413,6 +416,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> }
> }
>
> +static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct dw_pcie_ob_atu_cfg atu = {0};
> + resource_size_t bus_range_max;
> + struct resource_entry *bus;
> + int ret;
> +
> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> +
> + /*
> + * Root bus under the host bridge doesn't require any iATU configuration
> + * as DBI region will be used to access root bus config space.
> + * Immediate bus under Root Bus, needs type 0 iATU configuration and
> + * remaining buses need type 1 iATU configuration.
> + */
> + atu.index = 0;
> + atu.type = PCIE_ATU_TYPE_CFG0;
> + atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
> + /* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
> + atu.size = SZ_1M;
> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
> + ret = dw_pcie_prog_outbound_atu(pci, &atu);
> + if (ret)
> + return ret;
> +
> + bus_range_max = resource_size(bus->res);
> +
> + if (bus_range_max < 2)
> + return 0;
> +
> + /* Configure remaining buses in type 1 iATU configuration */
> + atu.index = 1;
> + atu.type = PCIE_ATU_TYPE_CFG1;
> + atu.parent_bus_addr = pp->cfg0_base + SZ_2M;
> + atu.size = (SZ_1M * bus_range_max) - SZ_2M;
> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
> +
> + return dw_pcie_prog_outbound_atu(pci, &atu);
> +}
> +
> +static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct device *dev = pci->dev;
> + struct resource_entry *bus;
> +
> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> + if (!bus)
> + return -ENODEV;
> +
> + pp->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
> + if (IS_ERR(pp->cfg))
> + return PTR_ERR(pp->cfg);
> +
> + pci->dbi_base = pp->cfg->win;
> + pci->dbi_phys_addr = res->start;
> +
> + return 0;
> +}
> +
> +static bool dw_pcie_ecam_enabled(struct dw_pcie_rp *pp, struct resource *config_res)
> +{
> + struct resource *bus_range;
> + u64 nr_buses;
> +
> + /*
> + * 256MB alignment is required for Enhanced Configuration Address Mapping (ECAM),
> + * as per PCIe Spec 6, Sec 7.2.2. It ensures proper mapping of memory addresses
> + * to Bus-Device-Function (BDF) fields in config TLPs.
> + *
> + * The synopsys iATU also uses bits [27:12] to form BDF, so the base address must
> + * be 256MB aligned.
I've reworded the comment and description to clarify that the alignment
requirement comes from the PCIe spec, but the 256 MiB requirement comes from DWC
implementation since DWC always uses 8 bits for representing PCIe buses.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration
2025-08-28 7:34 ` [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
2025-09-01 13:52 ` Manivannan Sadhasivam
@ 2025-09-03 20:10 ` Bjorn Helgaas
1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 20:10 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han, linux-arm-msm, devicetree, linux-kernel, linux-pci,
quic_vbadigan, quic_mrana, quic_vpernami, mmareddy,
Jonathan Chocron
[+cc Jonathan for potential issue in pcie-al.c]
On Thu, Aug 28, 2025 at 01:04:25PM +0530, Krishna Chaitanya Chundru wrote:
> The current implementation requires iATU for every configuration
> space access which increases latency & cpu utilization.
>
> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> would be matched against the Base and Limit addresses) of the incoming
> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>
> Configuring iATU in config shift feature enables ECAM feature to access the
> config space, which avoids iATU configuration for every config access.
>
> Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift feature.
>
> As DBI comes under config space, this avoids remapping of DBI space
> separately. Instead, it uses the mapped config space address returned from
> ECAM initialization. Change the order of dw_pcie_get_resources() execution
> to achieve this.
>
> Enable the ECAM feature if the config space size is equal to size required
> to represent number of buses in the bus range property.
>
> As per PCIe spec 6, sec 7.2.2 the memory should be aligned to 256MB for
> ECAM. The synopsys iATU also uses bits [27:12] to form BDF, so the base
> address must be 256MB aligned. Add a check to ensure the configuration
> space base address is 256MB aligned before enabling ECAM.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-designware-host.c | 145 +++++++++++++++++++---
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 5 +
> 4 files changed, 138 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index ff6b6d9e18ecfa44273e87931551f9e63fbe3cba..a0e7ad3fb5afec63b0f919732a50147229623186 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -20,6 +20,7 @@ config PCIE_DW_HOST
> bool
> select PCIE_DW
> select IRQ_MSI_LIB
> + select PCI_HOST_COMMON
>
> config PCIE_DW_EP
> bool
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..eda7affcdcb2075d07ba6eeab70e41b6548a4b18 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -8,6 +8,7 @@
> * Author: Jingoo Han <jg1.han@samsung.com>
> */
>
> +#include <linux/align.h>
> #include <linux/iopoll.h>
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqchip/irq-msi-lib.h>
> @@ -32,6 +33,8 @@ static struct pci_ops dw_child_pcie_ops;
> MSI_FLAG_PCI_MSIX | \
> MSI_GENERIC_FLAGS_MASK)
>
> +#define IS_256MB_ALIGNED(x) IS_ALIGNED(x, SZ_256M)
> +
> static const struct msi_parent_ops dw_pcie_msi_parent_ops = {
> .required_flags = DW_PCIE_MSI_FLAGS_REQUIRED,
> .supported_flags = DW_PCIE_MSI_FLAGS_SUPPORTED,
> @@ -413,6 +416,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> }
> }
>
> +static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct dw_pcie_ob_atu_cfg atu = {0};
> + resource_size_t bus_range_max;
> + struct resource_entry *bus;
> + int ret;
> +
> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> +
> + /*
> + * Root bus under the host bridge doesn't require any iATU configuration
> + * as DBI region will be used to access root bus config space.
> + * Immediate bus under Root Bus, needs type 0 iATU configuration and
> + * remaining buses need type 1 iATU configuration.
> + */
> + atu.index = 0;
> + atu.type = PCIE_ATU_TYPE_CFG0;
> + atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
> + /* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
> + atu.size = SZ_1M;
> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
> + ret = dw_pcie_prog_outbound_atu(pci, &atu);
> + if (ret)
> + return ret;
> +
> + bus_range_max = resource_size(bus->res);
> +
> + if (bus_range_max < 2)
> + return 0;
> +
> + /* Configure remaining buses in type 1 iATU configuration */
> + atu.index = 1;
> + atu.type = PCIE_ATU_TYPE_CFG1;
> + atu.parent_bus_addr = pp->cfg0_base + SZ_2M;
> + atu.size = (SZ_1M * bus_range_max) - SZ_2M;
> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
> +
> + return dw_pcie_prog_outbound_atu(pci, &atu);
> +}
> +
> +static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct device *dev = pci->dev;
> + struct resource_entry *bus;
> +
> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> + if (!bus)
> + return -ENODEV;
> +
> + pp->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
> + if (IS_ERR(pp->cfg))
> + return PTR_ERR(pp->cfg);
> +
> + pci->dbi_base = pp->cfg->win;
> + pci->dbi_phys_addr = res->start;
> +
> + return 0;
> +}
> +
> +static bool dw_pcie_ecam_enabled(struct dw_pcie_rp *pp, struct resource *config_res)
> +{
> + struct resource *bus_range;
> + u64 nr_buses;
> +
> + /*
> + * 256MB alignment is required for Enhanced Configuration Address Mapping (ECAM),
> + * as per PCIe Spec 6, Sec 7.2.2. It ensures proper mapping of memory addresses
> + * to Bus-Device-Function (BDF) fields in config TLPs.
> + *
> + * The synopsys iATU also uses bits [27:12] to form BDF, so the base address must
> + * be 256MB aligned.
> + */
> + if (!IS_256MB_ALIGNED(config_res->start))
> + return false;
> +
> + bus_range = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res;
> + if (!bus_range)
> + return false;
> +
> + nr_buses = resource_size(config_res) >> PCIE_ECAM_BUS_SHIFT;
> +
> + return !!(nr_buses >= resource_size(bus_range));
> +}
> +
> static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -422,10 +511,6 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
> struct resource *res;
> int ret;
>
> - ret = dw_pcie_get_resources(pci);
> - if (ret)
> - return ret;
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> if (!res) {
> dev_err(dev, "Missing \"config\" reg space\n");
> @@ -435,9 +520,32 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
> pp->cfg0_size = resource_size(res);
> pp->cfg0_base = res->start;
>
> - pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
> - if (IS_ERR(pp->va_cfg0_base))
> - return PTR_ERR(pp->va_cfg0_base);
> + pp->ecam_enabled = dw_pcie_ecam_enabled(pp, res);
> + if (pp->ecam_enabled) {
> + ret = dw_pcie_create_ecam_window(pp, res);
> + if (ret)
> + return ret;
> +
> + pp->bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
> + pp->bridge->sysdata = pp->cfg;
> + pp->cfg->priv = pp;
> + } else {
> + pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
Now we only set va_cfg0_base when dw_pcie_ecam_enabled() returned
false. dw_pcie_ecam_enabled() only depends on the "config" resource
alignment and size, which come straight from the devicetree.
Doesn't that mean that other users of va_cfg0_base (keystone and al)
will potentially be broken if their devicetree has a "config" resource
that causes dw_pcie_ecam_enabled() to return true?
> + if (IS_ERR(pp->va_cfg0_base))
> + return PTR_ERR(pp->va_cfg0_base);
> +
> + /* Set default bus ops */
> + pp->bridge->ops = &dw_pcie_ops;
> + pp->bridge->child_ops = &dw_child_pcie_ops;
> + pp->bridge->sysdata = pp;
> + }
> +
> + ret = dw_pcie_get_resources(pci);
> + if (ret) {
> + if (pp->cfg)
> + pci_ecam_free(pp->cfg);
> + return ret;
> + }
>
> /* Get the I/O range from DT */
> win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_IO);
> @@ -476,14 +584,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret)
> return ret;
>
> - /* Set default bus ops */
> - bridge->ops = &dw_pcie_ops;
> - bridge->child_ops = &dw_child_pcie_ops;
> -
> if (pp->ops->init) {
> ret = pp->ops->init(pp);
> if (ret)
> - return ret;
> + goto err_free_ecam;
> }
>
> if (pci_msi_enabled()) {
> @@ -525,6 +629,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret)
> goto err_free_msi;
>
> + if (pp->ecam_enabled) {
> + ret = dw_pcie_config_ecam_iatu(pp);
> + if (ret) {
> + dev_err(dev, "Failed to configure iATU in ECAM mode\n");
> + goto err_free_msi;
> + }
> + }
> +
> /*
> * Allocate the resource for MSG TLP before programming the iATU
> * outbound window in dw_pcie_setup_rc(). Since the allocation depends
> @@ -560,8 +672,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> /* Ignore errors, the link may come up later */
> dw_pcie_wait_for_link(pci);
>
> - bridge->sysdata = pp;
> -
> ret = pci_host_probe(bridge);
> if (ret)
> goto err_stop_link;
> @@ -587,6 +697,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> if (pp->ops->deinit)
> pp->ops->deinit(pp);
>
> +err_free_ecam:
> + if (pp->cfg)
> + pci_ecam_free(pp->cfg);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(dw_pcie_host_init);
> @@ -609,6 +723,9 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>
> if (pp->ops->deinit)
> pp->ops->deinit(pp);
> +
> + if (pp->cfg)
> + pci_ecam_free(pp->cfg);
> }
> EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 4684c671a81bee468f686a83cc992433b38af59d..6826ddb9478d41227fa011018cffa8d2242336a9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -576,7 +576,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
> val = dw_pcie_enable_ecrc(val);
> dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
>
> - val = PCIE_ATU_ENABLE;
> + val = PCIE_ATU_ENABLE | atu->ctrl2;
> if (atu->type == PCIE_ATU_TYPE_MSG) {
> /* The data-less messages only for now */
> val |= PCIE_ATU_INHIBIT_PAYLOAD | atu->code;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ceb022506c3191cd8fe580411526e20cc3758fed..f770e160ce7c538e0835e7cf80bae9ed099f906c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -20,6 +20,7 @@
> #include <linux/irq.h>
> #include <linux/msi.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/reset.h>
>
> #include <linux/pci-epc.h>
> @@ -169,6 +170,7 @@
> #define PCIE_ATU_REGION_CTRL2 0x004
> #define PCIE_ATU_ENABLE BIT(31)
> #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> +#define PCIE_ATU_CFG_SHIFT_MODE_ENABLE BIT(28)
> #define PCIE_ATU_INHIBIT_PAYLOAD BIT(22)
> #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
> #define PCIE_ATU_LOWER_BASE 0x008
> @@ -387,6 +389,7 @@ struct dw_pcie_ob_atu_cfg {
> u8 func_no;
> u8 code;
> u8 routing;
> + u32 ctrl2;
> u64 parent_bus_addr;
> u64 pci_addr;
> u64 size;
> @@ -425,6 +428,8 @@ struct dw_pcie_rp {
> struct resource *msg_res;
> bool use_linkup_irq;
> struct pci_eq_presets presets;
> + bool ecam_enabled;
> + struct pci_config_window *cfg;
> };
>
> struct dw_pcie_ep_ops {
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
` (3 preceding siblings ...)
2025-08-28 7:34 ` [PATCH v8 4/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
@ 2025-08-28 7:34 ` Krishna Chaitanya Chundru
2025-09-03 19:57 ` Bjorn Helgaas
2025-09-03 20:12 ` Bjorn Helgaas
2025-09-01 13:42 ` (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Manivannan Sadhasivam
5 siblings, 2 replies; 18+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-28 7:34 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Krishna Chaitanya Chundru
The ELBI registers falls after the DBI space, PARF_SLV_DBI_ELBI register
gives us the offset from which ELBI starts. So override ELBI with the
offset from PARF_SLV_DBI_ELBI and cfg win to map these regions.
On root bus, we have only the root port. Any access other than that
should not go out of the link and should return all F's. Since the iATU
is configured for the buses which starts after root bus, block the
transactions starting from function 1 of the root bus to the end of
the root bus (i.e from dbi_base + 4kb to dbi_base + 1MB) from going
outside the link through ECAM blocker through PARF registers.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 70 ++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5092752de23866ef95036bb3f8fae9bb06e8ea1e..8f3c86c77e2604fd7826083f63b66b4cb62a341d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -55,6 +55,7 @@
#define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8
#define PARF_Q2A_FLUSH 0x1ac
#define PARF_LTSSM 0x1b0
+#define PARF_SLV_DBI_ELBI 0x1b4
#define PARF_INT_ALL_STATUS 0x224
#define PARF_INT_ALL_CLEAR 0x228
#define PARF_INT_ALL_MASK 0x22c
@@ -64,6 +65,16 @@
#define PARF_DBI_BASE_ADDR_V2_HI 0x354
#define PARF_SLV_ADDR_SPACE_SIZE_V2 0x358
#define PARF_SLV_ADDR_SPACE_SIZE_V2_HI 0x35c
+#define PARF_BLOCK_SLV_AXI_WR_BASE 0x360
+#define PARF_BLOCK_SLV_AXI_WR_BASE_HI 0x364
+#define PARF_BLOCK_SLV_AXI_WR_LIMIT 0x368
+#define PARF_BLOCK_SLV_AXI_WR_LIMIT_HI 0x36c
+#define PARF_BLOCK_SLV_AXI_RD_BASE 0x370
+#define PARF_BLOCK_SLV_AXI_RD_BASE_HI 0x374
+#define PARF_BLOCK_SLV_AXI_RD_LIMIT 0x378
+#define PARF_BLOCK_SLV_AXI_RD_LIMIT_HI 0x37c
+#define PARF_ECAM_BASE 0x380
+#define PARF_ECAM_BASE_HI 0x384
#define PARF_NO_SNOOP_OVERRIDE 0x3d4
#define PARF_ATU_BASE_ADDR 0x634
#define PARF_ATU_BASE_ADDR_HI 0x638
@@ -87,6 +98,7 @@
/* PARF_SYS_CTRL register fields */
#define MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN BIT(29)
+#define PCIE_ECAM_BLOCKER_EN BIT(26)
#define MST_WAKEUP_EN BIT(13)
#define SLV_WAKEUP_EN BIT(12)
#define MSTR_ACLK_CGC_DIS BIT(10)
@@ -134,6 +146,9 @@
/* PARF_LTSSM register fields */
#define LTSSM_EN BIT(8)
+/* PARF_SLV_DBI_ELBI */
+#define SLV_DBI_ELBI_ADDR_BASE GENMASK(11, 0)
+
/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
#define PARF_INT_ALL_LINK_UP BIT(13)
#define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)
@@ -317,6 +332,48 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
qcom_perst_assert(pcie, false);
}
+static void qcom_pci_config_ecam(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct qcom_pcie *pcie = to_qcom_pcie(pci);
+ u64 addr, addr_end;
+ u32 val;
+
+ /* Set the ECAM base */
+ writel_relaxed(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
+ writel_relaxed(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
+
+ /*
+ * The only device on root bus is the Root Port. Any access to the PCIe
+ * region will go outside the PCIe link. As part of enumeration the PCI
+ * sw can try to read to vendor ID & device ID with different device
+ * number and function number under root bus. As any access other than
+ * root bus, device 0, function 0, should not go out of the link and
+ * should return all F's. Since the iATU is configured for the buses
+ * which starts after root bus, block the transactions starting from
+ * function 1 of the root bus to the end of the root bus (i.e from
+ * dbi_base + 4kb to dbi_base + 1MB) from going outside the link.
+ */
+ addr = pci->dbi_phys_addr + SZ_4K;
+ writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
+ writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);
+
+ writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
+ writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
+
+ addr_end = pci->dbi_phys_addr + SZ_1M - 1;
+
+ writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
+ writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
+
+ writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
+ writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
+
+ val = readl_relaxed(pcie->parf + PARF_SYS_CTRL);
+ val |= PCIE_ECAM_BLOCKER_EN;
+ writel_relaxed(val, pcie->parf + PARF_SYS_CTRL);
+}
+
static int qcom_pcie_start_link(struct dw_pcie *pci)
{
struct qcom_pcie *pcie = to_qcom_pcie(pci);
@@ -326,6 +383,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
qcom_pcie_common_set_16gt_lane_margining(pci);
}
+ if (pci->pp.ecam_enabled)
+ qcom_pci_config_ecam(&pci->pp);
+
/* Enable Link Training state machine */
if (pcie->cfg->ops->ltssm_enable)
pcie->cfg->ops->ltssm_enable(pcie);
@@ -1314,6 +1374,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct qcom_pcie *pcie = to_qcom_pcie(pci);
+ u16 offset;
int ret;
qcom_ep_reset_assert(pcie);
@@ -1322,6 +1383,15 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
if (ret)
return ret;
+ if (pp->ecam_enabled) {
+ /*
+ * Override ELBI when ECAM is enabled, as when ECAM
+ * is enabled ELBI moves along with the dbi config space.
+ */
+ offset = FIELD_GET(SLV_DBI_ELBI_ADDR_BASE, readl(pcie->parf + PARF_SLV_DBI_ELBI));
+ pci->elbi_base = pci->dbi_base + offset;
+ }
+
ret = qcom_pcie_phy_power_on(pcie);
if (ret)
goto err_deinit;
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature
2025-08-28 7:34 ` [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature Krishna Chaitanya Chundru
@ 2025-09-03 19:57 ` Bjorn Helgaas
2025-09-03 20:12 ` Bjorn Helgaas
1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 19:57 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han, linux-arm-msm, devicetree, linux-kernel, linux-pci,
quic_vbadigan, quic_mrana, quic_vpernami, mmareddy
On Thu, Aug 28, 2025 at 01:04:26PM +0530, Krishna Chaitanya Chundru wrote:
> The ELBI registers falls after the DBI space, PARF_SLV_DBI_ELBI register
> gives us the offset from which ELBI starts. So override ELBI with the
> offset from PARF_SLV_DBI_ELBI and cfg win to map these regions.
>
> On root bus, we have only the root port. Any access other than that
> should not go out of the link and should return all F's. Since the iATU
> is configured for the buses which starts after root bus, block the
> transactions starting from function 1 of the root bus to the end of
> the root bus (i.e from dbi_base + 4kb to dbi_base + 1MB) from going
> outside the link through ECAM blocker through PARF registers.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 70 ++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5092752de23866ef95036bb3f8fae9bb06e8ea1e..8f3c86c77e2604fd7826083f63b66b4cb62a341d 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -55,6 +55,7 @@
> #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8
> #define PARF_Q2A_FLUSH 0x1ac
> #define PARF_LTSSM 0x1b0
> +#define PARF_SLV_DBI_ELBI 0x1b4
> #define PARF_INT_ALL_STATUS 0x224
> #define PARF_INT_ALL_CLEAR 0x228
> #define PARF_INT_ALL_MASK 0x22c
> @@ -64,6 +65,16 @@
> #define PARF_DBI_BASE_ADDR_V2_HI 0x354
> #define PARF_SLV_ADDR_SPACE_SIZE_V2 0x358
> #define PARF_SLV_ADDR_SPACE_SIZE_V2_HI 0x35c
> +#define PARF_BLOCK_SLV_AXI_WR_BASE 0x360
> +#define PARF_BLOCK_SLV_AXI_WR_BASE_HI 0x364
> +#define PARF_BLOCK_SLV_AXI_WR_LIMIT 0x368
> +#define PARF_BLOCK_SLV_AXI_WR_LIMIT_HI 0x36c
> +#define PARF_BLOCK_SLV_AXI_RD_BASE 0x370
> +#define PARF_BLOCK_SLV_AXI_RD_BASE_HI 0x374
> +#define PARF_BLOCK_SLV_AXI_RD_LIMIT 0x378
> +#define PARF_BLOCK_SLV_AXI_RD_LIMIT_HI 0x37c
> +#define PARF_ECAM_BASE 0x380
> +#define PARF_ECAM_BASE_HI 0x384
> #define PARF_NO_SNOOP_OVERRIDE 0x3d4
> #define PARF_ATU_BASE_ADDR 0x634
> #define PARF_ATU_BASE_ADDR_HI 0x638
> @@ -87,6 +98,7 @@
>
> /* PARF_SYS_CTRL register fields */
> #define MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN BIT(29)
> +#define PCIE_ECAM_BLOCKER_EN BIT(26)
> #define MST_WAKEUP_EN BIT(13)
> #define SLV_WAKEUP_EN BIT(12)
> #define MSTR_ACLK_CGC_DIS BIT(10)
> @@ -134,6 +146,9 @@
> /* PARF_LTSSM register fields */
> #define LTSSM_EN BIT(8)
>
> +/* PARF_SLV_DBI_ELBI */
> +#define SLV_DBI_ELBI_ADDR_BASE GENMASK(11, 0)
> +
> /* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
> #define PARF_INT_ALL_LINK_UP BIT(13)
> #define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)
> @@ -317,6 +332,48 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> qcom_perst_assert(pcie, false);
> }
>
> +static void qcom_pci_config_ecam(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct qcom_pcie *pcie = to_qcom_pcie(pci);
> + u64 addr, addr_end;
> + u32 val;
> +
> + /* Set the ECAM base */
> + writel_relaxed(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
> + writel_relaxed(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
> +
> + /*
> + * The only device on root bus is the Root Port. Any access to the PCIe
> + * region will go outside the PCIe link. As part of enumeration the PCI
> + * sw can try to read to vendor ID & device ID with different device
> + * number and function number under root bus. As any access other than
> + * root bus, device 0, function 0, should not go out of the link and
> + * should return all F's. Since the iATU is configured for the buses
> + * which starts after root bus, block the transactions starting from
> + * function 1 of the root bus to the end of the root bus (i.e from
> + * dbi_base + 4kb to dbi_base + 1MB) from going outside the link.
> + */
> + addr = pci->dbi_phys_addr + SZ_4K;
> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);
> +
> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
> +
> + addr_end = pci->dbi_phys_addr + SZ_1M - 1;
I guess this is an implicit restriction to a single Root Port on the
root bus at bb:00.0, right? So when the qcom IP eventually supports
multiple Root Ports or even a single Root Port at a different
device/function number, this would have to be updated somehow?
No need to change anything here; just making sure I understand what's
going on.
> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
> +
> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
> +
> + val = readl_relaxed(pcie->parf + PARF_SYS_CTRL);
> + val |= PCIE_ECAM_BLOCKER_EN;
> + writel_relaxed(val, pcie->parf + PARF_SYS_CTRL);
> +}
> +
> static int qcom_pcie_start_link(struct dw_pcie *pci)
> {
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -326,6 +383,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
> qcom_pcie_common_set_16gt_lane_margining(pci);
> }
>
> + if (pci->pp.ecam_enabled)
> + qcom_pci_config_ecam(&pci->pp);
> +
> /* Enable Link Training state machine */
> if (pcie->cfg->ops->ltssm_enable)
> pcie->cfg->ops->ltssm_enable(pcie);
> @@ -1314,6 +1374,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> + u16 offset;
> int ret;
>
> qcom_ep_reset_assert(pcie);
> @@ -1322,6 +1383,15 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret)
> return ret;
>
> + if (pp->ecam_enabled) {
> + /*
> + * Override ELBI when ECAM is enabled, as when ECAM
> + * is enabled ELBI moves along with the dbi config space.
> + */
> + offset = FIELD_GET(SLV_DBI_ELBI_ADDR_BASE, readl(pcie->parf + PARF_SLV_DBI_ELBI));
> + pci->elbi_base = pci->dbi_base + offset;
> + }
> +
> ret = qcom_pcie_phy_power_on(pcie);
> if (ret)
> goto err_deinit;
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature
2025-08-28 7:34 ` [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature Krishna Chaitanya Chundru
2025-09-03 19:57 ` Bjorn Helgaas
@ 2025-09-03 20:12 ` Bjorn Helgaas
1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 20:12 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Manivannan Sadhasivam, Krzysztof Wilczyński, Bjorn Helgaas,
Jingoo Han, linux-arm-msm, devicetree, linux-kernel, linux-pci,
quic_vbadigan, quic_mrana, quic_vpernami, mmareddy
On Thu, Aug 28, 2025 at 01:04:26PM +0530, Krishna Chaitanya Chundru wrote:
> The ELBI registers falls after the DBI space, PARF_SLV_DBI_ELBI register
> gives us the offset from which ELBI starts. So override ELBI with the
> offset from PARF_SLV_DBI_ELBI and cfg win to map these regions.
>
> On root bus, we have only the root port. Any access other than that
> should not go out of the link and should return all F's. Since the iATU
> is configured for the buses which starts after root bus, block the
> transactions starting from function 1 of the root bus to the end of
> the root bus (i.e from dbi_base + 4kb to dbi_base + 1MB) from going
> outside the link through ECAM blocker through PARF registers.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 70 ++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5092752de23866ef95036bb3f8fae9bb06e8ea1e..8f3c86c77e2604fd7826083f63b66b4cb62a341d 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -55,6 +55,7 @@
> #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8
> #define PARF_Q2A_FLUSH 0x1ac
> #define PARF_LTSSM 0x1b0
> +#define PARF_SLV_DBI_ELBI 0x1b4
> #define PARF_INT_ALL_STATUS 0x224
> #define PARF_INT_ALL_CLEAR 0x228
> #define PARF_INT_ALL_MASK 0x22c
> @@ -64,6 +65,16 @@
> #define PARF_DBI_BASE_ADDR_V2_HI 0x354
> #define PARF_SLV_ADDR_SPACE_SIZE_V2 0x358
> #define PARF_SLV_ADDR_SPACE_SIZE_V2_HI 0x35c
> +#define PARF_BLOCK_SLV_AXI_WR_BASE 0x360
> +#define PARF_BLOCK_SLV_AXI_WR_BASE_HI 0x364
> +#define PARF_BLOCK_SLV_AXI_WR_LIMIT 0x368
> +#define PARF_BLOCK_SLV_AXI_WR_LIMIT_HI 0x36c
> +#define PARF_BLOCK_SLV_AXI_RD_BASE 0x370
> +#define PARF_BLOCK_SLV_AXI_RD_BASE_HI 0x374
> +#define PARF_BLOCK_SLV_AXI_RD_LIMIT 0x378
> +#define PARF_BLOCK_SLV_AXI_RD_LIMIT_HI 0x37c
> +#define PARF_ECAM_BASE 0x380
> +#define PARF_ECAM_BASE_HI 0x384
> #define PARF_NO_SNOOP_OVERRIDE 0x3d4
> #define PARF_ATU_BASE_ADDR 0x634
> #define PARF_ATU_BASE_ADDR_HI 0x638
> @@ -87,6 +98,7 @@
>
> /* PARF_SYS_CTRL register fields */
> #define MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN BIT(29)
> +#define PCIE_ECAM_BLOCKER_EN BIT(26)
> #define MST_WAKEUP_EN BIT(13)
> #define SLV_WAKEUP_EN BIT(12)
> #define MSTR_ACLK_CGC_DIS BIT(10)
> @@ -134,6 +146,9 @@
> /* PARF_LTSSM register fields */
> #define LTSSM_EN BIT(8)
>
> +/* PARF_SLV_DBI_ELBI */
> +#define SLV_DBI_ELBI_ADDR_BASE GENMASK(11, 0)
> +
> /* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
> #define PARF_INT_ALL_LINK_UP BIT(13)
> #define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)
> @@ -317,6 +332,48 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> qcom_perst_assert(pcie, false);
> }
>
> +static void qcom_pci_config_ecam(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct qcom_pcie *pcie = to_qcom_pcie(pci);
> + u64 addr, addr_end;
> + u32 val;
> +
> + /* Set the ECAM base */
> + writel_relaxed(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
> + writel_relaxed(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
> +
> + /*
> + * The only device on root bus is the Root Port. Any access to the PCIe
> + * region will go outside the PCIe link. As part of enumeration the PCI
> + * sw can try to read to vendor ID & device ID with different device
> + * number and function number under root bus. As any access other than
> + * root bus, device 0, function 0, should not go out of the link and
> + * should return all F's. Since the iATU is configured for the buses
> + * which starts after root bus, block the transactions starting from
> + * function 1 of the root bus to the end of the root bus (i.e from
> + * dbi_base + 4kb to dbi_base + 1MB) from going outside the link.
> + */
> + addr = pci->dbi_phys_addr + SZ_4K;
> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);
> +
> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
> +
> + addr_end = pci->dbi_phys_addr + SZ_1M - 1;
> +
> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
> +
> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
> +
> + val = readl_relaxed(pcie->parf + PARF_SYS_CTRL);
> + val |= PCIE_ECAM_BLOCKER_EN;
> + writel_relaxed(val, pcie->parf + PARF_SYS_CTRL);
> +}
> +
> static int qcom_pcie_start_link(struct dw_pcie *pci)
> {
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -326,6 +383,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
> qcom_pcie_common_set_16gt_lane_margining(pci);
> }
>
> + if (pci->pp.ecam_enabled)
> + qcom_pci_config_ecam(&pci->pp);
> +
> /* Enable Link Training state machine */
> if (pcie->cfg->ops->ltssm_enable)
> pcie->cfg->ops->ltssm_enable(pcie);
> @@ -1314,6 +1374,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> + u16 offset;
> int ret;
>
> qcom_ep_reset_assert(pcie);
> @@ -1322,6 +1383,15 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret)
> return ret;
>
> + if (pp->ecam_enabled) {
> + /*
> + * Override ELBI when ECAM is enabled, as when ECAM
> + * is enabled ELBI moves along with the dbi config space.
> + */
> + offset = FIELD_GET(SLV_DBI_ELBI_ADDR_BASE, readl(pcie->parf + PARF_SLV_DBI_ELBI));
> + pci->elbi_base = pci->dbi_base + offset;
This looks like there might be a bisection hole between this patch and
the previous patch that enables ECAM in the DWC core? Obviously I
would want to avoid a bisection hole.
What happens to qcom ELBI accesses between these two patches? It
looks like they would go to the wrong address until this elbi_base
update.
Is this connection between DBI and ELBI specific to qcom, or might
other users of ELBI (only exynos, I guess) need a similar update to
elbi_base?
> + }
> +
> ret = qcom_pcie_phy_power_on(pcie);
> if (ret)
> goto err_deinit;
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
2025-08-28 7:34 [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration Krishna Chaitanya Chundru
` (4 preceding siblings ...)
2025-08-28 7:34 ` [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature Krishna Chaitanya Chundru
@ 2025-09-01 13:42 ` Manivannan Sadhasivam
5 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-01 13:42 UTC (permalink / raw)
To: cros-qcom-dts-watchers, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Jingoo Han,
Krishna Chaitanya Chundru
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, quic_vbadigan,
quic_mrana, quic_vpernami, mmareddy, Konrad Dybcio
On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
> The current implementation requires iATU for every configuration
> space access which increases latency & cpu utilization.
>
> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> would be matched against the Base and Limit addresses) of the incoming
> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>
> [...]
Applied, thanks!
[2/5] PCI: dwc: Add support for ELBI resource mapping
commit: d39e0103e38f9889271a77a837b6179b42d6730d
[3/5] PCI: dwc: qcom: Switch to dwc ELBI resource mapping
commit: 6955a13b1349376a00ebca80d1a8e2960de3fb0f
[4/5] PCI: dwc: Add ECAM support with iATU configuration
commit: 0c959f675ce0338f3bee2636437bce4d5713ea03
[5/5] PCI: qcom: Add support for ECAM feature
commit: 0bba488d552dccda4b803fa4b5d4d5d3029d601d
Best regards,
--
Manivannan Sadhasivam <mani@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread