Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
@ 2025-03-05 17:07 Frank Li
  2025-03-17 17:59 ` Bjorn Helgaas
  2025-03-20 20:56 ` Bjorn Helgaas
  0 siblings, 2 replies; 8+ messages in thread
From: Frank Li @ 2025-03-05 17:07 UTC (permalink / raw)
  To: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Frank Li

Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
address translation. Set use_parent_dt_ranges to allow the DWC core driver to
fetch address translation from the device tree.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
This patches basic on
https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/

I have not hardware to test and there are not intel,lgm-pcie in kernel
tree.

Your dts should correct reflect hardware behavor, ref:
https://lore.kernel.org/linux-pci/Z8huvkENIBxyPKJv@axis.com/T/#mb7ae78c3a22324b37567d24ecc1c810c1b3f55c5

According to your intel_pcie_cpu_addr_fixup()

Basically, config space/io/mem space need minus SZ_256. parent bus range
convert it to original value.

Look for driver owner, who help test this and start move forward to remove
cpu_addr_fixup() work.
---
Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 9b53b8f6f268e..c21906eced618 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -57,7 +57,6 @@
 	PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
 	PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
 
-#define BUS_IATU_OFFSET			SZ_256M
 #define RESET_INTERVAL_MS		100
 
 struct intel_pcie {
@@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
 	return intel_pcie_host_setup(pcie);
 }
 
-static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
-{
-	return cpu_addr + BUS_IATU_OFFSET;
-}
-
 static const struct dw_pcie_ops intel_pcie_ops = {
-	.cpu_addr_fixup = intel_pcie_cpu_addr,
 };
 
 static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
@@ -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, pcie);
 	pci = &pcie->pci;
 	pci->dev = dev;
+	pci->use_parent_dt_ranges = true;
 	pp = &pci->pp;
 
 	ret = intel_pcie_get_resources(pdev);

---
base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
change-id: 20250305-intel-7c25bfb498b1

Best regards,


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

* Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-05 17:07 [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup() Frank Li
@ 2025-03-17 17:59 ` Bjorn Helgaas
  2025-03-18  1:49   ` Lei Chuan Hua
  2025-03-20 20:56 ` Bjorn Helgaas
  1 sibling, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2025-03-17 17:59 UTC (permalink / raw)
  To: Frank Li
  Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, linux-pci,
	linux-kernel

On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> fetch address translation from the device tree.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Any update on this, Chuanhua?

I plan to merge v12 of Frank's series [1] for v6.15.  We need to know
ASAP if that would break intel-gw.

If we knew that it was safe to also apply this patch to remove
intel_pcie_cpu_addr(), that would be even better.

I will plan to apply the patch below on top of Frank's series [1] for
v6.15 unless I hear that it would break something.

Bjorn

[1] https://lore.kernel.org/r/20250315201548.858189-1-helgaas@kernel.org

> ---
> This patches basic on
> https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> 
> I have not hardware to test and there are not intel,lgm-pcie in kernel
> tree.
> 
> Your dts should correct reflect hardware behavor, ref:
> https://lore.kernel.org/linux-pci/Z8huvkENIBxyPKJv@axis.com/T/#mb7ae78c3a22324b37567d24ecc1c810c1b3f55c5
> 
> According to your intel_pcie_cpu_addr_fixup()
> 
> Basically, config space/io/mem space need minus SZ_256. parent bus range
> convert it to original value.
> 
> Look for driver owner, who help test this and start move forward to remove
> cpu_addr_fixup() work.
> ---
> Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 9b53b8f6f268e..c21906eced618 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -57,7 +57,6 @@
>  	PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
>  	PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
>  
> -#define BUS_IATU_OFFSET			SZ_256M
>  #define RESET_INTERVAL_MS		100
>  
>  struct intel_pcie {
> @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
>  	return intel_pcie_host_setup(pcie);
>  }
>  
> -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> -{
> -	return cpu_addr + BUS_IATU_OFFSET;
> -}
> -
>  static const struct dw_pcie_ops intel_pcie_ops = {
> -	.cpu_addr_fixup = intel_pcie_cpu_addr,
>  };
>  
>  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> @@ -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, pcie);
>  	pci = &pcie->pci;
>  	pci->dev = dev;
> +	pci->use_parent_dt_ranges = true;
>  	pp = &pci->pp;
>  
>  	ret = intel_pcie_get_resources(pdev);
> 
> ---
> base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> change-id: 20250305-intel-7c25bfb498b1
> 
> Best regards,
> 

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

* Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-17 17:59 ` Bjorn Helgaas
@ 2025-03-18  1:49   ` Lei Chuan Hua
  2025-03-18 15:31     ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Lei Chuan Hua @ 2025-03-18  1:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Frank Li
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Bjorn,

I did a quick test with necessary change in dts. It worked, please move on.

Regards,
Chuanhua

________________________________________
From: Bjorn Helgaas <helgaas@kernel.org>
Sent: Tuesday, March 18, 2025 1:59 AM
To: Frank Li <Frank.Li@nxp.com>
Cc: Lei Chuan Hua <lchuanhua@maxlinear.com>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>; Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob Herring <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()



On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> fetch address translation from the device tree.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Any update on this, Chuanhua?

I plan to merge v12 of Frank's series [1] for v6.15.  We need to know
ASAP if that would break intel-gw.

If we knew that it was safe to also apply this patch to remove
intel_pcie_cpu_addr(), that would be even better.

I will plan to apply the patch below on top of Frank's series [1] for
v6.15 unless I hear that it would break something.

Bjorn

[1] https://lore.kernel.org/r/20250315201548.858189-1-helgaas@kernel.org

> ---
> This patches basic on
> https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
>
> I have not hardware to test and there are not intel,lgm-pcie in kernel
> tree.
>
> Your dts should correct reflect hardware behavor, ref:
> https://lore.kernel.org/linux-pci/Z8huvkENIBxyPKJv@axis.com/T/#mb7ae78c3a22324b37567d24ecc1c810c1b3f55c5
>
> According to your intel_pcie_cpu_addr_fixup()
>
> Basically, config space/io/mem space need minus SZ_256. parent bus range
> convert it to original value.
>
> Look for driver owner, who help test this and start move forward to remove
> cpu_addr_fixup() work.
> ---
> Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 9b53b8f6f268e..c21906eced618 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -57,7 +57,6 @@
>       PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
>       PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
>
> -#define BUS_IATU_OFFSET                      SZ_256M
>  #define RESET_INTERVAL_MS            100
>
>  struct intel_pcie {
> @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
>       return intel_pcie_host_setup(pcie);
>  }
>
> -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> -{
> -     return cpu_addr + BUS_IATU_OFFSET;
> -}
> -
>  static const struct dw_pcie_ops intel_pcie_ops = {
> -     .cpu_addr_fixup = intel_pcie_cpu_addr,
>  };
>
>  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> @@ -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device *pdev)
>       platform_set_drvdata(pdev, pcie);
>       pci = &pcie->pci;
>       pci->dev = dev;
> +     pci->use_parent_dt_ranges = true;
>       pp = &pci->pp;
>
>       ret = intel_pcie_get_resources(pdev);
>
> ---
> base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> change-id: 20250305-intel-7c25bfb498b1
>
> Best regards,
>

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

* Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-18  1:49   ` Lei Chuan Hua
@ 2025-03-18 15:31     ` Bjorn Helgaas
  2025-03-19  6:10       ` Lei Chuan Hua
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2025-03-18 15:31 UTC (permalink / raw)
  To: Lei Chuan Hua
  Cc: Frank Li, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org

On Tue, Mar 18, 2025 at 01:49:46AM +0000, Lei Chuan Hua wrote:
> Hi Bjorn,
> 
> I did a quick test with necessary change in dts. It worked, please move on.

What does this mean?  By "move on", do you mean that I should merge
the patch below (the removal of intel_pcie_cpu_addr())?

I do not want to merge a change that will break any existing intel-gw
platform.  When you say "with necessary change in dts", it makes me
think the removal of intel_pcie_cpu_addr() forces a change to dts,
which would not be acceptable.  We can't force users to upgrade the
dts just to run a newer kernel.

I assume 250318 linux-next, which includes Frank's v12 series, should
work with no change to dts required.  (It would be awesome if you can
verify that.)

If you apply this patch to remove intel_pcie_cpu_addr() on top of
250318 linux-next, does it still work with no changes to dts?

If you have to make a dts change for it to work after removing
intel_pcie_cpu_addr(), then we have a problem.

I do not see a .dts file in the upstream tree that contains
"intel,lgm-pcie", so I don't know what the .dts contains or how it is
distributed.

I do see the binding at
Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml,
but the example there does not include anything about address
translation between the CPU and the PCI controller, so my guess is
that there are .dts files in the field that will not work if we remove
intel_pcie_cpu_addr().

> ________________________________________
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, March 18, 2025 1:59 AM
> To: Frank Li <Frank.Li@nxp.com>
> Cc: Lei Chuan Hua <lchuanhua@maxlinear.com>; Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>; Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob Herring <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> 
> 
> 
> On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> > Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> > address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> > fetch address translation from the device tree.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> 
> Any update on this, Chuanhua?
> 
> I plan to merge v12 of Frank's series [1] for v6.15.  We need to know
> ASAP if that would break intel-gw.
> 
> If we knew that it was safe to also apply this patch to remove
> intel_pcie_cpu_addr(), that would be even better.
> 
> I will plan to apply the patch below on top of Frank's series [1] for
> v6.15 unless I hear that it would break something.
> 
> Bjorn
> 
> [1] https://lore.kernel.org/r/20250315201548.858189-1-helgaas@kernel.org
> 
> > ---
> > This patches basic on
> > https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> >
> > I have not hardware to test and there are not intel,lgm-pcie in kernel
> > tree.
> >
> > Your dts should correct reflect hardware behavor, ref:
> > https://lore.kernel.org/linux-pci/Z8huvkENIBxyPKJv@axis.com/T/#mb7ae78c3a22324b37567d24ecc1c810c1b3f55c5
> >
> > According to your intel_pcie_cpu_addr_fixup()
> >
> > Basically, config space/io/mem space need minus SZ_256. parent bus range
> > convert it to original value.
> >
> > Look for driver owner, who help test this and start move forward to remove
> > cpu_addr_fixup() work.
> > ---
> > Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > index 9b53b8f6f268e..c21906eced618 100644
> > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > @@ -57,7 +57,6 @@
> >       PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> >       PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
> >
> > -#define BUS_IATU_OFFSET                      SZ_256M
> >  #define RESET_INTERVAL_MS            100
> >
> >  struct intel_pcie {
> > @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
> >       return intel_pcie_host_setup(pcie);
> >  }
> >
> > -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> > -{
> > -     return cpu_addr + BUS_IATU_OFFSET;
> > -}
> > -
> >  static const struct dw_pcie_ops intel_pcie_ops = {
> > -     .cpu_addr_fixup = intel_pcie_cpu_addr,
> >  };
> >
> >  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> > @@ -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device *pdev)
> >       platform_set_drvdata(pdev, pcie);
> >       pci = &pcie->pci;
> >       pci->dev = dev;
> > +     pci->use_parent_dt_ranges = true;
> >       pp = &pci->pp;
> >
> >       ret = intel_pcie_get_resources(pdev);
> >
> > ---
> > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > change-id: 20250305-intel-7c25bfb498b1
> >
> > Best regards,
> >

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

* RE: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-18 15:31     ` Bjorn Helgaas
@ 2025-03-19  6:10       ` Lei Chuan Hua
  2025-03-19 19:22         ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Lei Chuan Hua @ 2025-03-19  6:10 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Frank Li, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, 18 March 2025 11:32 pm
> To: Lei Chuan Hua <lchuanhua@maxlinear.com>
> Cc: Frank Li <Frank.Li@nxp.com>; Lorenzo Pieralisi
> <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>; Manivannan
> Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob Herring
> <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-
> pci@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> 
> This email was sent from outside of MaxLinear.
> 
> 
> On Tue, Mar 18, 2025 at 01:49:46AM +0000, Lei Chuan Hua wrote:
> > Hi Bjorn,
> >
> > I did a quick test with necessary change in dts. It worked, please
> move on.
> 
> What does this mean?  By "move on", do you mean that I should merge the
> patch below (the removal of intel_pcie_cpu_addr())?
  I mean you can merge the patch with removal of intel_pcie_cpu_addr()

> I do not want to merge a change that will break any existing intel-gw
> platform.  When you say "with necessary change in dts", it makes me
> think the removal of intel_pcie_cpu_addr() forces a change to dts, which
> would not be acceptable.  We can't force users to upgrade the dts just
> to run a newer kernel.
> 
  Actually, intel_pcie_cpu_addr() did the address translation, so in our case,
  Dts has to adapt to this change.
> I assume 250318 linux-next, which includes Frank's v12 series, should
> work with no change to dts required.  (It would be awesome if you can
> verify that.)
> 

  I will try 250318 linux-next and let you know the result once it is done.

> If you apply this patch to remove intel_pcie_cpu_addr() on top of
> 250318 linux-next, does it still work with no changes to dts?
> 
  I think we need to adapt dts change. Even this series patch has dts
  adaption part.

> If you have to make a dts change for it to work after removing
> intel_pcie_cpu_addr(), then we have a problem.
> 
  We can update the dts yaml file.
> I do not see a .dts file in the upstream tree that contains "intel,lgm-
> pcie", so I don't know what the .dts contains or how it is distributed.
> 
> I do see the binding at
> Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml,
> but the example there does not include anything about address
> translation between the CPU and the PCI controller, so my guess is that
> there are .dts files in the field that will not work if we remove
> intel_pcie_cpu_addr().
> 
  This driver is for x86 atom platform, no upstream dts file even in arch/x86/boot
  Since upstream x86 platforms use acpi, even several platforms use dts, but
  never create dts directory in arch/x86/boot.

  As I mentioned earlier, dts needs a minor change.
> > ________________________________________
> > From: Bjorn Helgaas <mailto:helgaas@kernel.org>
> > Sent: Tuesday, March 18, 2025 1:59 AM
> > To: Frank Li <mailto:Frank.Li@nxp.com>
> > Cc: Lei Chuan Hua <mailto:lchuanhua@maxlinear.com>; Lorenzo Pieralisi
> > <mailto:lpieralisi@kernel.org>; Krzysztof Wilczyński <mailto:kw@linux.com>;
> > Manivannan Sadhasivam <mailto:manivannan.sadhasivam@linaro.org>; Rob Herring
> > <mailto:robh@kernel.org>; Bjorn Helgaas <mailto:bhelgaas@google.com>;
> > mailto:linux-pci@vger.kernel.org <mailto:linux-pci@vger.kernel.org>;
> > mailto:linux-kernel@vger.kernel.org <mailto:linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> > use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> >
> >
> >
> > On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> > > Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should
> > > provide correct address translation. Set use_parent_dt_ranges to
> > > allow the DWC core driver to fetch address translation from the
> device tree.
> > >
> > > Signed-off-by: Frank Li <mailto:Frank.Li@nxp.com>
> >
> > Any update on this, Chuanhua?
> >
> > I plan to merge v12 of Frank's series [1] for v6.15.  We need to know
> > ASAP if that would break intel-gw.
> >
> > If we knew that it was safe to also apply this patch to remove
> > intel_pcie_cpu_addr(), that would be even better.
> >
> > I will plan to apply the patch below on top of Frank's series [1] for
> > v6.15 unless I hear that it would break something.
> >
> > Bjorn
> >
> > [1]
> > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fr%2F20250315201548.858189-1-helgaas%40kernel.org&data=05
> > %7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded5741bbd37508dd66320100%7
> > Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638779087153570342%7CUnkno
> > wn%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa
> > W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aIuZWzwFy2r
> > rzsJ5KfbxWKMx%2BPn1WHx2KvpSR0nxsl8%3D&reserved=0
> >
> > > ---
> > > This patches basic on
> > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.kernel.org%2Fimx%2F20250128-pci_fixup_addr-v9-0-3c4bb506f665%40nx
> > > p.com%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded5741bb
> > > d37508dd66320100%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638779
> > > 087153596851%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiI
> > > wLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C
> > > %7C%7C&sdata=mht19VSB24Znpvtz1pOlmtHYec%2BDBDH70zuLOZmwlSI%3D&reserv
> > > ed=0
> > >
> > > I have not hardware to test and there are not intel,lgm-pcie in
> > > kernel tree.
> > >
> > > Your dts should correct reflect hardware behavor, ref:
> > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.kernel.org%2Flinux-pci%2FZ8huvkENIBxyPKJv%40axis.com%2FT%2F%23mb7
> > > ae78c3a22324b37567d24ecc1c810c1b3f55c5&data=05%7C02%7Clchuanhua%40ma
> > > xlinear.com%7C1612d73ded5741bbd37508dd66320100%7Cdac2800513e041b8828
> > > 07663835f2b1d%7C0%7C0%7C638779087153612764%7CUnknown%7CTWFpbGZsb3d8e
> > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT
> > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=DUsGCW%2FZpvx4whLteoIjYqw
> > > d6oOk9rXks%2BV40i5sovI%3D&reserved=0
> > >
> > > According to your intel_pcie_cpu_addr_fixup()
> > >
> > > Basically, config space/io/mem space need minus SZ_256. parent bus
> > > range convert it to original value.
> > >
> > > Look for driver owner, who help test this and start move forward to
> > > remove
> > > cpu_addr_fixup() work.
> > > ---
> > > Frank Li <mailto:Frank.Li@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
> > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > index 9b53b8f6f268e..c21906eced618 100644
> > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > @@ -57,7 +57,6 @@
> > >       PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> > >       PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
> > >
> > > -#define BUS_IATU_OFFSET                      SZ_256M
> > >  #define RESET_INTERVAL_MS            100
> > >
> > >  struct intel_pcie {
> > > @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp
> *pp)
> > >       return intel_pcie_host_setup(pcie);  }
> > >
> > > -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> > > -{
> > > -     return cpu_addr + BUS_IATU_OFFSET;
> > > -}
> > > -
> > >  static const struct dw_pcie_ops intel_pcie_ops = {
> > > -     .cpu_addr_fixup = intel_pcie_cpu_addr,
> > >  };
> > >
> > >  static const struct dw_pcie_host_ops intel_pcie_dw_ops = { @@
> > > -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device
> *pdev)
> > >       platform_set_drvdata(pdev, pcie);
> > >       pci = &pcie->pci;
> > >       pci->dev = dev;
> > > +     pci->use_parent_dt_ranges = true;
> > >       pp = &pci->pp;
> > >
> > >       ret = intel_pcie_get_resources(pdev);
> > >
> > > ---
> > > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > > change-id: 20250305-intel-7c25bfb498b1
> > >
> > > Best regards,
> > >

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

* Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-19  6:10       ` Lei Chuan Hua
@ 2025-03-19 19:22         ` Bjorn Helgaas
  2025-03-20  2:44           ` Lei Chuan Hua
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2025-03-19 19:22 UTC (permalink / raw)
  To: Lei Chuan Hua
  Cc: Frank Li, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org

On Wed, Mar 19, 2025 at 06:10:57AM +0000, Lei Chuan Hua wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: Tuesday, 18 March 2025 11:32 pm
> > To: Lei Chuan Hua <lchuanhua@maxlinear.com>
> > Cc: Frank Li <Frank.Li@nxp.com>; Lorenzo Pieralisi
> > <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>; Manivannan
> > Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob Herring
> > <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-
> > pci@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> > use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> > 
> > This email was sent from outside of MaxLinear.
> > 
> > On Tue, Mar 18, 2025 at 01:49:46AM +0000, Lei Chuan Hua wrote:
> > > Hi Bjorn,
> > >
> > > I did a quick test with necessary change in dts. It worked, please
> > > move on.
> > 
> > What does this mean?  By "move on", do you mean that I should merge the
> > patch below (the removal of intel_pcie_cpu_addr())?
>
>   I mean you can merge the patch with removal of intel_pcie_cpu_addr()
> 
> > I do not want to merge a change that will break any existing intel-gw
> > platform.  When you say "with necessary change in dts", it makes me
> > think the removal of intel_pcie_cpu_addr() forces a change to dts, which
> > would not be acceptable.  We can't force users to upgrade the dts just
> > to run a newer kernel.
>
>   Actually, intel_pcie_cpu_addr() did the address translation, so in our case,
>   Dts has to adapt to this change.
>
> > I assume 250318 linux-next, which includes Frank's v12 series, should
> > work with no change to dts required.  (It would be awesome if you can
> > verify that.)
> 
>   I will try 250318 linux-next and let you know the result once it is done.
> 
> > If you apply this patch to remove intel_pcie_cpu_addr() on top of
> > 250318 linux-next, does it still work with no changes to dts?
>
>   I think we need to adapt dts change. Even this series patch has dts
>   adaption part.
> 
> > If you have to make a dts change for it to work after removing
> > intel_pcie_cpu_addr(), then we have a problem.
>
>   We can update the dts yaml file.
>
> > I do not see a .dts file in the upstream tree that contains "intel,lgm-
> > pcie", so I don't know what the .dts contains or how it is distributed.
> > 
> > I do see the binding at
> > Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml,
> > but the example there does not include anything about address
> > translation between the CPU and the PCI controller, so my guess is that
> > there are .dts files in the field that will not work if we remove
> > intel_pcie_cpu_addr().
>
>   This driver is for x86 atom platform, no upstream dts file even in
>   arch/x86/boot Since upstream x86 platforms use acpi, even several
>   platforms use dts, but never create dts directory in
>   arch/x86/boot.
> 
>   As I mentioned earlier, dts needs a minor change.

If there are end users that have a dts that must be changed before
intel_pcie_cpu_addr() can be removed, we can't remove it because we
can't force those users to upgrade their dts.

If this driver is only used internally to Intel, or if the hardware
has never been shipped to end users, it's OK to remove
intel_pcie_cpu_addr() and assume those internal users will update dts.

> > > ________________________________________
> > > From: Bjorn Helgaas <mailto:helgaas@kernel.org>
> > > Sent: Tuesday, March 18, 2025 1:59 AM
> > > To: Frank Li <mailto:Frank.Li@nxp.com>
> > > Cc: Lei Chuan Hua <mailto:lchuanhua@maxlinear.com>; Lorenzo Pieralisi
> > > <mailto:lpieralisi@kernel.org>; Krzysztof Wilczyński <mailto:kw@linux.com>;
> > > Manivannan Sadhasivam <mailto:manivannan.sadhasivam@linaro.org>; Rob Herring
> > > <mailto:robh@kernel.org>; Bjorn Helgaas <mailto:bhelgaas@google.com>;
> > > mailto:linux-pci@vger.kernel.org <mailto:linux-pci@vger.kernel.org>;
> > > mailto:linux-kernel@vger.kernel.org <mailto:linux-kernel@vger.kernel.org>
> > > Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> > > use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> > >
> > >
> > >
> > > On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> > > > Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should
> > > > provide correct address translation. Set use_parent_dt_ranges to
> > > > allow the DWC core driver to fetch address translation from the
> > device tree.
> > > >
> > > > Signed-off-by: Frank Li <mailto:Frank.Li@nxp.com>
> > >
> > > Any update on this, Chuanhua?
> > >
> > > I plan to merge v12 of Frank's series [1] for v6.15.  We need to know
> > > ASAP if that would break intel-gw.
> > >
> > > If we knew that it was safe to also apply this patch to remove
> > > intel_pcie_cpu_addr(), that would be even better.
> > >
> > > I will plan to apply the patch below on top of Frank's series [1] for
> > > v6.15 unless I hear that it would break something.
> > >
> > > Bjorn
> > >
> > > [1]
> > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Fr%2F20250315201548.858189-1-helgaas%40kernel.org&data=05
> > > %7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded5741bbd37508dd66320100%7
> > > Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638779087153570342%7CUnkno
> > > wn%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa
> > > W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aIuZWzwFy2r
> > > rzsJ5KfbxWKMx%2BPn1WHx2KvpSR0nxsl8%3D&reserved=0
> > >
> > > > ---
> > > > This patches basic on
> > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > > re.kernel.org%2Fimx%2F20250128-pci_fixup_addr-v9-0-3c4bb506f665%40nx
> > > > p.com%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded5741bb
> > > > d37508dd66320100%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638779
> > > > 087153596851%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiI
> > > > wLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C
> > > > %7C%7C&sdata=mht19VSB24Znpvtz1pOlmtHYec%2BDBDH70zuLOZmwlSI%3D&reserv
> > > > ed=0
> > > >
> > > > I have not hardware to test and there are not intel,lgm-pcie in
> > > > kernel tree.
> > > >
> > > > Your dts should correct reflect hardware behavor, ref:
> > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > > re.kernel.org%2Flinux-pci%2FZ8huvkENIBxyPKJv%40axis.com%2FT%2F%23mb7
> > > > ae78c3a22324b37567d24ecc1c810c1b3f55c5&data=05%7C02%7Clchuanhua%40ma
> > > > xlinear.com%7C1612d73ded5741bbd37508dd66320100%7Cdac2800513e041b8828
> > > > 07663835f2b1d%7C0%7C0%7C638779087153612764%7CUnknown%7CTWFpbGZsb3d8e
> > > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT
> > > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=DUsGCW%2FZpvx4whLteoIjYqw
> > > > d6oOk9rXks%2BV40i5sovI%3D&reserved=0
> > > >
> > > > According to your intel_pcie_cpu_addr_fixup()
> > > >
> > > > Basically, config space/io/mem space need minus SZ_256. parent bus
> > > > range convert it to original value.
> > > >
> > > > Look for driver owner, who help test this and start move forward to
> > > > remove
> > > > cpu_addr_fixup() work.
> > > > ---
> > > > Frank Li <mailto:Frank.Li@nxp.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
> > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > index 9b53b8f6f268e..c21906eced618 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > @@ -57,7 +57,6 @@
> > > >       PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> > > >       PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
> > > >
> > > > -#define BUS_IATU_OFFSET                      SZ_256M
> > > >  #define RESET_INTERVAL_MS            100
> > > >
> > > >  struct intel_pcie {
> > > > @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp
> > *pp)
> > > >       return intel_pcie_host_setup(pcie);  }
> > > >
> > > > -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> > > > -{
> > > > -     return cpu_addr + BUS_IATU_OFFSET;
> > > > -}
> > > > -
> > > >  static const struct dw_pcie_ops intel_pcie_ops = {
> > > > -     .cpu_addr_fixup = intel_pcie_cpu_addr,
> > > >  };
> > > >
> > > >  static const struct dw_pcie_host_ops intel_pcie_dw_ops = { @@
> > > > -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device
> > *pdev)
> > > >       platform_set_drvdata(pdev, pcie);
> > > >       pci = &pcie->pci;
> > > >       pci->dev = dev;
> > > > +     pci->use_parent_dt_ranges = true;
> > > >       pp = &pci->pp;
> > > >
> > > >       ret = intel_pcie_get_resources(pdev);
> > > >
> > > > ---
> > > > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > > > change-id: 20250305-intel-7c25bfb498b1
> > > >
> > > > Best regards,
> > > >

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

* RE: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-19 19:22         ` Bjorn Helgaas
@ 2025-03-20  2:44           ` Lei Chuan Hua
  0 siblings, 0 replies; 8+ messages in thread
From: Lei Chuan Hua @ 2025-03-20  2:44 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Frank Li, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Thursday, 20 March 2025 3:23 am
> To: Lei Chuan Hua <lchuanhua@maxlinear.com>
> Cc: Frank Li <Frank.Li@nxp.com>; Lorenzo Pieralisi
> <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>; Manivannan
> Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob Herring
> <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-
> pci@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
>  
> 
> On Wed, Mar 19, 2025 at 06:10:57AM +0000, Lei Chuan Hua wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: Tuesday, 18 March 2025 11:32 pm
> > > To: Lei Chuan Hua <lchuanhua@maxlinear.com>
> > > Cc: Frank Li <Frank.Li@nxp.com>; Lorenzo Pieralisi
> > > <lpieralisi@kernel.org>; Krzysztof Wilczyński <kw@linux.com>;
> > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>; Rob
> > > Herring <robh@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>;
> > > linux- pci@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> > > use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> > >
> > > This email was sent from outside of MaxLinear.
> > >
> > > On Tue, Mar 18, 2025 at 01:49:46AM +0000, Lei Chuan Hua wrote:
> > > > Hi Bjorn,
> > > >
> > > > I did a quick test with necessary change in dts. It worked, please
> > > > move on.
> > >
> > > What does this mean?  By "move on", do you mean that I should merge
> > > the patch below (the removal of intel_pcie_cpu_addr())?
> >
> >   I mean you can merge the patch with removal of intel_pcie_cpu_addr()
> >
> > > I do not want to merge a change that will break any existing
> > > intel-gw platform.  When you say "with necessary change in dts", it
> > > makes me think the removal of intel_pcie_cpu_addr() forces a change
> > > to dts, which would not be acceptable.  We can't force users to
> > > upgrade the dts just to run a newer kernel.
> >
> >   Actually, intel_pcie_cpu_addr() did the address translation, so in
> our case,
> >   Dts has to adapt to this change.
> >
> > > I assume 250318 linux-next, which includes Frank's v12 series,
> > > should work with no change to dts required.  (It would be awesome if
> > > you can verify that.)
> >
> >   I will try 250318 linux-next and let you know the result once it is
> done.
> >
> > > If you apply this patch to remove intel_pcie_cpu_addr() on top of
> > > 250318 linux-next, does it still work with no changes to dts?
> >
> >   I think we need to adapt dts change. Even this series patch has dts
> >   adaption part.
> >
> > > If you have to make a dts change for it to work after removing
> > > intel_pcie_cpu_addr(), then we have a problem.
> >
> >   We can update the dts yaml file.
> >
> > > I do not see a .dts file in the upstream tree that contains
> > > "intel,lgm- pcie", so I don't know what the .dts contains or how it
> is distributed.
> > >
> > > I do see the binding at
> > > Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml,
> > > but the example there does not include anything about address
> > > translation between the CPU and the PCI controller, so my guess is
> > > that there are .dts files in the field that will not work if we
> > > remove intel_pcie_cpu_addr().
> >
> >   This driver is for x86 atom platform, no upstream dts file even in
> >   arch/x86/boot Since upstream x86 platforms use acpi, even several
> >   platforms use dts, but never create dts directory in
> >   arch/x86/boot.
> >
> >   As I mentioned earlier, dts needs a minor change.
> 
> If there are end users that have a dts that must be changed before
> intel_pcie_cpu_addr() can be removed, we can't remove it because we
> can't force those users to upgrade their dts.
> 
> If this driver is only used internally to Intel, or if the hardware has
> never been shipped to end users, it's OK to remove
> intel_pcie_cpu_addr() and assume those internal users will update dts.

This driver is only used for internally to Maxlinear. Internal users will update dts
Accordingly. Anyway, we will take linux-next-20250318 to run the test with dts adaptation
Internally on the hardware board.
> 
> > > > ________________________________________
> > > > From: Bjorn Helgaas <mailto:helgaas@kernel.org>
> > > > Sent: Tuesday, March 18, 2025 1:59 AM
> > > > To: Frank Li <mailto:Frank.Li@nxp.com>
> > > > Cc: Lei Chuan Hua <mailto:lchuanhua@maxlinear.com>; Lorenzo
> > > > Pieralisi <mailto:lpieralisi@kernel.org>; Krzysztof Wilczyński
> > > > <mailto:kw@linux.com>; Manivannan Sadhasivam
> > > > <mailto:manivannan.sadhasivam@linaro.org>; Rob Herring
> > > > <mailto:robh@kernel.org>; Bjorn Helgaas
> > > > <mailto:bhelgaas@google.com>; mailto:linux-pci@vger.kernel.org
> > > > <mailto:linux-pci@vger.kernel.org>;
> > > > mailto:linux-kernel@vger.kernel.org
> > > > <mailto:linux-kernel@vger.kernel.org>
> > > > Subject: Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use
> > > > use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
> > > >
> > > >
> > > >
> > > > On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> > > > > Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should
> > > > > provide correct address translation. Set use_parent_dt_ranges to
> > > > > allow the DWC core driver to fetch address translation from the
> > > device tree.
> > > > >
> > > > > Signed-off-by: Frank Li <mailto:Frank.Li@nxp.com>
> > > >
> > > > Any update on this, Chuanhua?
> > > >
> > > > I plan to merge v12 of Frank's series [1] for v6.15.  We need to
> > > > know ASAP if that would break intel-gw.
> > > >
> > > > If we knew that it was safe to also apply this patch to remove
> > > > intel_pcie_cpu_addr(), that would be even better.
> > > >
> > > > I will plan to apply the patch below on top of Frank's series [1]
> > > > for
> > > > v6.15 unless I hear that it would break something.
> > > >
> > > > Bjorn
> > > >
> > > > [1]
> > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > lore%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1c7a4180ddc340f
> > > > 2399308dd671b6e6a%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638
> > > > 780089712963723%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIl
> > > > YiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D
> > > > %7C0%7C%7C%7C&sdata=ESUTIdyb342xMYwUhzwj%2BXsKoNQzth86rV3dfis5dn0%
> > > > 3D&reserved=0
> > > > .kernel.org%2Fr%2F20250315201548.858189-1-helgaas%40kernel.org&dat
> > > > a=05
> > > > %7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded5741bbd37508dd663201
> > > > 00%7
> > > > Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C638779087153570342%7CU
> > > > nkno
> > > > wn%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiO
> > > > iJXa
> > > > W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aIuZWzw
> > > > Fy2r
> > > > rzsJ5KfbxWKMx%2BPn1WHx2KvpSR0nxsl8%3D&reserved=0
> > > >
> > > > > ---
> > > > > This patches basic on
> > > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Flo%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1c7a4180ddc34
> > > > > 0f2399308dd671b6e6a%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7
> > > > > C638780089712987168%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRy
> > > > > dWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoy
> > > > > fQ%3D%3D%7C0%7C%7C%7C&sdata=PguxNdlqlw4e%2FqmgY4aM67UarmNDwx7RCV
> > > > > 69x0myXJc%3D&reserved=0
> > > > > re.kernel.org%2Fimx%2F20250128-pci_fixup_addr-v9-0-3c4bb506f665%
> > > > > 40nx
> > > > > p.com%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1612d73ded57
> > > > > 41bb
> > > > > d37508dd66320100%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7C63
> > > > > 8779
> > > > > 087153596851%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlY
> > > > > iOiI
> > > > > wLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C
> > > > > 0%7C
> > > > > %7C%7C&sdata=mht19VSB24Znpvtz1pOlmtHYec%2BDBDH70zuLOZmwlSI%3D&re
> > > > > serv
> > > > > ed=0
> > > > >
> > > > > I have not hardware to test and there are not intel,lgm-pcie in
> > > > > kernel tree.
> > > > >
> > > > > Your dts should correct reflect hardware behavor, ref:
> > > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Flo%2F&data=05%7C02%7Clchuanhua%40maxlinear.com%7C1c7a4180ddc34
> > > > > 0f2399308dd671b6e6a%7Cdac2800513e041b882807663835f2b1d%7C0%7C0%7
> > > > > C638780089713001616%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRy
> > > > > dWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoy
> > > > > fQ%3D%3D%7C0%7C%7C%7C&sdata=DyXeezC8qsW2cdRg37sKVq2AH82vCV7L7YJ%
> > > > > 2FtgH6jU0%3D&reserved=0
> > > > > re.kernel.org%2Flinux-pci%2FZ8huvkENIBxyPKJv%40axis.com%2FT%2F%2
> > > > > 3mb7
> > > > > ae78c3a22324b37567d24ecc1c810c1b3f55c5&data=05%7C02%7Clchuanhua%
> > > > > 40ma
> > > > > xlinear.com%7C1612d73ded5741bbd37508dd66320100%7Cdac2800513e041b
> > > > > 8828
> > > > > 07663835f2b1d%7C0%7C0%7C638779087153612764%7CUnknown%7CTWFpbGZsb
> > > > > 3d8e
> > > > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOI
> > > > > joiT
> > > > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=DUsGCW%2FZpvx4whLteoI
> > > > > jYqw
> > > > > d6oOk9rXks%2BV40i5sovI%3D&reserved=0
> > > > >
> > > > > According to your intel_pcie_cpu_addr_fixup()
> > > > >
> > > > > Basically, config space/io/mem space need minus SZ_256. parent
> > > > > bus range convert it to original value.
> > > > >
> > > > > Look for driver owner, who help test this and start move forward
> > > > > to remove
> > > > > cpu_addr_fixup() work.
> > > > > ---
> > > > > Frank Li <mailto:Frank.Li@nxp.com>
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
> > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > > b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > > index 9b53b8f6f268e..c21906eced618 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > > @@ -57,7 +57,6 @@
> > > > >       PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> > > > >       PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
> > > > >
> > > > > -#define BUS_IATU_OFFSET                      SZ_256M
> > > > >  #define RESET_INTERVAL_MS            100
> > > > >
> > > > >  struct intel_pcie {
> > > > > @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct
> > > > > dw_pcie_rp
> > > *pp)
> > > > >       return intel_pcie_host_setup(pcie);  }
> > > > >
> > > > > -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64
> > > > > cpu_addr) -{
> > > > > -     return cpu_addr + BUS_IATU_OFFSET;
> > > > > -}
> > > > > -
> > > > >  static const struct dw_pcie_ops intel_pcie_ops = {
> > > > > -     .cpu_addr_fixup = intel_pcie_cpu_addr,
> > > > >  };
> > > > >
> > > > >  static const struct dw_pcie_host_ops intel_pcie_dw_ops = { @@
> > > > > -409,6 +402,7 @@ static int intel_pcie_probe(struct
> > > > > platform_device
> > > *pdev)
> > > > >       platform_set_drvdata(pdev, pcie);
> > > > >       pci = &pcie->pci;
> > > > >       pci->dev = dev;
> > > > > +     pci->use_parent_dt_ranges = true;
> > > > >       pp = &pci->pp;
> > > > >
> > > > >       ret = intel_pcie_get_resources(pdev);
> > > > >
> > > > > ---
> > > > > base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> > > > > change-id: 20250305-intel-7c25bfb498b1
> > > > >
> > > > > Best regards,
> > > > >

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

* Re: [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup()
  2025-03-05 17:07 [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup() Frank Li
  2025-03-17 17:59 ` Bjorn Helgaas
@ 2025-03-20 20:56 ` Bjorn Helgaas
  1 sibling, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2025-03-20 20:56 UTC (permalink / raw)
  To: Frank Li
  Cc: Chuanhua Lei, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, linux-pci,
	linux-kernel

On Wed, Mar 05, 2025 at 12:07:54PM -0500, Frank Li wrote:
> Remove intel_pcie_cpu_addr_fixup() as the DT bus fabric should provide correct
> address translation. Set use_parent_dt_ranges to allow the DWC core driver to
> fetch address translation from the device tree.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Applied to pci/controller/dwc-cpu-addr-fixup for v6.15, thanks!

There is a minor dts change required, but Lei Chuan Hua has confirmed
that all users are internal to Maxlinear and will update:

https://lore.kernel.org/r/BY3PR19MB507667CE7531D863E1E5F8AEBDD82@BY3PR19MB5076.namprd19.prod.outlook.com

> ---
> This patches basic on
> https://lore.kernel.org/imx/20250128-pci_fixup_addr-v9-0-3c4bb506f665@nxp.com/
> 
> I have not hardware to test and there are not intel,lgm-pcie in kernel
> tree.
> 
> Your dts should correct reflect hardware behavor, ref:
> https://lore.kernel.org/linux-pci/Z8huvkENIBxyPKJv@axis.com/T/#mb7ae78c3a22324b37567d24ecc1c810c1b3f55c5
> 
> According to your intel_pcie_cpu_addr_fixup()
> 
> Basically, config space/io/mem space need minus SZ_256. parent bus range
> convert it to original value.
> 
> Look for driver owner, who help test this and start move forward to remove
> cpu_addr_fixup() work.
> ---
> Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-intel-gw.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 9b53b8f6f268e..c21906eced618 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -57,7 +57,6 @@
>  	PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
>  	PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
>  
> -#define BUS_IATU_OFFSET			SZ_256M
>  #define RESET_INTERVAL_MS		100
>  
>  struct intel_pcie {
> @@ -381,13 +380,7 @@ static int intel_pcie_rc_init(struct dw_pcie_rp *pp)
>  	return intel_pcie_host_setup(pcie);
>  }
>  
> -static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> -{
> -	return cpu_addr + BUS_IATU_OFFSET;
> -}
> -
>  static const struct dw_pcie_ops intel_pcie_ops = {
> -	.cpu_addr_fixup = intel_pcie_cpu_addr,
>  };
>  
>  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> @@ -409,6 +402,7 @@ static int intel_pcie_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, pcie);
>  	pci = &pcie->pci;
>  	pci->dev = dev;
> +	pci->use_parent_dt_ranges = true;
>  	pp = &pci->pp;
>  
>  	ret = intel_pcie_get_resources(pdev);
> 
> ---
> base-commit: 1552be4855dacca5ea39b15b1ef0b96c91dbea0d
> change-id: 20250305-intel-7c25bfb498b1
> 
> Best regards,
> 

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

end of thread, other threads:[~2025-03-20 20:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 17:07 [PATCH RFC NOT TESTED] PCI: intel-gw: Use use_parent_dt_ranges and clean up intel_pcie_cpu_addr_fixup() Frank Li
2025-03-17 17:59 ` Bjorn Helgaas
2025-03-18  1:49   ` Lei Chuan Hua
2025-03-18 15:31     ` Bjorn Helgaas
2025-03-19  6:10       ` Lei Chuan Hua
2025-03-19 19:22         ` Bjorn Helgaas
2025-03-20  2:44           ` Lei Chuan Hua
2025-03-20 20:56 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox