[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250318153148.GA1000275@bhelgaas>
Date: Tue, 18 Mar 2025 10:31:48 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Lei Chuan Hua <lchuanhua@...linear.com>
Cc: Frank Li <Frank.Li@....com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.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 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@...nel.org>
> Sent: Tuesday, March 18, 2025 1:59 AM
> To: Frank Li <Frank.Li@....com>
> Cc: Lei Chuan Hua <lchuanhua@...linear.com>; Lorenzo Pieralisi <lpieralisi@...nel.org>; Krzysztof Wilczyński <kw@...ux.com>; Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>; Rob Herring <robh@...nel.org>; Bjorn Helgaas <bhelgaas@...gle.com>; linux-pci@...r.kernel.org <linux-pci@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.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@....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@....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,
> >
Powered by blists - more mailing lists