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

[s.trumtrar@pengutronix.de (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 |