| Message ID | 1392315347-32967-4-git-send-email-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 02/13/2014 07:15 PM, Suman Anna wrote: > From: Florian Vaussard <florian.vaussard@epfl.ch> > > This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > the standard bindings used by OMAP peripherals, this patch uses a > 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > [s-anna@ti.com: split bindings document, add dra7 and bus error back] > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > > diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > new file mode 100644 > index 0000000..116492d > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > @@ -0,0 +1,28 @@ > +OMAP2+ IOMMU > + > +Required properties: > +- compatible : Should be one of, > + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > + "ti,dra7-iommu" for DRA7xx IOMMU instances > +- ti,hwmods : Name of the hwmod associated with the IOMMU instance > +- reg : Address space for the configuration registers > +- interrupts : Interrupt specifier for the IOMMU instance > +- dma-window : IOVA start address and length > + > +Optional properties: > +- ti,#tlb-entries : Number of entries in the translation look-aside buffer. > + Should be either 8 or 32 (default: 32) > +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing > + back a bus error response on MMU faults. > + > +Example: > + /* OMAP3 ISP MMU */ > + mmu_isp: mmu@480bd400 { > + compatible = "ti,omap2-iommu"; > + reg = <0x480bd400 0x80>; > + interrupts = <24>; > + ti,hwmods = "mmu_isp"; > + ti,#tlb-entries = <8>; > + dma-window = <0 0xfffff000>; > + }; > Any comments on this binding? Regards, Florian
Mark, Kumar, Rob, > On 02/13/2014 07:15 PM, Suman Anna wrote: >> From: Florian Vaussard <florian.vaussard@epfl.ch> >> >> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >> the standard bindings used by OMAP peripherals, this patch uses a >> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >> >> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >> [s-anna@ti.com: split bindings document, add dra7 and bus error back] >> Signed-off-by: Suman Anna <s-anna@ti.com> >> --- >> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> >> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> new file mode 100644 >> index 0000000..116492d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> @@ -0,0 +1,28 @@ >> +OMAP2+ IOMMU >> + >> +Required properties: >> +- compatible : Should be one of, >> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >> + "ti,dra7-iommu" for DRA7xx IOMMU instances >> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance >> +- reg : Address space for the configuration registers >> +- interrupts : Interrupt specifier for the IOMMU instance >> +- dma-window : IOVA start address and length >> + >> +Optional properties: >> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer. >> + Should be either 8 or 32 (default: 32) >> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing >> + back a bus error response on MMU faults. >> + >> +Example: >> + /* OMAP3 ISP MMU */ >> + mmu_isp: mmu@480bd400 { >> + compatible = "ti,omap2-iommu"; >> + reg = <0x480bd400 0x80>; >> + interrupts = <24>; >> + ti,hwmods = "mmu_isp"; >> + ti,#tlb-entries = <8>; >> + dma-window = <0 0xfffff000>; >> + }; >> > > Any comments on this binding? Can one of you look through this binding and ack/suggest changes? As I mentioned in the cover letter, the optional properties usually apply only to specific subsystems, so do let me know if this should be handled using compatible string per subsystem per SoC rather than per SoC. regards Suman
Hi Suman, Thank you for the patch. On Thursday 13 February 2014 12:15:34 Suman Anna wrote: > From: Florian Vaussard <florian.vaussard@epfl.ch> > > This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > the standard bindings used by OMAP peripherals, this patch uses a > 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > [s-anna@ti.com: split bindings document, add dra7 and bus error back] > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > > diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode > 100644 > index 0000000..116492d > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > @@ -0,0 +1,28 @@ > +OMAP2+ IOMMU > + > +Required properties: > +- compatible : Should be one of, > + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > + "ti,dra7-iommu" for DRA7xx IOMMU instances > +- ti,hwmods : Name of the hwmod associated with the IOMMU instance > +- reg : Address space for the configuration registers > +- interrupts : Interrupt specifier for the IOMMU instance > +- dma-window : IOVA start address and length Isn't the dma window more of a system configuration property than a hardware property ? How do you expect it to be set ? > +Optional properties: > +- ti,#tlb-entries : Number of entries in the translation look-aside buffer. > + Should be either 8 or 32 (default: 32) > +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing > + back a bus error response on MMU faults. Do these features vary per IOMMU instance or per IOMMU model ? In the latter case they could be inferred from the compatible string by the driver without requiring them to be explicit in DT (whether you want to do so is left to you though). > +Example: > + /* OMAP3 ISP MMU */ > + mmu_isp: mmu@480bd400 { > + compatible = "ti,omap2-iommu"; > + reg = <0x480bd400 0x80>; > + interrupts = <24>; > + ti,hwmods = "mmu_isp"; > + ti,#tlb-entries = <8>; > + dma-window = <0 0xfffff000>; > + };
Hi Laurent, On 02/25/2014 03:26 PM, Laurent Pinchart wrote: > Hi Suman, > > Thank you for the patch. > > On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >> From: Florian Vaussard <florian.vaussard@epfl.ch> >> >> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >> the standard bindings used by OMAP peripherals, this patch uses a >> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >> >> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >> [s-anna@ti.com: split bindings document, add dra7 and bus error back] >> Signed-off-by: Suman Anna <s-anna@ti.com> >> --- >> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> >> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode >> 100644 >> index 0000000..116492d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >> @@ -0,0 +1,28 @@ >> +OMAP2+ IOMMU >> + >> +Required properties: >> +- compatible : Should be one of, >> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >> + "ti,dra7-iommu" for DRA7xx IOMMU instances >> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance >> +- reg : Address space for the configuration registers >> +- interrupts : Interrupt specifier for the IOMMU instance >> +- dma-window : IOVA start address and length > > Isn't the dma window more of a system configuration property than a hardware > property ? How do you expect it to be set? We are setting it based on the addressable range for the MMU. We are reusing the existing defined property and it allows us to get rid of the IOVA start and end addresses defined in the pre-DT OMAP iommu platform data. > >> +Optional properties: >> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer. >> + Should be either 8 or 32 (default: 32) >> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing >> + back a bus error response on MMU faults. > > Do these features vary per IOMMU instance or per IOMMU model ? In the latter > case they could be inferred from the compatible string by the driver without > requiring them to be explicit in DT (whether you want to do so is left to you > though). Well, these are fixed features given an IOMMU instance, like the OMAP3 ISP is the only one that has 8 TLB entries, all the remaining ones have 32, and the IPU iommu instances are the only ones that support the bus error response back. I have no preference to any particular way, and sure the driver can infer these easily based on unique compatible strings per subsystem per SoC. I just happened to go with defining compatible strings per SoC, with the optional properties differentiating the fixed behavior between different IOMMU instances on that SoC. This is where I was looking for some inputs/guidance from the DT bindings maintainers on what is the preferred method. regards Suman > >> +Example: >> + /* OMAP3 ISP MMU */ >> + mmu_isp: mmu@480bd400 { >> + compatible = "ti,omap2-iommu"; >> + reg = <0x480bd400 0x80>; >> + interrupts = <24>; >> + ti,hwmods = "mmu_isp"; >> + ti,#tlb-entries = <8>; >> + dma-window = <0 0xfffff000>; >> + }; >
Hi Suman, On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: > On 02/25/2014 03:26 PM, Laurent Pinchart wrote: > > On Thursday 13 February 2014 12:15:34 Suman Anna wrote: > >> From: Florian Vaussard <florian.vaussard@epfl.ch> > >> > >> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > >> the standard bindings used by OMAP peripherals, this patch uses a > >> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > >> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > >> > >> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > >> [s-anna@ti.com: split bindings document, add dra7 and bus error back] > >> Signed-off-by: Suman Anna <s-anna@ti.com> > >> --- > >> > >> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 > >> +++++++++++++++++++ > >> 1 file changed, 28 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >> > >> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode > >> 100644 > >> index 0000000..116492d > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >> @@ -0,0 +1,28 @@ > >> +OMAP2+ IOMMU > >> + > >> +Required properties: > >> +- compatible : Should be one of, > >> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > >> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > >> + "ti,dra7-iommu" for DRA7xx IOMMU instances > >> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance > >> +- reg : Address space for the configuration registers > >> +- interrupts : Interrupt specifier for the IOMMU instance > >> +- dma-window : IOVA start address and length > > > > Isn't the dma window more of a system configuration property than a > > hardware property ? How do you expect it to be set? > > We are setting it based on the addressable range for the MMU. A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both support the full 4GB VA space. Why do you need to restrict it ? > We are reusing the existing defined property and it allows us to get rid of > the IOVA start and end addresses defined in the pre-DT OMAP iommu platform > data. > > >> +Optional properties: > >> +- ti,#tlb-entries : Number of entries in the translation look-aside > >> buffer. + Should be either 8 or 32 (default: 32) > >> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing > >> + back a bus error response on MMU faults. > > > > Do these features vary per IOMMU instance or per IOMMU model ? In the > > latter case they could be inferred from the compatible string by the > > driver without requiring them to be explicit in DT (whether you want to > > do so is left to you though). > > Well, these are fixed features given an IOMMU instance, like the OMAP3 > ISP is the only one that has 8 TLB entries, all the remaining ones have > 32, and the IPU iommu instances are the only ones that support the bus > error response back. I have no preference to any particular way, and > sure the driver can infer these easily based on unique compatible > strings per subsystem per SoC. I just happened to go with defining > compatible strings per SoC, with the optional properties differentiating > the fixed behavior between different IOMMU instances on that SoC. This > is where I was looking for some inputs/guidance from the DT bindings > maintainers on what is the preferred method. I think you've made the right choice. I wasn't sure whether those parameters varied across IOMMU instances of compatible devices (from a compatible string point of view) or were constant. As they vary they should be expressed in DT. > >> +Example: > >> + /* OMAP3 ISP MMU */ > >> + mmu_isp: mmu@480bd400 { > >> + compatible = "ti,omap2-iommu"; > >> + reg = <0x480bd400 0x80>; > >> + interrupts = <24>; > >> + ti,hwmods = "mmu_isp"; > >> + ti,#tlb-entries = <8>; > >> + dma-window = <0 0xfffff000>; > >> + };
Hi Laurent, On 02/25/2014 08:13 PM, Laurent Pinchart wrote: > Hi Suman, > > On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: >> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: >>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >>>> From: Florian Vaussard <florian.vaussard@epfl.ch> >>>> >>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >>>> the standard bindings used by OMAP peripherals, this patch uses a >>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >>>> >>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back] >>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>> --- >>>> >>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 >>>> +++++++++++++++++++ >>>> 1 file changed, 28 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode >>>> 100644 >>>> index 0000000..116492d >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>> @@ -0,0 +1,28 @@ >>>> +OMAP2+ IOMMU >>>> + >>>> +Required properties: >>>> +- compatible : Should be one of, >>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances >>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance >>>> +- reg : Address space for the configuration registers >>>> +- interrupts : Interrupt specifier for the IOMMU instance >>>> +- dma-window : IOVA start address and length >>> >>> Isn't the dma window more of a system configuration property than a >>> hardware property ? How do you expect it to be set? >> >> We are setting it based on the addressable range for the MMU. > > A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both support > the full 4GB VA space. Why do you need to restrict it ? I should have rephrased it better when I said addressable range. While the MMUs are capable of programming the full 4GB space, there are some address ranges that are private from the processor view. This window is currently used to set the range for the omap-iovmm driver (which only OMAP3 ISP is using atm), and there is no point in allowing the omap-iovmm driver the full range when the processor could never reach/access those addresses. > >> We are reusing the existing defined property and it allows us to get rid of >> the IOVA start and end addresses defined in the pre-DT OMAP iommu platform >> data. >> >>>> +Optional properties: >>>> +- ti,#tlb-entries : Number of entries in the translation look-aside >>>> buffer. + Should be either 8 or 32 (default: 32) >>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing >>>> + back a bus error response on MMU faults. >>> >>> Do these features vary per IOMMU instance or per IOMMU model ? In the >>> latter case they could be inferred from the compatible string by the >>> driver without requiring them to be explicit in DT (whether you want to >>> do so is left to you though). >> >> Well, these are fixed features given an IOMMU instance, like the OMAP3 >> ISP is the only one that has 8 TLB entries, all the remaining ones have >> 32, and the IPU iommu instances are the only ones that support the bus >> error response back. I have no preference to any particular way, and >> sure the driver can infer these easily based on unique compatible >> strings per subsystem per SoC. I just happened to go with defining >> compatible strings per SoC, with the optional properties differentiating >> the fixed behavior between different IOMMU instances on that SoC. This >> is where I was looking for some inputs/guidance from the DT bindings >> maintainers on what is the preferred method. > > I think you've made the right choice. I wasn't sure whether those parameters > varied across IOMMU instances of compatible devices (from a compatible string > point of view) or were constant. As they vary they should be expressed in DT. Yeah, I wasn't sure if these qualify as features (as per Documentation/devicetree/bindings/ABI.txt section II.2). regards Suman > >>>> +Example: >>>> + /* OMAP3 ISP MMU */ >>>> + mmu_isp: mmu@480bd400 { >>>> + compatible = "ti,omap2-iommu"; >>>> + reg = <0x480bd400 0x80>; >>>> + interrupts = <24>; >>>> + ti,hwmods = "mmu_isp"; >>>> + ti,#tlb-entries = <8>; >>>> + dma-window = <0 0xfffff000>; >>>> + }; >
Hi Suman, On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: > On 02/25/2014 08:13 PM, Laurent Pinchart wrote: > > On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: > >> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: > >>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: > >>>> From: Florian Vaussard <florian.vaussard@epfl.ch> > >>>> > >>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > >>>> the standard bindings used by OMAP peripherals, this patch uses a > >>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > >>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > >>>> > >>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > >>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back] > >>>> Signed-off-by: Suman Anna <s-anna@ti.com> > >>>> --- > >>>> > >>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++++ > >>>> 1 file changed, 28 insertions(+) > >>>> create mode 100644 > >>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file > >>>> mode 100644 > >>>> index 0000000..116492d > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>> @@ -0,0 +1,28 @@ > >>>> +OMAP2+ IOMMU > >>>> + > >>>> +Required properties: > >>>> +- compatible : Should be one of, > >>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > >>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > >>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances > >>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance > >>>> +- reg : Address space for the configuration registers > >>>> +- interrupts : Interrupt specifier for the IOMMU instance > >>>> +- dma-window : IOVA start address and length > >>> > >>> Isn't the dma window more of a system configuration property than a > >>> hardware property ? How do you expect it to be set? > >> > >> We are setting it based on the addressable range for the MMU. > > > > A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both > > support the full 4GB VA space. Why do you need to restrict it ? > > I should have rephrased it better when I said addressable range. While > the MMUs are capable of programming the full 4GB space, there are some > address ranges that are private from the processor view. This window is > currently used to set the range for the omap-iovmm driver (which only > OMAP3 ISP is using atm), and there is no point in allowing the > omap-iovmm driver the full range when the processor could never > reach/access those addresses. But the IOMMU VA space is from a device point of view, not from a CPU point of view. Could you point me to where those private ranges are documented, in order to understand the problem correctly ? > >> We are reusing the existing defined property and it allows us to get rid > >> of the IOVA start and end addresses defined in the pre-DT OMAP iommu > >> platform data. > >> > >>>> +Optional properties: > >>>> +- ti,#tlb-entries : Number of entries in the translation look-aside > >>>> buffer. + Should be either 8 or 32 (default: 32) > >>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports > >>>> throwing > >>>> + back a bus error response on MMU faults. > >>> > >>> Do these features vary per IOMMU instance or per IOMMU model ? In the > >>> latter case they could be inferred from the compatible string by the > >>> driver without requiring them to be explicit in DT (whether you want to > >>> do so is left to you though). > >> > >> Well, these are fixed features given an IOMMU instance, like the OMAP3 > >> ISP is the only one that has 8 TLB entries, all the remaining ones have > >> 32, and the IPU iommu instances are the only ones that support the bus > >> error response back. I have no preference to any particular way, and > >> sure the driver can infer these easily based on unique compatible > >> strings per subsystem per SoC. I just happened to go with defining > >> compatible strings per SoC, with the optional properties differentiating > >> the fixed behavior between different IOMMU instances on that SoC. This > >> is where I was looking for some inputs/guidance from the DT bindings > >> maintainers on what is the preferred method. > > > > I think you've made the right choice. I wasn't sure whether those > > parameters varied across IOMMU instances of compatible devices (from a > > compatible string point of view) or were constant. As they vary they > > should be expressed in DT. > > Yeah, I wasn't sure if these qualify as features (as per > Documentation/devicetree/bindings/ABI.txt section II.2). > > regards > Suman > > >>>> +Example: > >>>> + /* OMAP3 ISP MMU */ > >>>> + mmu_isp: mmu@480bd400 { > >>>> + compatible = "ti,omap2-iommu"; > >>>> + reg = <0x480bd400 0x80>; > >>>> + interrupts = <24>; > >>>> + ti,hwmods = "mmu_isp"; > >>>> + ti,#tlb-entries = <8>; > >>>> + dma-window = <0 0xfffff000>; > >>>> + };
Hi Laurent, > > On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: >> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: >>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: >>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: >>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>> >>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >>>>>> the standard bindings used by OMAP peripherals, this patch uses a >>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >>>>>> >>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back] >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>>> --- >>>>>> >>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++++ >>>>>> 1 file changed, 28 insertions(+) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file >>>>>> mode 100644 >>>>>> index 0000000..116492d >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>> @@ -0,0 +1,28 @@ >>>>>> +OMAP2+ IOMMU >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : Should be one of, >>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances >>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance >>>>>> +- reg : Address space for the configuration registers >>>>>> +- interrupts : Interrupt specifier for the IOMMU instance >>>>>> +- dma-window : IOVA start address and length >>>>> >>>>> Isn't the dma window more of a system configuration property than a >>>>> hardware property ? How do you expect it to be set? >>>> >>>> We are setting it based on the addressable range for the MMU. >>> >>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both >>> support the full 4GB VA space. Why do you need to restrict it ? >> >> I should have rephrased it better when I said addressable range. While >> the MMUs are capable of programming the full 4GB space, there are some >> address ranges that are private from the processor view. This window is >> currently used to set the range for the omap-iovmm driver (which only >> OMAP3 ISP is using atm), and there is no point in allowing the >> omap-iovmm driver the full range when the processor could never >> reach/access those addresses. > > But the IOMMU VA space is from a device point of view, not from a CPU point of > view. Could you point me to where those private ranges are documented, in > order to understand the problem correctly ? Yes, they are indeed from the device perspective. I meant DSP and/or IPU by processor. For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external addressable range starts from 0x11000000. regards Suman > >>>> We are reusing the existing defined property and it allows us to get rid >>>> of the IOVA start and end addresses defined in the pre-DT OMAP iommu >>>> platform data. >>>> >>>>>> +Optional properties: >>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside >>>>>> buffer. + Should be either 8 or 32 (default: 32) >>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports >>>>>> throwing >>>>>> + back a bus error response on MMU faults. >>>>> >>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the >>>>> latter case they could be inferred from the compatible string by the >>>>> driver without requiring them to be explicit in DT (whether you want to >>>>> do so is left to you though). >>>> >>>> Well, these are fixed features given an IOMMU instance, like the OMAP3 >>>> ISP is the only one that has 8 TLB entries, all the remaining ones have >>>> 32, and the IPU iommu instances are the only ones that support the bus >>>> error response back. I have no preference to any particular way, and >>>> sure the driver can infer these easily based on unique compatible >>>> strings per subsystem per SoC. I just happened to go with defining >>>> compatible strings per SoC, with the optional properties differentiating >>>> the fixed behavior between different IOMMU instances on that SoC. This >>>> is where I was looking for some inputs/guidance from the DT bindings >>>> maintainers on what is the preferred method. >>> >>> I think you've made the right choice. I wasn't sure whether those >>> parameters varied across IOMMU instances of compatible devices (from a >>> compatible string point of view) or were constant. As they vary they >>> should be expressed in DT. >> >> Yeah, I wasn't sure if these qualify as features (as per >> Documentation/devicetree/bindings/ABI.txt section II.2). >> >> regards >> Suman >> >>>>>> +Example: >>>>>> + /* OMAP3 ISP MMU */ >>>>>> + mmu_isp: mmu@480bd400 { >>>>>> + compatible = "ti,omap2-iommu"; >>>>>> + reg = <0x480bd400 0x80>; >>>>>> + interrupts = <24>; >>>>>> + ti,hwmods = "mmu_isp"; >>>>>> + ti,#tlb-entries = <8>; >>>>>> + dma-window = <0 0xfffff000>; >>>>>> + }; >
Hi Suman, On Wednesday 26 February 2014 14:23:03 Suman Anna wrote: > > On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: > >> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: > >>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: > >>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: > >>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: > >>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> > >>>>>> > >>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > >>>>>> the standard bindings used by OMAP peripherals, this patch uses a > >>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > >>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > >>>>>> > >>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > >>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back] > >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> > >>>>>> --- > >>>>>> > >>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++ > >>>>>> 1 file changed, 28 insertions(+) > >>>>>> create mode 100644 > >>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>> > >>>>>> diff --git > >>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file > >>>>>> mode 100644 > >>>>>> index 0000000..116492d > >>>>>> --- /dev/null > >>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>> @@ -0,0 +1,28 @@ > >>>>>> +OMAP2+ IOMMU > >>>>>> + > >>>>>> +Required properties: > >>>>>> +- compatible : Should be one of, > >>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > >>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > >>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances > >>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance > >>>>>> +- reg : Address space for the configuration registers > >>>>>> +- interrupts : Interrupt specifier for the IOMMU instance > >>>>>> +- dma-window : IOVA start address and length > >>>>> > >>>>> Isn't the dma window more of a system configuration property than a > >>>>> hardware property ? How do you expect it to be set? > >>>> > >>>> We are setting it based on the addressable range for the MMU. > >>> > >>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both > >>> support the full 4GB VA space. Why do you need to restrict it ? > >> > >> I should have rephrased it better when I said addressable range. While > >> the MMUs are capable of programming the full 4GB space, there are some > >> address ranges that are private from the processor view. This window is > >> currently used to set the range for the omap-iovmm driver (which only > >> OMAP3 ISP is using atm), and there is no point in allowing the > >> omap-iovmm driver the full range when the processor could never > >> reach/access those addresses. > > > > But the IOMMU VA space is from a device point of view, not from a CPU > > point of view. Could you point me to where those private ranges are > > documented, in order to understand the problem correctly ? > > Yes, they are indeed from the device perspective. I meant DSP and/or IPU > by processor. > > For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP > Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external > addressable range starts from 0x11000000. OK, so it looks more like a property of the IOMMU master than a property of the IOMMU itself. It would be better to express it as such, but I wonder how that could be done, and if it would be worth it in this case. As not all masters (the OMAP3 ISP doesn't for instance) have restrictions regarding the VA range they can address, should this property be at least made optional ? > >>>> We are reusing the existing defined property and it allows us to get > >>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP > >>>> iommu platform data. > >>>> > >>>>>> +Optional properties: > >>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside > >>>>>> buffer. + Should be either 8 or 32 (default: 32) > >>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports > >>>>>> throwing > >>>>>> + back a bus error response on MMU faults. > >>>>> > >>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the > >>>>> latter case they could be inferred from the compatible string by the > >>>>> driver without requiring them to be explicit in DT (whether you want > >>>>> to do so is left to you though). > >>>> > >>>> Well, these are fixed features given an IOMMU instance, like the OMAP3 > >>>> ISP is the only one that has 8 TLB entries, all the remaining ones have > >>>> 32, and the IPU iommu instances are the only ones that support the bus > >>>> error response back. I have no preference to any particular way, and > >>>> sure the driver can infer these easily based on unique compatible > >>>> strings per subsystem per SoC. I just happened to go with defining > >>>> compatible strings per SoC, with the optional properties > >>>> differentiating the fixed behavior between different IOMMU instances on > >>>> that SoC. This is where I was looking for some inputs/guidance from the > >>>> DT bindings maintainers on what is the preferred method. > >>> > >>> I think you've made the right choice. I wasn't sure whether those > >>> parameters varied across IOMMU instances of compatible devices (from a > >>> compatible string point of view) or were constant. As they vary they > >>> should be expressed in DT. > >> > >> Yeah, I wasn't sure if these qualify as features (as per > >> Documentation/devicetree/bindings/ABI.txt section II.2). > >> > >> regards > >> Suman > >> > >>>>>> +Example: > >>>>>> + /* OMAP3 ISP MMU */ > >>>>>> + mmu_isp: mmu@480bd400 { > >>>>>> + compatible = "ti,omap2-iommu"; > >>>>>> + reg = <0x480bd400 0x80>; > >>>>>> + interrupts = <24>; > >>>>>> + ti,hwmods = "mmu_isp"; > >>>>>> + ti,#tlb-entries = <8>; > >>>>>> + dma-window = <0 0xfffff000>; > >>>>>> + };
Hi Laurent, On 02/26/2014 02:36 PM, Laurent Pinchart wrote: > Hi Suman, > > On Wednesday 26 February 2014 14:23:03 Suman Anna wrote: >>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: >>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: >>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: >>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: >>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>> >>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a >>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >>>>>>>> >>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back] >>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>>>>> --- >>>>>>>> >>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++ >>>>>>>> 1 file changed, 28 insertions(+) >>>>>>>> create mode 100644 >>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file >>>>>>>> mode 100644 >>>>>>>> index 0000000..116492d >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>> @@ -0,0 +1,28 @@ >>>>>>>> +OMAP2+ IOMMU >>>>>>>> + >>>>>>>> +Required properties: >>>>>>>> +- compatible : Should be one of, >>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances >>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance >>>>>>>> +- reg : Address space for the configuration registers >>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance >>>>>>>> +- dma-window : IOVA start address and length >>>>>>> >>>>>>> Isn't the dma window more of a system configuration property than a >>>>>>> hardware property ? How do you expect it to be set? >>>>>> >>>>>> We are setting it based on the addressable range for the MMU. >>>>> >>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both >>>>> support the full 4GB VA space. Why do you need to restrict it ? >>>> >>>> I should have rephrased it better when I said addressable range. While >>>> the MMUs are capable of programming the full 4GB space, there are some >>>> address ranges that are private from the processor view. This window is >>>> currently used to set the range for the omap-iovmm driver (which only >>>> OMAP3 ISP is using atm), and there is no point in allowing the >>>> omap-iovmm driver the full range when the processor could never >>>> reach/access those addresses. >>> >>> But the IOMMU VA space is from a device point of view, not from a CPU >>> point of view. Could you point me to where those private ranges are >>> documented, in order to understand the problem correctly ? >> >> Yes, they are indeed from the device perspective. I meant DSP and/or IPU >> by processor. >> >> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP >> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external >> addressable range starts from 0x11000000. > > OK, so it looks more like a property of the IOMMU master than a property of > the IOMMU itself. It would be better to express it as such, but I wonder how > that could be done, and if it would be worth it in this case. This property is currently solely used to configure the range for the omap-iovmm module, which were supplied through platform data in the non-DT case. I am wondering if the way to go here is to use iommu_domain_set_attr() and use the domain geometry values. regards Suman > > As not all masters (the OMAP3 ISP doesn't for instance) have restrictions > regarding the VA range they can address, should this property be at least made > optional ? > >>>>>> We are reusing the existing defined property and it allows us to get >>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP >>>>>> iommu platform data. >>>>>> >>>>>>>> +Optional properties: >>>>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside >>>>>>>> buffer. + Should be either 8 or 32 (default: 32) >>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports >>>>>>>> throwing >>>>>>>> + back a bus error response on MMU faults. >>>>>>> >>>>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the >>>>>>> latter case they could be inferred from the compatible string by the >>>>>>> driver without requiring them to be explicit in DT (whether you want >>>>>>> to do so is left to you though). >>>>>> >>>>>> Well, these are fixed features given an IOMMU instance, like the OMAP3 >>>>>> ISP is the only one that has 8 TLB entries, all the remaining ones have >>>>>> 32, and the IPU iommu instances are the only ones that support the bus >>>>>> error response back. I have no preference to any particular way, and >>>>>> sure the driver can infer these easily based on unique compatible >>>>>> strings per subsystem per SoC. I just happened to go with defining >>>>>> compatible strings per SoC, with the optional properties >>>>>> differentiating the fixed behavior between different IOMMU instances on >>>>>> that SoC. This is where I was looking for some inputs/guidance from the >>>>>> DT bindings maintainers on what is the preferred method. >>>>> >>>>> I think you've made the right choice. I wasn't sure whether those >>>>> parameters varied across IOMMU instances of compatible devices (from a >>>>> compatible string point of view) or were constant. As they vary they >>>>> should be expressed in DT. >>>> >>>> Yeah, I wasn't sure if these qualify as features (as per >>>> Documentation/devicetree/bindings/ABI.txt section II.2). >>>> >>>> regards >>>> Suman >>>> >>>>>>>> +Example: >>>>>>>> + /* OMAP3 ISP MMU */ >>>>>>>> + mmu_isp: mmu@480bd400 { >>>>>>>> + compatible = "ti,omap2-iommu"; >>>>>>>> + reg = <0x480bd400 0x80>; >>>>>>>> + interrupts = <24>; >>>>>>>> + ti,hwmods = "mmu_isp"; >>>>>>>> + ti,#tlb-entries = <8>; >>>>>>>> + dma-window = <0 0xfffff000>; >>>>>>>> + }; >
On 02/26/2014 04:18 PM, Suman Anna wrote: > Hi Laurent, > > On 02/26/2014 02:36 PM, Laurent Pinchart wrote: >> Hi Suman, >> >> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote: >>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: >>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: >>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: >>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: >>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >>>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>>> >>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a >>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >>>>>>>>> >>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error >>>>>>>>> back] >>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>>>>>> --- >>>>>>>>> >>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 >>>>>>>>> ++++++++++++ >>>>>>>>> 1 file changed, 28 insertions(+) >>>>>>>>> create mode 100644 >>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>> >>>>>>>>> diff --git >>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new >>>>>>>>> file >>>>>>>>> mode 100644 >>>>>>>>> index 0000000..116492d >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>> @@ -0,0 +1,28 @@ >>>>>>>>> +OMAP2+ IOMMU >>>>>>>>> + >>>>>>>>> +Required properties: >>>>>>>>> +- compatible : Should be one of, >>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances >>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU >>>>>>>>> instance >>>>>>>>> +- reg : Address space for the configuration registers >>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance >>>>>>>>> +- dma-window : IOVA start address and length >>>>>>>> >>>>>>>> Isn't the dma window more of a system configuration property than a >>>>>>>> hardware property ? How do you expect it to be set? >>>>>>> >>>>>>> We are setting it based on the addressable range for the MMU. >>>>>> >>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both >>>>>> support the full 4GB VA space. Why do you need to restrict it ? >>>>> >>>>> I should have rephrased it better when I said addressable range. While >>>>> the MMUs are capable of programming the full 4GB space, there are some >>>>> address ranges that are private from the processor view. This >>>>> window is >>>>> currently used to set the range for the omap-iovmm driver (which only >>>>> OMAP3 ISP is using atm), and there is no point in allowing the >>>>> omap-iovmm driver the full range when the processor could never >>>>> reach/access those addresses. >>>> >>>> But the IOMMU VA space is from a device point of view, not from a CPU >>>> point of view. Could you point me to where those private ranges are >>>> documented, in order to understand the problem correctly ? >>> >>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU >>> by processor. >>> >>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP >>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external >>> addressable range starts from 0x11000000. >> >> OK, so it looks more like a property of the IOMMU master than a >> property of >> the IOMMU itself. It would be better to express it as such, but I >> wonder how >> that could be done, and if it would be worth it in this case. > > This property is currently solely used to configure the range for the > omap-iovmm module, which were supplied through platform data in the > non-DT case. I am wondering if the way to go here is to use > iommu_domain_set_attr() and use the domain geometry values. The other option is to supply these as driver match data, and switching the compatible strings to identify the MMU instance precisely. regards Suman >> >> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions >> regarding the VA range they can address, should this property be at >> least made >> optional ? >> >>>>>>> We are reusing the existing defined property and it allows us to get >>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP >>>>>>> iommu platform data. >>>>>>> >>>>>>>>> +Optional properties: >>>>>>>>> +- ti,#tlb-entries : Number of entries in the translation >>>>>>>>> look-aside >>>>>>>>> buffer. + Should be either 8 or 32 (default: >>>>>>>>> 32) >>>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports >>>>>>>>> throwing >>>>>>>>> + back a bus error response on MMU faults. >>>>>>>> >>>>>>>> Do these features vary per IOMMU instance or per IOMMU model ? >>>>>>>> In the >>>>>>>> latter case they could be inferred from the compatible string by >>>>>>>> the >>>>>>>> driver without requiring them to be explicit in DT (whether you >>>>>>>> want >>>>>>>> to do so is left to you though). >>>>>>> >>>>>>> Well, these are fixed features given an IOMMU instance, like the >>>>>>> OMAP3 >>>>>>> ISP is the only one that has 8 TLB entries, all the remaining >>>>>>> ones have >>>>>>> 32, and the IPU iommu instances are the only ones that support >>>>>>> the bus >>>>>>> error response back. I have no preference to any particular way, and >>>>>>> sure the driver can infer these easily based on unique compatible >>>>>>> strings per subsystem per SoC. I just happened to go with defining >>>>>>> compatible strings per SoC, with the optional properties >>>>>>> differentiating the fixed behavior between different IOMMU >>>>>>> instances on >>>>>>> that SoC. This is where I was looking for some inputs/guidance >>>>>>> from the >>>>>>> DT bindings maintainers on what is the preferred method. >>>>>> >>>>>> I think you've made the right choice. I wasn't sure whether those >>>>>> parameters varied across IOMMU instances of compatible devices >>>>>> (from a >>>>>> compatible string point of view) or were constant. As they vary they >>>>>> should be expressed in DT. >>>>> >>>>> Yeah, I wasn't sure if these qualify as features (as per >>>>> Documentation/devicetree/bindings/ABI.txt section II.2). >>>>> >>>>> regards >>>>> Suman >>>>> >>>>>>>>> +Example: >>>>>>>>> + /* OMAP3 ISP MMU */ >>>>>>>>> + mmu_isp: mmu@480bd400 { >>>>>>>>> + compatible = "ti,omap2-iommu"; >>>>>>>>> + reg = <0x480bd400 0x80>; >>>>>>>>> + interrupts = <24>; >>>>>>>>> + ti,hwmods = "mmu_isp"; >>>>>>>>> + ti,#tlb-entries = <8>; >>>>>>>>> + dma-window = <0 0xfffff000>; >>>>>>>>> + }; >> >
Hi Suman, On Wednesday 26 February 2014 16:28:08 Suman Anna wrote: > On 02/26/2014 04:18 PM, Suman Anna wrote: > > On 02/26/2014 02:36 PM, Laurent Pinchart wrote: > >> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote: > >>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: > >>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: > >>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: > >>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: > >>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: > >>>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> > >>>>>>>>> > >>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from > >>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a > >>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom > >>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > >>>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error > >>>>>>>>> back] > >>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> > >>>>>>>>> --- > >>>>>>>>> > >>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++ > >>>>>>>>> 1 file changed, 28 insertions(+) > >>>>>>>>> create mode 100644 > >>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>>>>> > >>>>>>>>> diff --git > >>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new > >>>>>>>>> file > >>>>>>>>> mode 100644 > >>>>>>>>> index 0000000..116492d > >>>>>>>>> --- /dev/null > >>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt > >>>>>>>>> @@ -0,0 +1,28 @@ > >>>>>>>>> +OMAP2+ IOMMU > >>>>>>>>> + > >>>>>>>>> +Required properties: > >>>>>>>>> +- compatible : Should be one of, > >>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances > >>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances > >>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances > >>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU > >>>>>>>>> instance > >>>>>>>>> +- reg : Address space for the configuration registers > >>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance > >>>>>>>>> +- dma-window : IOVA start address and length > >>>>>>>> > >>>>>>>> Isn't the dma window more of a system configuration property than a > >>>>>>>> hardware property ? How do you expect it to be set? > >>>>>>> > >>>>>>> We are setting it based on the addressable range for the MMU. > >>>>>> > >>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both > >>>>>> support the full 4GB VA space. Why do you need to restrict it ? > >>>>> > >>>>> I should have rephrased it better when I said addressable range. While > >>>>> the MMUs are capable of programming the full 4GB space, there are some > >>>>> address ranges that are private from the processor view. This > >>>>> window is currently used to set the range for the omap-iovmm driver > >>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing > >>>>> the omap-iovmm driver the full range when the processor could never > >>>>> reach/access those addresses. > >>>> > >>>> But the IOMMU VA space is from a device point of view, not from a CPU > >>>> point of view. Could you point me to where those private ranges are > >>>> documented, in order to understand the problem correctly ? > >>> > >>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU > >>> by processor. > >>> > >>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP > >>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external > >>> addressable range starts from 0x11000000. > >> > >> OK, so it looks more like a property of the IOMMU master than a > >> property of the IOMMU itself. It would be better to express it as such, > >> but I wonder how that could be done, and if it would be worth it in this > >> case. > > > > This property is currently solely used to configure the range for the > > omap-iovmm module, which were supplied through platform data in the > > non-DT case. If I'm not mistaken omap-iovmm is something we want to get rid of. I know that the OMAP3 ISP driver is the only user of that module, and I've started working on fixing that, but I'm currently lacking time to complete the work. Now, if we get rid of omap-iovmm, does that mean that the dma-window property won't need to be specified anymore ? If so, given that the only omap-iovmm user is the OMAP3 ISP driver, I would propose to drop the property and just hardcode the value. Please let me know if there's something I've missed. > > I am wondering if the way to go here is to use iommu_domain_set_attr() and > > use the domain geometry values. > > The other option is to supply these as driver match data, and switching > the compatible strings to identify the MMU instance precisely. > > regards > Suman > > >> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions > >> regarding the VA range they can address, should this property be at > >> least made > >> optional ? > >> > >>>>>>> We are reusing the existing defined property and it allows us to get > >>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP > >>>>>>> iommu platform data. [snip]
Hi Laurent, > > On Wednesday 26 February 2014 16:28:08 Suman Anna wrote: >> On 02/26/2014 04:18 PM, Suman Anna wrote: >>> On 02/26/2014 02:36 PM, Laurent Pinchart wrote: >>>> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote: >>>>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote: >>>>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote: >>>>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote: >>>>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote: >>>>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote: >>>>>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>>>>> >>>>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from >>>>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a >>>>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom >>>>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>>>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error >>>>>>>>>>> back] >>>>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++ >>>>>>>>>>> 1 file changed, 28 insertions(+) >>>>>>>>>>> create mode 100644 >>>>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>>>> >>>>>>>>>>> diff --git >>>>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new >>>>>>>>>>> file >>>>>>>>>>> mode 100644 >>>>>>>>>>> index 0000000..116492d >>>>>>>>>>> --- /dev/null >>>>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt >>>>>>>>>>> @@ -0,0 +1,28 @@ >>>>>>>>>>> +OMAP2+ IOMMU >>>>>>>>>>> + >>>>>>>>>>> +Required properties: >>>>>>>>>>> +- compatible : Should be one of, >>>>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances >>>>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances >>>>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances >>>>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU >>>>>>>>>>> instance >>>>>>>>>>> +- reg : Address space for the configuration registers >>>>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance >>>>>>>>>>> +- dma-window : IOVA start address and length >>>>>>>>>> >>>>>>>>>> Isn't the dma window more of a system configuration property than a >>>>>>>>>> hardware property ? How do you expect it to be set? >>>>>>>>> >>>>>>>>> We are setting it based on the addressable range for the MMU. >>>>>>>> >>>>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both >>>>>>>> support the full 4GB VA space. Why do you need to restrict it ? >>>>>>> >>>>>>> I should have rephrased it better when I said addressable range. While >>>>>>> the MMUs are capable of programming the full 4GB space, there are some >>>>>>> address ranges that are private from the processor view. This >>>>>>> window is currently used to set the range for the omap-iovmm driver >>>>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing >>>>>>> the omap-iovmm driver the full range when the processor could never >>>>>>> reach/access those addresses. >>>>>> >>>>>> But the IOMMU VA space is from a device point of view, not from a CPU >>>>>> point of view. Could you point me to where those private ranges are >>>>>> documented, in order to understand the problem correctly ? >>>>> >>>>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU >>>>> by processor. >>>>> >>>>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP >>>>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external >>>>> addressable range starts from 0x11000000. >>>> >>>> OK, so it looks more like a property of the IOMMU master than a >>>> property of the IOMMU itself. It would be better to express it as such, >>>> but I wonder how that could be done, and if it would be worth it in this >>>> case. >>> >>> This property is currently solely used to configure the range for the >>> omap-iovmm module, which were supplied through platform data in the >>> non-DT case. > > If I'm not mistaken omap-iovmm is something we want to get rid of. I know that > the OMAP3 ISP driver is the only user of that module, and I've started working > on fixing that, but I'm currently lacking time to complete the work. > > Now, if we get rid of omap-iovmm, does that mean that the dma-window property > won't need to be specified anymore ? If so, given that the only omap-iovmm > user is the OMAP3 ISP driver, I would propose to drop the property and just > hardcode the value. Yeah, none of our OMAP4+ stacks use omap-iovmm, or similar dynamic reservation scheme at the moment. I am perfectly fine with dropping the property and hard-coding it in the driver with a note. DSP/Bridge has a similar usage (in dmm.c), but that code is localized within the driver. Thanks for all the comments. regards Suman > > Please let me know if there's something I've missed. > >>> I am wondering if the way to go here is to use iommu_domain_set_attr() and >>> use the domain geometry values. >> >> The other option is to supply these as driver match data, and switching >> the compatible strings to identify the MMU instance precisely. >> >> regards >> Suman >> >>>> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions >>>> regarding the VA range they can address, should this property be at >>>> least made >>>> optional ? >>>> >>>>>>>>> We are reusing the existing defined property and it allows us to get >>>>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP >>>>>>>>> iommu platform data. > > [snip] >
diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode 100644 index 0000000..116492d --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt @@ -0,0 +1,28 @@ +OMAP2+ IOMMU + +Required properties: +- compatible : Should be one of, + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances + "ti,dra7-iommu" for DRA7xx IOMMU instances +- ti,hwmods : Name of the hwmod associated with the IOMMU instance +- reg : Address space for the configuration registers +- interrupts : Interrupt specifier for the IOMMU instance +- dma-window : IOVA start address and length + +Optional properties: +- ti,#tlb-entries : Number of entries in the translation look-aside buffer. + Should be either 8 or 32 (default: 32) +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing + back a bus error response on MMU faults. + +Example: + /* OMAP3 ISP MMU */ + mmu_isp: mmu@480bd400 { + compatible = "ti,omap2-iommu"; + reg = <0x480bd400 0x80>; + interrupts = <24>; + ti,hwmods = "mmu_isp"; + ti,#tlb-entries = <8>; + dma-window = <0 0xfffff000>; + };