[PATCH v2 1/2] ARM: dts: i.MX53: add aipstz nodes

[PATCH v2 1/2] ARM: dts: i.MX53: add aipstz nodes

[Steffen Trumtrar]

The AHB to IP bridges (AIPSTZ) allow fine grained access rights management.
Add both bridges to the DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/boot/dts/imx53.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 6456a00..aaea979 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -260,6 +260,11 @@
 				};
 			};
 
+			aipstz1: bridge at 53f00000 {
+				compatible = "fsl,imx53-aipstz";
+				reg = <0x53f00000 0x60>;
+			};
+
 			usbphy0: usbphy at 0 {
 				compatible = "usb-nop-xceiv";
 				clocks = <&clks IMX5_CLK_USB_PHY1_GATE>;
@@ -572,6 +577,11 @@
 			reg = <0x60000000 0x10000000>;
 			ranges;
 
+			aipstz2: bridge at 63f00000 {
+				compatible = "fsl,imx53-aipstz";
+				reg = <0x63f00000 0x60>;
+			};
+
 			iim: iim at 63f98000 {
 				compatible = "fsl,imx53-iim", "fsl,imx27-iim";
 				reg = <0x63f98000 0x4000>;
-- 
2.0.0

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Steffen Trumtrar]

Most peripherals on the i.MX53 have an
	Off-Platform Peripheral Access Control Register (OPACR)
in which the access rights (together with the MPROT registers) can be declared.
However, this does not seem to work for example for SSI1+SDMA, because the
supervisor bit is not set for the SDMA unit.
It does work for SSI2, the QSB for example uses SSI2 for its audio. But SSI2 only
works because it does NOT have an OPACR.

The right solution would be to fix the access rights for the SDMA, but the unit
responsible for this is the Central Security Unit (CSU), which of course is NOT
documented. So, until documentation for this is openly available, turn off the
supervisor protection because it cripples the hardware.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/mach-imx/mach-imx53.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
index 2bad387..380c7ee 100644
--- a/arch/arm/mach-imx/mach-imx53.c
+++ b/arch/arm/mach-imx/mach-imx53.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <asm/mach/arch.h>
@@ -24,11 +25,24 @@
 #include "hardware.h"
 #include "mx53.h"
 
+static const struct of_device_id aips_of_matches[] __initconst = {
+	{ .compatible = "fsl,imx53-aipstz" },
+	{ /*sentinel*/ }
+};
+
 static void __init imx53_dt_init(void)
 {
+	void __iomem *aips_base_addr;
+	struct device_node *np;
+
 	mxc_arch_reset_init_dt();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
+	for_each_matching_node(np, aips_of_matches) {
+		aips_base_addr = of_iomap(np, 0);
+		imx_set_aips(aips_base_addr);
+	}
 }
 
 static const char *imx53_dt_board_compat[] __initconst = {
-- 
2.0.0

[PATCH v2 1/2] ARM: dts: i.MX53: add aipstz nodes

[Shawn Guo]

On Wed, Jun 25, 2014 at 01:01:30PM +0200, Steffen Trumtrar wrote:
> The AHB to IP bridges (AIPSTZ) allow fine grained access rights management.
> Add both bridges to the DT.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>

Applied, thanks.

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Shawn Guo]

On Wed, Jun 25, 2014 at 01:01:31PM +0200, Steffen Trumtrar wrote:
> Most peripherals on the i.MX53 have an
> 	Off-Platform Peripheral Access Control Register (OPACR)
> in which the access rights (together with the MPROT registers) can be declared.
> However, this does not seem to work for example for SSI1+SDMA, because the
> supervisor bit is not set for the SDMA unit.
> It does work for SSI2, the QSB for example uses SSI2 for its audio. But SSI2 only
> works because it does NOT have an OPACR.
> 
> The right solution would be to fix the access rights for the SDMA, but the unit
> responsible for this is the Central Security Unit (CSU), which of course is NOT
> documented. So, until documentation for this is openly available, turn off the
> supervisor protection because it cripples the hardware.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  arch/arm/mach-imx/mach-imx53.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
> index 2bad387..380c7ee 100644
> --- a/arch/arm/mach-imx/mach-imx53.c
> +++ b/arch/arm/mach-imx/mach-imx53.c
> @@ -15,6 +15,7 @@
>  #include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
> +#include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <asm/mach/arch.h>
> @@ -24,11 +25,24 @@
>  #include "hardware.h"
>  #include "mx53.h"
>  
> +static const struct of_device_id aips_of_matches[] __initconst = {
> +	{ .compatible = "fsl,imx53-aipstz" },
> +	{ /*sentinel*/ }
> +};
> +
>  static void __init imx53_dt_init(void)
>  {
> +	void __iomem *aips_base_addr;
> +	struct device_node *np;
> +
>  	mxc_arch_reset_init_dt();
>  
>  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +
> +	for_each_matching_node(np, aips_of_matches) {
> +		aips_base_addr = of_iomap(np, 0);
> +		imx_set_aips(aips_base_addr);
> +	}

The same code pattern might be needed for i.MX31 and i.MX35 device tree
support later.  Can we have a helper for it, so that i.MX3 DT conversion
can benefit from it?

Shawn

>  }
>  
>  static const char *imx53_dt_board_compat[] __initconst = {
> -- 
> 2.0.0
> 

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Steffen Trumtrar]

Hi!

On Mon, Jun 30, 2014 at 12:12:27PM +0800, Shawn Guo wrote:
> On Wed, Jun 25, 2014 at 01:01:31PM +0200, Steffen Trumtrar wrote:
> > Most peripherals on the i.MX53 have an
> > 	Off-Platform Peripheral Access Control Register (OPACR)
> > in which the access rights (together with the MPROT registers) can be declared.
> > However, this does not seem to work for example for SSI1+SDMA, because the
> > supervisor bit is not set for the SDMA unit.
> > It does work for SSI2, the QSB for example uses SSI2 for its audio. But SSI2 only
> > works because it does NOT have an OPACR.
> > 
> > The right solution would be to fix the access rights for the SDMA, but the unit
> > responsible for this is the Central Security Unit (CSU), which of course is NOT
> > documented. So, until documentation for this is openly available, turn off the
> > supervisor protection because it cripples the hardware.
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/mach-imx53.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
> > index 2bad387..380c7ee 100644
> > --- a/arch/arm/mach-imx/mach-imx53.c
> > +++ b/arch/arm/mach-imx/mach-imx53.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/err.h>
> >  #include <linux/io.h>
> >  #include <linux/irq.h>
> > +#include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <asm/mach/arch.h>
> > @@ -24,11 +25,24 @@
> >  #include "hardware.h"
> >  #include "mx53.h"
> >  
> > +static const struct of_device_id aips_of_matches[] __initconst = {
> > +	{ .compatible = "fsl,imx53-aipstz" },
> > +	{ /*sentinel*/ }
> > +};
> > +
> >  static void __init imx53_dt_init(void)
> >  {
> > +	void __iomem *aips_base_addr;
> > +	struct device_node *np;
> > +
> >  	mxc_arch_reset_init_dt();
> >  
> >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > +
> > +	for_each_matching_node(np, aips_of_matches) {
> > +		aips_base_addr = of_iomap(np, 0);
> > +		imx_set_aips(aips_base_addr);
> > +	}
> 
> The same code pattern might be needed for i.MX31 and i.MX35 device tree
> support later.  Can we have a helper for it, so that i.MX3 DT conversion
> can benefit from it?
> 

What would be the best place to put this? common.h?
Maybe we should then also rename the imx_set_aips to imx_disable_supervisor,
because that is basically what the function does.
If I understood our graphics department correctly, the i.MX6 needs this, too,
for the VPU (?).

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Shawn Guo]

On Mon, Jun 30, 2014 at 09:04:21AM +0200, Steffen Trumtrar wrote:
> Hi!
> 
> On Mon, Jun 30, 2014 at 12:12:27PM +0800, Shawn Guo wrote:
> > On Wed, Jun 25, 2014 at 01:01:31PM +0200, Steffen Trumtrar wrote:
> > > Most peripherals on the i.MX53 have an
> > > 	Off-Platform Peripheral Access Control Register (OPACR)
> > > in which the access rights (together with the MPROT registers) can be declared.
> > > However, this does not seem to work for example for SSI1+SDMA, because the
> > > supervisor bit is not set for the SDMA unit.
> > > It does work for SSI2, the QSB for example uses SSI2 for its audio. But SSI2 only
> > > works because it does NOT have an OPACR.
> > > 
> > > The right solution would be to fix the access rights for the SDMA, but the unit
> > > responsible for this is the Central Security Unit (CSU), which of course is NOT
> > > documented. So, until documentation for this is openly available, turn off the
> > > supervisor protection because it cripples the hardware.
> > > 
> > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > ---
> > >  arch/arm/mach-imx/mach-imx53.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
> > > index 2bad387..380c7ee 100644
> > > --- a/arch/arm/mach-imx/mach-imx53.c
> > > +++ b/arch/arm/mach-imx/mach-imx53.c
> > > @@ -15,6 +15,7 @@
> > >  #include <linux/err.h>
> > >  #include <linux/io.h>
> > >  #include <linux/irq.h>
> > > +#include <linux/of_address.h>
> > >  #include <linux/of_irq.h>
> > >  #include <linux/of_platform.h>
> > >  #include <asm/mach/arch.h>
> > > @@ -24,11 +25,24 @@
> > >  #include "hardware.h"
> > >  #include "mx53.h"
> > >  
> > > +static const struct of_device_id aips_of_matches[] __initconst = {
> > > +	{ .compatible = "fsl,imx53-aipstz" },
> > > +	{ /*sentinel*/ }
> > > +};
> > > +
> > >  static void __init imx53_dt_init(void)
> > >  {
> > > +	void __iomem *aips_base_addr;
> > > +	struct device_node *np;
> > > +
> > >  	mxc_arch_reset_init_dt();
> > >  
> > >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > > +
> > > +	for_each_matching_node(np, aips_of_matches) {
> > > +		aips_base_addr = of_iomap(np, 0);
> > > +		imx_set_aips(aips_base_addr);
> > > +	}
> > 
> > The same code pattern might be needed for i.MX31 and i.MX35 device tree
> > support later.  Can we have a helper for it, so that i.MX3 DT conversion
> > can benefit from it?
> > 
> 
> What would be the best place to put this? common.h?

How about the same place as imx_set_aips()?

> Maybe we should then also rename the imx_set_aips to imx_disable_supervisor,
> because that is basically what the function does.

I'm fine with either way.

> If I understood our graphics department correctly, the i.MX6 needs this, too,
> for the VPU (?).

Hmm, I do not see this setup for i.MX6 in FSL kernel tree.  Is it being
done at somewhere else?

Shawn

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Steffen Trumtrar]

Hi!

On Tue, Jul 01, 2014 at 01:24:39PM +0800, Shawn Guo wrote:
> On Mon, Jun 30, 2014 at 09:04:21AM +0200, Steffen Trumtrar wrote:
> > Hi!
> > 
> > On Mon, Jun 30, 2014 at 12:12:27PM +0800, Shawn Guo wrote:
> > > On Wed, Jun 25, 2014 at 01:01:31PM +0200, Steffen Trumtrar wrote:
> > > > Most peripherals on the i.MX53 have an
> > > > 	Off-Platform Peripheral Access Control Register (OPACR)
> > > > in which the access rights (together with the MPROT registers) can be declared.
> > > > However, this does not seem to work for example for SSI1+SDMA, because the
> > > > supervisor bit is not set for the SDMA unit.
> > > > It does work for SSI2, the QSB for example uses SSI2 for its audio. But SSI2 only
> > > > works because it does NOT have an OPACR.
> > > > 
> > > > The right solution would be to fix the access rights for the SDMA, but the unit
> > > > responsible for this is the Central Security Unit (CSU), which of course is NOT
> > > > documented. So, until documentation for this is openly available, turn off the
> > > > supervisor protection because it cripples the hardware.
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > > ---
> > > >  arch/arm/mach-imx/mach-imx53.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
> > > > index 2bad387..380c7ee 100644
> > > > --- a/arch/arm/mach-imx/mach-imx53.c
> > > > +++ b/arch/arm/mach-imx/mach-imx53.c
> > > > @@ -15,6 +15,7 @@
> > > >  #include <linux/err.h>
> > > >  #include <linux/io.h>
> > > >  #include <linux/irq.h>
> > > > +#include <linux/of_address.h>
> > > >  #include <linux/of_irq.h>
> > > >  #include <linux/of_platform.h>
> > > >  #include <asm/mach/arch.h>
> > > > @@ -24,11 +25,24 @@
> > > >  #include "hardware.h"
> > > >  #include "mx53.h"
> > > >  
> > > > +static const struct of_device_id aips_of_matches[] __initconst = {
> > > > +	{ .compatible = "fsl,imx53-aipstz" },
> > > > +	{ /*sentinel*/ }
> > > > +};
> > > > +
> > > >  static void __init imx53_dt_init(void)
> > > >  {
> > > > +	void __iomem *aips_base_addr;
> > > > +	struct device_node *np;
> > > > +
> > > >  	mxc_arch_reset_init_dt();
> > > >  
> > > >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > > > +
> > > > +	for_each_matching_node(np, aips_of_matches) {
> > > > +		aips_base_addr = of_iomap(np, 0);
> > > > +		imx_set_aips(aips_base_addr);
> > > > +	}
> > > 
> > > The same code pattern might be needed for i.MX31 and i.MX35 device tree
> > > support later.  Can we have a helper for it, so that i.MX3 DT conversion
> > > can benefit from it?
> > > 
> > 
> > What would be the best place to put this? common.h?
> 
> How about the same place as imx_set_aips()?
> 

Oh, yeah. Okay.

> > Maybe we should then also rename the imx_set_aips to imx_disable_supervisor,
> > because that is basically what the function does.
> 
> I'm fine with either way.
> 
> > If I understood our graphics department correctly, the i.MX6 needs this, too,
> > for the VPU (?).
> 
> Hmm, I do not see this setup for i.MX6 in FSL kernel tree.  Is it being
> done at somewhere else?
> 

barebox does it for i.MX6. But it would be better to have it in the kernel,
so everything works no matter which bootloader is used. AFAIK it is not needed
for anything the bootloader does.

Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 2/2] ARM: imx53: globally disable supervisor protect

[Shawn Guo]

On Tue, Jul 01, 2014 at 08:30:45AM +0200, Steffen Trumtrar wrote:
> barebox does it for i.MX6. But it would be better to have it in the kernel,
> so everything works no matter which bootloader is used. AFAIK it is not needed
> for anything the bootloader does.

Okay, sounds reasonable to me.

Shawn