[v2,1/3] Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"
diff mbox series

Message ID 221deb47d7d38ecd6e0c7636f9a87ab693a3c539.1532090446.git.leonard.crestez@nxp.com
State Superseded
Headers show
Series
  • PCI: imx: Initial imx7d pm support
Related show

Commit Message

Leonard Crestez July 20, 2018, 12:47 p.m. UTC
This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.

That commit followed the reference manual but unfortunately the imx7d
manual is incorrect.

Tested with ath9k pcie card and confirmed internally.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm/boot/dts/imx7d.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Andrey Smirnov July 20, 2018, 3:33 p.m. UTC | #1
On Fri, Jul 20, 2018 at 5:48 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.
>
> That commit followed the reference manual but unfortunately the imx7d
> manual is incorrect.
>

I'd also add similar comment to DT file to prevent people from trying
to "fix" this in the future.

Also, this change is going to break QEMU's mapping found here:

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/arm/fsl-imx7.c;h=d5e26855a552e2d52b91f0435a607fae2f88456b;hb=HEAD#l464

any change you are planning to make the change there as well?

Thanks,
Andrey Smirnov
Leonard Crestez July 23, 2018, 12:41 p.m. UTC | #2
On Fri, 2018-07-20 at 08:33 -0700, Andrey Smirnov wrote:
> On Fri, Jul 20, 2018 at 5:48 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> > 
> > This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.
> > 
> > That commit followed the reference manual but unfortunately the imx7d
> > manual is incorrect.
> 
> I'd also add similar comment to DT file to prevent people from trying
> to "fix" this in the future.

I'll try to see if I can follow up internally with docs team to get
this updated in the next revision of the reference manual.

> Also, this change is going to break QEMU's mapping found here:

I had no idea that existed, I guess somebody needs to fix that as well.

Do you have an imx7d board using pci or did you just test in emulation?

--
Regards,
Leonard
Andrey Smirnov July 23, 2018, 6:38 p.m. UTC | #3
On Mon, Jul 23, 2018 at 5:41 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On Fri, 2018-07-20 at 08:33 -0700, Andrey Smirnov wrote:
> > On Fri, Jul 20, 2018 at 5:48 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> > >
> > > This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.
> > >
> > > That commit followed the reference manual but unfortunately the imx7d
> > > manual is incorrect.
> >
> > I'd also add similar comment to DT file to prevent people from trying
> > to "fix" this in the future.
>
> I'll try to see if I can follow up internally with docs team to get
> this updated in the next revision of the reference manual.
>

Not sure if we are on the same page here, but what I meant is adding
the same explanation that is in your commit message to Device Tree
file as well so that if anyone looks at DT code and goes "Huh?" in the
future, they wouldn't have to research commit history to see the
reason why things the way they are.

> > Also, this change is going to break QEMU's mapping found here:
>
> I had no idea that existed, I guess somebody needs to fix that as well.
>

I take it it's a no to my "any chance you can fix that as well?" ;-).
I'll see if I can find time to do that this week.

> Do you have an imx7d board using pci or did you just test in emulation?
>

I used real i.MX7D Sabre board with i210 network card, when I was
porting i.MX7 PCIe patches. However, as per note I made in the
original submission:

https://lkml.org/lkml/2017/10/9/913

this IRQ swap patch came up as a result of "connecting" emulated ICH4
in QEMU which was using legacy interrupts.

Thanks,
Andrey Smirnov
Leonard Crestez July 24, 2018, 11:34 a.m. UTC | #4
On Mon, 2018-07-23 at 11:38 -0700, Andrey Smirnov wrote:
> On Mon, Jul 23, 2018 at 5:41 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> > On Fri, 2018-07-20 at 08:33 -0700, Andrey Smirnov wrote:
> > > On Fri, Jul 20, 2018 at 5:48 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> > > > 
> > > > This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.
> > > > 
> > > > That commit followed the reference manual but unfortunately the imx7d
> > > > manual is incorrect.
> > > 
> > > I'd also add similar comment to DT file to prevent people from trying
> > > to "fix" this in the future.
> > 
> > I'll try to see if I can follow up internally with docs team to get
> > this updated in the next revision of the reference manual.
> 
> Not sure if we are on the same page here, but what I meant is adding
> the same explanation that is in your commit message to Device Tree
> file as well so that if anyone looks at DT code and goes "Huh?" in the
> future, they wouldn't have to research commit history to see the
> reason why things the way they are.

OK, I will add a comment on irq mappings in the next version explaining
that the reference manual is incorrect.

> > > Also, this change is going to break QEMU's mapping found here:
> > 
> > I had no idea that existed, I guess somebody needs to fix that as well.
> 
> I take it it's a no to my "any chance you can fix that as well?" ;-).
> I'll see if I can find time to do that this week.

Sorry, I'm not familiar with qemu source at all.

Patch
diff mbox series

diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 8d3d123d0a5c..12c5ba7e9f1e 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -123,14 +123,14 @@ 
 		num-lanes = <1>;
 		interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-names = "msi";
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0x7>;
-		interrupt-map = <0 0 0 1 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 2 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 3 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 4 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-map = <0 0 0 1 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 2 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 3 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 4 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clks IMX7D_PCIE_CTRL_ROOT_CLK>,
 			 <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>,
 			 <&clks IMX7D_PCIE_PHY_ROOT_CLK>;
 		clock-names = "pcie", "pcie_bus", "pcie_phy";
 		assigned-clocks = <&clks IMX7D_PCIE_CTRL_ROOT_SRC>,