[PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Kuninori Morimoto]

r8a7740 chip has lasting errata on MERAM buffer.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/setup-r8a7740.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/setup-r8a7740.c b/arch/arm/mach-shmobile/setup-r8a7740.c
index ec4eb49..a45ca43 100644
--- a/arch/arm/mach-shmobile/setup-r8a7740.c
+++ b/arch/arm/mach-shmobile/setup-r8a7740.c
@@ -324,6 +324,24 @@ static struct platform_device *r8a7740_late_devices[] __initdata = {
 	&i2c1_device,
 };
 
+/*
+ * r8a7740 chip has lasting errata on MERAM buffer.
+ * this is work-around for it.
+ * see
+ *	"Media RAM (MERAM)" on r8a7740 documentation
+ */
+#define MEBUFCNTR	0xFE950098
+static void r8a7740_meram_workaround(void)
+{
+	void __iomem *reg;
+
+	reg = ioremap_nocache(MEBUFCNTR, 4);
+	if (reg) {
+		iowrite32(0x01600164, reg);
+		iounmap(reg);
+	}
+}
+
 #define ICCR	0x0004
 #define ICSTART	0x0070
 
@@ -380,6 +398,9 @@ void __init r8a7740_add_standard_devices(void)
 	r8a7740_i2c_workaround(&i2c0_device);
 	r8a7740_i2c_workaround(&i2c1_device);
 
+	/* MERAM work-around */
+	r8a7740_meram_workaround();
+
 	platform_add_devices(r8a7740_early_devices,
 			    ARRAY_SIZE(r8a7740_early_devices));
 	platform_add_devices(r8a7740_late_devices,
-- 
1.7.5.4

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Paul Mundt]

On Mon, May 21, 2012 at 07:27:13PM -0700, kuninori.morimoto.gx@renesas.com wrote:
> --- a/arch/arm/mach-shmobile/setup-r8a7740.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7740.c
> @@ -324,6 +324,24 @@ static struct platform_device *r8a7740_late_devices[] __initdata = {
> +/*
> + * r8a7740 chip has lasting errata on MERAM buffer.
> + * this is work-around for it.
> + * see
> + *	"Media RAM (MERAM)" on r8a7740 documentation
> + */
> +#define MEBUFCNTR	0xFE950098
> +static void r8a7740_meram_workaround(void)
> +{
> +	void __iomem *reg;
> +
> +	reg = ioremap_nocache(MEBUFCNTR, 4);
> +	if (reg) {
> +		iowrite32(0x01600164, reg);
> +		iounmap(reg);
> +	}
> +}
> +
If this exists within the MERAM register space, it should probably be
handled in the MERAM driver itself. If this is all going to go to DT in
the future it will be easy to to node to quirk matching then, but in the
mean time we can match on CPU or machine types.

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Kuninori Morimoto]

Hi Paul

Thank you for checking patch

> On Mon, May 21, 2012 at 07:27:13PM -0700, kuninori.morimoto.gx@renesas.com wrote:
> > --- a/arch/arm/mach-shmobile/setup-r8a7740.c
> > +++ b/arch/arm/mach-shmobile/setup-r8a7740.c
> > @@ -324,6 +324,24 @@ static struct platform_device *r8a7740_late_devices[] __initdata = {
> > +/*
> > + * r8a7740 chip has lasting errata on MERAM buffer.
> > + * this is work-around for it.
> > + * see
> > + *	"Media RAM (MERAM)" on r8a7740 documentation
> > + */
> > +#define MEBUFCNTR	0xFE950098
> > +static void r8a7740_meram_workaround(void)
> > +{
> > +	void __iomem *reg;
> > +
> > +	reg = ioremap_nocache(MEBUFCNTR, 4);
> > +	if (reg) {
> > +		iowrite32(0x01600164, reg);
> > +		iounmap(reg);
> > +	}
> > +}
> > +
> If this exists within the MERAM register space, it should probably be
> handled in the MERAM driver itself. If this is all going to go to DT in
> the future it will be easy to to node to quirk matching then, but in the
> mean time we can match on CPU or machine types.

It is difficult. 
A1 MERAM datasheet doesn't have detail explain / register map.
It just say

------------------------------------------
Set the following register in the MERAM
 Register name: ICBR buffer control
 Address: H’FE95 0098 
 Access size: 32 bits 
 Initial value: H’0160 0168 
 Setting value: H’0160 0164 
------------------------------------------

This work-around fixup MERAM memroy access erorr bug
(CEU/VIO6C/2D-DMAC/VCP1/VPU5F/JPU/DISP)
But we don't know what is these value.

Best regards
---
Kuninori Morimoto

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Paul Mundt]

On Mon, May 21, 2012 at 09:27:03PM -0700, Kuninori Morimoto wrote:
> > If this exists within the MERAM register space, it should probably be
> > handled in the MERAM driver itself. If this is all going to go to DT in
> > the future it will be easy to to node to quirk matching then, but in the
> > mean time we can match on CPU or machine types.
> 
> It is difficult. 
> A1 MERAM datasheet doesn't have detail explain / register map.

Yes, I'm familiar with the quality/reliability of our documentation.

> It just say
> 
> ------------------------------------------
> Set the following register in the MERAM
>  Register name: ICBR buffer control
>  Address: H?$B!GFE95 0098 
>  Access size: 32 bits 
>  Initial value: H?$B!G0160 0168 
>  Setting value: H?$B!G0160 0164 
> ------------------------------------------
> 
> This work-around fixup MERAM memroy access erorr bug
> (CEU/VIO6C/2D-DMAC/VCP1/VPU5F/JPU/DISP)
> But we don't know what is these value.
> 
The problem with doing these sorts of setting outside of the context of
the MERAM driver is that the MERAM driver could just turn around and undo
the work by setting whatever bit this workaround is clearing.

As long as this exists outside of the MERAM register map and we don't
have to worry about the MERAM driver touching these values then having it
as a board-specific thing is probably fine. If it falls within the MERAM
mapping somewhere then the MERAM driver really should be dealing with it
(or we can set it as a board-specific thing when the MERAM driver isn't
being used), even if it is a magic value for the given CPU.

The main thing is avoiding subtle hard to debug problems where the board
config workarounds are inadvertently trampled later on.

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[kuninori.morimoto.gx]

Hi Paul

> The problem with doing these sorts of setting outside of the context of
> the MERAM driver is that the MERAM driver could just turn around and undo
> the work by setting whatever bit this workaround is clearing.
> 
> As long as this exists outside of the MERAM register map and we don't
> have to worry about the MERAM driver touching these values then having it
> as a board-specific thing is probably fine. If it falls within the MERAM
> mapping somewhere then the MERAM driver really should be dealing with it
> (or we can set it as a board-specific thing when the MERAM driver isn't
> being used), even if it is a magic value for the given CPU.
> 
> The main thing is avoiding subtle hard to debug problems where the board
> config workarounds are inadvertently trampled later on.

OK. I see.
There should be no necessity that we worry about the MERAM driver.
Initial setting is enough at this point.

But, I noticed that (maybe) it needs re-initialization if suspend/resume are happen.
It is not supported at this point though.

I can move this workaround from setup.c to board.c at this point.
Is it OK for now ?

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Laurent Pinchart]

Hi,

On Tuesday 22 May 2012 13:41:54 Paul Mundt wrote:
> On Mon, May 21, 2012 at 09:27:03PM -0700, Kuninori Morimoto wrote:
> > > If this exists within the MERAM register space, it should probably be
> > > handled in the MERAM driver itself. If this is all going to go to DT in
> > > the future it will be easy to to node to quirk matching then, but in the
> > > mean time we can match on CPU or machine types.
> > 
> > It is difficult.
> > A1 MERAM datasheet doesn't have detail explain / register map.
> 
> Yes, I'm familiar with the quality/reliability of our documentation.
> 
> > It just say
> > 
> > ------------------------------------------
> > Set the following register in the MERAM
> > 
> >  Register name: ICBR buffer control
> >  Address: H?$B!GFE95 0098
> >  Access size: 32 bits
> >  Initial value: H?$B!G0160 0168
> >  Setting value: H?$B!G0160 0164
> > 
> > ------------------------------------------
> > 
> > This work-around fixup MERAM memroy access erorr bug
> > (CEU/VIO6C/2D-DMAC/VCP1/VPU5F/JPU/DISP)
> > But we don't know what is these value.
> 
> The problem with doing these sorts of setting outside of the context of
> the MERAM driver is that the MERAM driver could just turn around and undo
> the work by setting whatever bit this workaround is clearing.
> 
> As long as this exists outside of the MERAM register map and we don't have
> to worry about the MERAM driver touching these values then having it as a
> board-specific thing is probably fine. If it falls within the MERAM mapping
> somewhere then the MERAM driver really should be dealing with it (or we can
> set it as a board-specific thing when the MERAM driver isn't being used),
> even if it is a magic value for the given CPU.

I agree. 0xFE950098 is outside of the MERAM registers address space (at least 
in the SH7372 datasheet I have). I'm thus fine with setting the register in 
board code.

> The main thing is avoiding subtle hard to debug problems where the board
> config workarounds are inadvertently trampled later on.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Paul Mundt]

On Tue, May 22, 2012 at 12:07:57PM +0200, Laurent Pinchart wrote:
> On Tuesday 22 May 2012 13:41:54 Paul Mundt wrote:
> > As long as this exists outside of the MERAM register map and we don't have
> > to worry about the MERAM driver touching these values then having it as a
> > board-specific thing is probably fine. If it falls within the MERAM mapping
> > somewhere then the MERAM driver really should be dealing with it (or we can
> > set it as a board-specific thing when the MERAM driver isn't being used),
> > even if it is a magic value for the given CPU.
> 
> I agree. 0xFE950098 is outside of the MERAM registers address space (at least 
> in the SH7372 datasheet I have). I'm thus fine with setting the register in 
> board code.
> 
Ok, then we can just leave it in either the board or the CPU code.
Someone will want to verify that we do not have to do anything special
for this in the suspend/resume case, though. ie, losing the magic setting
after resume.

Re: [PATCH 02/11] ARM: mach-shmobile: setup-r8a7740: add MERAM work-around

[Kuninori Morimoto]

Hi Paul, Rafael

> > On Tuesday 22 May 2012 13:41:54 Paul Mundt wrote:
> > > As long as this exists outside of the MERAM register map and we don't have
> > > to worry about the MERAM driver touching these values then having it as a
> > > board-specific thing is probably fine. If it falls within the MERAM mapping
> > > somewhere then the MERAM driver really should be dealing with it (or we can
> > > set it as a board-specific thing when the MERAM driver isn't being used),
> > > even if it is a magic value for the given CPU.
> > 
> > I agree. 0xFE950098 is outside of the MERAM registers address space (at least 
> > in the SH7372 datasheet I have). I'm thus fine with setting the register in 
> > board code.
> > 
> Ok, then we can just leave it in either the board or the CPU code.
> Someone will want to verify that we do not have to do anything special
> for this in the suspend/resume case, though. ie, losing the magic setting
> after resume.

I send v2 patch set.
This work-around will be called from board.
And tidyup confusable patch order.

Best regards
---
Kuninori Morimoto