| Message ID | 1418602429-11276-11-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled > status. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> The addresses and interrupt numbers look OK to me. However, my comment about the "0x800" offset is still valid. Shouldn't we have two register blocks, and let the driver use only the second one? If you ignore, feel free to add my Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: > On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: > > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled > > status. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@ideasonboard.com> > > The addresses and interrupt numbers look OK to me. > However, my comment about the "0x800" offset is still valid. > Shouldn't we have two register blocks, and let the driver use only the > second one? > > If you ignore, feel free to add my > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> I don't want to ignore your comment, but I don't know what to do here :-/ The datasheet lacks detailed information about secure vs. non-secure mode and how the two register sets are supposed to interoperate and be handled by the operating system.
Hi Laurent, On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Geert, > > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: >> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled >> > status. >> > >> > Signed-off-by: Laurent Pinchart >> > <laurent.pinchart+renesas@ideasonboard.com> >> >> The addresses and interrupt numbers look OK to me. >> However, my comment about the "0x800" offset is still valid. >> Shouldn't we have two register blocks, and let the driver use only the >> second one? >> >> If you ignore, feel free to add my >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > I don't want to ignore your comment, but I don't know what to do here :-/ The > datasheet lacks detailed information about secure vs. non-secure mode and how > the two register sets are supposed to interoperate and be handled by the > operating system. I don't know about that either. But how about differences within R-Car Gen2 series so far? If there are known differences then perhaps we should use part number in the compatible string? Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus, On Tuesday 16 December 2014 07:44:32 Magnus Damm wrote: > On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart wrote: > > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: > >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: > >> > Add the seven IPMMU instances found in the r8a7791 to DT with a > >> > disabled status. > >> > > >> > Signed-off-by: Laurent Pinchart > >> > <laurent.pinchart+renesas@ideasonboard.com> > >> > >> The addresses and interrupt numbers look OK to me. > >> However, my comment about the "0x800" offset is still valid. > >> Shouldn't we have two register blocks, and let the driver use only the > >> second one? > >> > >> If you ignore, feel free to add my > >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > I don't want to ignore your comment, but I don't know what to do here :-/ > > The datasheet lacks detailed information about secure vs. non-secure mode > > and how the two register sets are supposed to interoperate and be handled > > by the operating system. > > I don't know about that either. > > But how about differences within R-Car Gen2 series so far? If there > are known differences then perhaps we should use part number in the > compatible string? I haven't noticed differences between H2 and M2. V2 and E2 are extended with IOMMU performance monitoring registers, so a specific compatible string is needed. "renesas,ipmmu-vmsa" should be the default fallback compatible string. I was thinking about playing with the newer SoCs first to see how the IOMMU react and then most likely add SoC-specific compatible strings.
Hi Laurent, On Mon, Dec 15, 2014 at 7:44 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: >> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled >> > status. >> > >> > Signed-off-by: Laurent Pinchart >> > <laurent.pinchart+renesas@ideasonboard.com> >> >> The addresses and interrupt numbers look OK to me. >> However, my comment about the "0x800" offset is still valid. >> Shouldn't we have two register blocks, and let the driver use only the >> second one? >> >> If you ignore, feel free to add my >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > I don't want to ignore your comment, but I don't know what to do here :-/ The > datasheet lacks detailed information about secure vs. non-secure mode and how > the two register sets are supposed to interoperate and be handled by the > operating system. When in doubt, the safest thing to do is "describe the hardware in DT". The datasheet says there are two register sets of size 0x800, so IMHO we should have both in DT. Whether the driver only uses the second bank due to our limited understanding of the hardware is something different. We can still fix the driver later, if needed. Fixing the DT is, ... well... complicated ;-) Following the rule "describe the hardware in DT", I would also add the second interrupt (marked "SEC") for the DS0, MX, SY0, and GP IPMMU instances. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Tuesday 16 December 2014 11:02:03 Geert Uytterhoeven wrote: > On Mon, Dec 15, 2014 at 7:44 PM, Laurent Pinchart wrote: > > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: > >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: > >> > Add the seven IPMMU instances found in the r8a7791 to DT with a > >> > disabled status. > >> > > >> > Signed-off-by: Laurent Pinchart > >> > <laurent.pinchart+renesas@ideasonboard.com> > >> > >> The addresses and interrupt numbers look OK to me. > >> However, my comment about the "0x800" offset is still valid. > >> Shouldn't we have two register blocks, and let the driver use only the > >> second one? > >> > >> If you ignore, feel free to add my > >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > I don't want to ignore your comment, but I don't know what to do here :-/ > > The datasheet lacks detailed information about secure vs. non-secure mode > > and how the two register sets are supposed to interoperate and be handled > > by the operating system. > > When in doubt, the safest thing to do is "describe the hardware in DT". > > The datasheet says there are two register sets of size 0x800, so IMHO we > should have both in DT. Whether the driver only uses the second bank due to > our limited understanding of the hardware is something different. We can > still fix the driver later, if needed. Fixing the DT is, ... well... > complicated ;-) According to the datasheet there are two register sets sharing the same base address, banked through secure mode. There's additionally an alias at a different base address for non-secure registers access when running in secure mode. The hardware description thus depends on whether we're running in secure or non-secure mode, which doesn't play nicely with DT. Additionally, I've tried to disable secure mode at boot time to test non- secure mode at the base address, but it didn't seem to make any difference. I might have done something wrong though. For these reasons I don't really like the idea of having two resources for secure and non-secure mode. Extending the single resource to cover the base address and the aliases should be fine though, I'll give it a go. > Following the rule "describe the hardware in DT", I would also add the > second interrupt (marked "SEC") for the DS0, MX, SY0, and GP IPMMU > instances. Agreed, I'll do that.
Hi Laurent, On Tue, Dec 16, 2014 at 8:16 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Magnus, > > On Tuesday 16 December 2014 07:44:32 Magnus Damm wrote: >> On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart wrote: >> > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote: >> >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote: >> >> > Add the seven IPMMU instances found in the r8a7791 to DT with a >> >> > disabled status. >> >> > >> >> > Signed-off-by: Laurent Pinchart >> >> > <laurent.pinchart+renesas@ideasonboard.com> >> >> >> >> The addresses and interrupt numbers look OK to me. >> >> However, my comment about the "0x800" offset is still valid. >> >> Shouldn't we have two register blocks, and let the driver use only the >> >> second one? >> >> >> >> If you ignore, feel free to add my >> >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> >> > >> > I don't want to ignore your comment, but I don't know what to do here :-/ >> > The datasheet lacks detailed information about secure vs. non-secure mode >> > and how the two register sets are supposed to interoperate and be handled >> > by the operating system. >> >> I don't know about that either. >> >> But how about differences within R-Car Gen2 series so far? If there >> are known differences then perhaps we should use part number in the >> compatible string? > > I haven't noticed differences between H2 and M2. V2 and E2 are extended with > IOMMU performance monitoring registers, so a specific compatible string is > needed. Ok! > "renesas,ipmmu-vmsa" should be the default fallback compatible string. I was > thinking about playing with the newer SoCs first to see how the IOMMU react > and then most likely add SoC-specific compatible strings. Sounds good, thanks! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi index 78d637135e77..fb4803084742 100644 --- a/arch/arm/boot/dts/r8a7791.dtsi +++ b/arch/arm/boot/dts/r8a7791.dtsi @@ -1384,6 +1384,62 @@ status = "disabled"; }; + ipmmu_sy0: mmu@e6280800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xe6280800 0 0x800>; + interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_sy1: mmu@e6290800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xe6290800 0 0x800>; + interrupts = <0 225 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_ds: mmu@e6740800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xe6740800 0 0x800>; + interrupts = <0 198 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_mp: mmu@ec680800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xec680800 0 0x800>; + interrupts = <0 226 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_mx: mmu@fe951800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xfe951800 0 0x800>; + interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_rt: mmu@ffc80800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xffc80800 0 0x800>; + interrupts = <0 307 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + + ipmmu_gp: mmu@e62a0800 { + compatible = "renesas,ipmmu-vmsa"; + reg = <0 0xe62a0800 0 0x800>; + interrupts = <0 260 IRQ_TYPE_LEVEL_HIGH>; + #iommu-cells = <1>; + status = "disabled"; + }; + rcar_sound: rcar_sound@ec500000 { #sound-dai-cells = <1>; compatible = "renesas,rcar_sound-r8a7791", "renesas,rcar_sound-gen2", "renesas,rcar_sound";
Add the seven IPMMU instances found in the r8a7791 to DT with a disabled status. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- arch/arm/boot/dts/r8a7791.dtsi | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)