ARM diagnostic register across suspend/resume

ARM diagnostic register across suspend/resume

[Shawn Guo]

Hi Will, Catalin,

The CP15 diagnostic register holds some bits for ARM errata workaround.
Since core gets power gated across suspend/resume cycle, these bits will
get lost along the way.  Is it okay for errata workaround to continue
working after suspend, or do we have to save/restore diagnostic register
to keep workaround effective?

Shawn

ARM diagnostic register across suspend/resume

[Will Deacon]

On Tue, Jun 17, 2014 at 09:31:18AM +0100, Shawn Guo wrote:
> Hi Will, Catalin,
> 
> The CP15 diagnostic register holds some bits for ARM errata workaround.
> Since core gets power gated across suspend/resume cycle, these bits will
> get lost along the way.  Is it okay for errata workaround to continue
> working after suspend, or do we have to save/restore diagnostic register
> to keep workaround effective?

I'm not sure that saving/restoring the diagnostic register on A9 actually
works at all (I seem to remember some bits always read as zero?).

Anyway, I'd expect the state could be lost, so you'd need to reprogram
the diagnostic register rather than save/restore its value.

Will

ARM diagnostic register across suspend/resume

[Russell King - ARM Linux]

On Tue, Jun 17, 2014 at 10:57:29AM +0100, Will Deacon wrote:
> On Tue, Jun 17, 2014 at 09:31:18AM +0100, Shawn Guo wrote:
> > Hi Will, Catalin,
> > 
> > The CP15 diagnostic register holds some bits for ARM errata workaround.
> > Since core gets power gated across suspend/resume cycle, these bits will
> > get lost along the way.  Is it okay for errata workaround to continue
> > working after suspend, or do we have to save/restore diagnostic register
> > to keep workaround effective?
> 
> I'm not sure that saving/restoring the diagnostic register on A9 actually
> works at all (I seem to remember some bits always read as zero?).

If that's true, then we have a problem.  We always read-modify-write
this register when enabling work-arounds.  If it always reads as
zero, then enabling a subsequent work-around will disable the
previous work-around.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

ARM diagnostic register across suspend/resume

[Will Deacon]

On Tue, Jun 17, 2014 at 11:16:06AM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 17, 2014 at 10:57:29AM +0100, Will Deacon wrote:
> > On Tue, Jun 17, 2014 at 09:31:18AM +0100, Shawn Guo wrote:
> > > Hi Will, Catalin,
> > > 
> > > The CP15 diagnostic register holds some bits for ARM errata workaround.
> > > Since core gets power gated across suspend/resume cycle, these bits will
> > > get lost along the way.  Is it okay for errata workaround to continue
> > > working after suspend, or do we have to save/restore diagnostic register
> > > to keep workaround effective?
> > 
> > I'm not sure that saving/restoring the diagnostic register on A9 actually
> > works at all (I seem to remember some bits always read as zero?).
> 
> If that's true, then we have a problem.  We always read-modify-write
> this register when enabling work-arounds.  If it always reads as
> zero, then enabling a subsequent work-around will disable the
> previous work-around.

I think that actually works ok, because writing zeroes doesn't actually
do anything as far as I understand. The problem with suspend/resume is
that the suspend/resume cycle could well clear the internal state and
writing zeroes won't re-enable the workaround bits.

I'll double-check this with the hardware guys, since this register really
is undocumented.

Will

ARM diagnostic register across suspend/resume

[Russell King - ARM Linux]

On Tue, Jun 17, 2014 at 11:21:23AM +0100, Will Deacon wrote:
> I think that actually works ok, because writing zeroes doesn't actually
> do anything as far as I understand. The problem with suspend/resume is
> that the suspend/resume cycle could well clear the internal state and
> writing zeroes won't re-enable the workaround bits.
> 
> I'll double-check this with the hardware guys, since this register really
> is undocumented.

Are you saying that it is write one to set, and writing zero is ignored?
If that's true, we should simplify the work-around code to get rid of the
read-modify-write.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

ARM diagnostic register across suspend/resume

[Will Deacon]

On Tue, Jun 17, 2014 at 11:23:44AM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 17, 2014 at 11:21:23AM +0100, Will Deacon wrote:
> > I think that actually works ok, because writing zeroes doesn't actually
> > do anything as far as I understand. The problem with suspend/resume is
> > that the suspend/resume cycle could well clear the internal state and
> > writing zeroes won't re-enable the workaround bits.
> > 
> > I'll double-check this with the hardware guys, since this register really
> > is undocumented.
> 
> Are you saying that it is write one to set, and writing zero is ignored?
> If that's true, we should simplify the work-around code to get rid of the
> read-modify-write.

That's my understanding for the diagnostic register, but I've asked for
clarification internally.

Will

ARM diagnostic register across suspend/resume

[Shawn Guo]

On Tue, Jun 17, 2014 at 11:25:20AM +0100, Will Deacon wrote:
> On Tue, Jun 17, 2014 at 11:23:44AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Jun 17, 2014 at 11:21:23AM +0100, Will Deacon wrote:
> > > I think that actually works ok, because writing zeroes doesn't actually
> > > do anything as far as I understand. The problem with suspend/resume is
> > > that the suspend/resume cycle could well clear the internal state and
> > > writing zeroes won't re-enable the workaround bits.
> > > 
> > > I'll double-check this with the hardware guys, since this register really
> > > is undocumented.
> > 
> > Are you saying that it is write one to set, and writing zero is ignored?
> > If that's true, we should simplify the work-around code to get rid of the
> > read-modify-write.
> 
> That's my understanding for the diagnostic register, but I've asked for
> clarification internally.

Hmm, I'm not sure that's the case.  On imx6q, I can read out diagnostic
register as 0x00200850 which matches the errata we enable.

Shawn

ARM diagnostic register across suspend/resume

[Shawn Guo]

On Tue, Jun 17, 2014 at 10:57:29AM +0100, Will Deacon wrote:
> On Tue, Jun 17, 2014 at 09:31:18AM +0100, Shawn Guo wrote:
> > Hi Will, Catalin,
> > 
> > The CP15 diagnostic register holds some bits for ARM errata workaround.
> > Since core gets power gated across suspend/resume cycle, these bits will
> > get lost along the way.  Is it okay for errata workaround to continue
> > working after suspend, or do we have to save/restore diagnostic register
> > to keep workaround effective?
> 
> I'm not sure that saving/restoring the diagnostic register on A9 actually
> works at all (I seem to remember some bits always read as zero?).
> 
> Anyway, I'd expect the state could be lost, so you'd need to reprogram
> the diagnostic register rather than save/restore its value.

Yes, we observed that diagnostic state gets lost after suspend.  But
per my testing I can see the effectiveness of errata workaround
preserves.  Is this expected somehow or something wrong in my testing?

Shawn

ARM diagnostic register across suspend/resume

[Shawn Guo]

On Tue, Jun 17, 2014 at 09:08:28PM +0800, Shawn Guo wrote:
> On Tue, Jun 17, 2014 at 10:57:29AM +0100, Will Deacon wrote:
> > On Tue, Jun 17, 2014 at 09:31:18AM +0100, Shawn Guo wrote:
> > > Hi Will, Catalin,
> > > 
> > > The CP15 diagnostic register holds some bits for ARM errata workaround.
> > > Since core gets power gated across suspend/resume cycle, these bits will
> > > get lost along the way.  Is it okay for errata workaround to continue
> > > working after suspend, or do we have to save/restore diagnostic register
> > > to keep workaround effective?
> > 
> > I'm not sure that saving/restoring the diagnostic register on A9 actually
> > works at all (I seem to remember some bits always read as zero?).
> > 
> > Anyway, I'd expect the state could be lost, so you'd need to reprogram
> > the diagnostic register rather than save/restore its value.
> 
> Yes, we observed that diagnostic state gets lost after suspend.  But
> per my testing I can see the effectiveness of errata workaround
> preserves.  Is this expected somehow or something wrong in my testing?

It is something wrong in my testing.  The effectiveness of errata
workaround goes off together with diagnostic register state.

Shawn

ARM diagnostic register across suspend/resume

[Will Deacon]

On Tue, Jun 17, 2014 at 02:03:35PM +0100, Shawn Guo wrote:
> On Tue, Jun 17, 2014 at 11:25:20AM +0100, Will Deacon wrote:
> > On Tue, Jun 17, 2014 at 11:23:44AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Jun 17, 2014 at 11:21:23AM +0100, Will Deacon wrote:
> > > > I think that actually works ok, because writing zeroes doesn't actually
> > > > do anything as far as I understand. The problem with suspend/resume is
> > > > that the suspend/resume cycle could well clear the internal state and
> > > > writing zeroes won't re-enable the workaround bits.
> > > > 
> > > > I'll double-check this with the hardware guys, since this register really
> > > > is undocumented.
> > > 
> > > Are you saying that it is write one to set, and writing zero is ignored?
> > > If that's true, we should simplify the work-around code to get rid of the
> > > read-modify-write.
> > 
> > That's my understanding for the diagnostic register, but I've asked for
> > clarification internally.
> 
> Hmm, I'm not sure that's the case.  On imx6q, I can read out diagnostic
> register as 0x00200850 which matches the errata we enable.

The hardware guys got back to me, and I was mistaken (in fact, confused by
another register). So the diagnostic register on A9 *does* read back with
the value written to it. You still need to save/restore it across suspend,
but read-modify-write is the right thing to do everywhere else.

Will

ARM diagnostic register across suspend/resume

[Shawn Guo]

On Tue, Jun 17, 2014 at 03:34:15PM +0100, Will Deacon wrote:
> The hardware guys got back to me, and I was mistaken (in fact, confused by
> another register). So the diagnostic register on A9 *does* read back with
> the value written to it. You still need to save/restore it across suspend,
> but read-modify-write is the right thing to do everywhere else.

Thanks for the confirmation, Will.

Shawn