[PATCH] MIPS: DECstation HRT calibration bug fixes

[PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

This change corrects DECstation HRT calibration, by removing the following 
bugs:

1. Calibration period selection -- HZ / 10 has been chosen, however on 
   DECstation computers, HZ never divides by 10, as the choice for HZ is 
   among 128, 256 and 1024.  The choice therefore results in a systematic
   calibration error, e.g. 6.25% for the usual choice of 128 for HZ:

   128 / 10 * 10 = 120

   (128 - 120) / 128 -> 6.25%

   The change therefore makes calibration use HZ / 8 that is always 
   accurate for the HZ values available, getting rid of the systematic
   error.

2. Calibration starting point synchronisation -- the duration of a number 
   of intervals between DS1287A periodic interrupt assertions is measured,
   however code does not ensure at the beginning that the interrupt has 
   not been previously asserted.  This results in a variable error of e.g. 
   up to another 6.25% for the period of HZ / 8 (8.(3)% with the original 
   HZ / 10 period) and the usual choice of 128 for HZ:

   1 / 16 -> 6.25%

   1 / 12 -> 8.(3)%

   The change therefore adds an initial call to ds1287_timer_state that 
   clears any previous periodic interrupt pending.

The same issue applies to both I/O ASIC counter and R4k CP0 timer 
calibration on DECstation systems as similar code is used in both cases 
and both pieces of code are covered by this fix.

On an R3400 test system used this fix results in a change of the I/O ASIC 
clock frequency reported from values like:

I/O ASIC clock frequency 23185830Hz

to:

I/O ASIC clock frequency 24999288Hz

removing the miscalculation by 6.25% from the systematic error and (for 
the individual sample provided) a further 1.00% from the variable error, 
accordingly.  The nominal I/O ASIC clock frequency is 25MHz on this 
system.

Here's another result, with the fix applied, from a system that has both 
HRTs available (using an R4400 at 60MHz nominal):

MIPS counter frequency 59999328Hz
I/O ASIC clock frequency 24999432Hz

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
Ralf,

 Please apply.

  Maciej

linux-csrc-dec-fix.patch
Index: linux/arch/mips/dec/time.c
===================================================================
--- linux.orig/arch/mips/dec/time.c
+++ linux/arch/mips/dec/time.c
@@ -126,12 +126,13 @@ int rtc_mips_set_mmss(unsigned long nowt
 void __init plat_time_init(void)
 {
 	u32 start, end;
-	int i = HZ / 10;
+	int i = HZ / 8;
 
 	/* Set up the rate of periodic DS1287 interrupts. */
 	ds1287_set_base_clock(HZ);
 
 	if (cpu_has_counter) {
+		ds1287_timer_state();
 		while (!ds1287_timer_state())
 			;
 
@@ -143,7 +144,7 @@ void __init plat_time_init(void)
 
 		end = read_c0_count();
 
-		mips_hpt_frequency = (end - start) * 10;
+		mips_hpt_frequency = (end - start) * 8;
 		printk(KERN_INFO "MIPS counter frequency %dHz\n",
 			mips_hpt_frequency);
 	} else if (IOASIC)
Index: linux/arch/mips/kernel/csrc-ioasic.c
===================================================================
--- linux.orig/arch/mips/kernel/csrc-ioasic.c
+++ linux/arch/mips/kernel/csrc-ioasic.c
@@ -41,9 +41,9 @@ void __init dec_ioasic_clocksource_init(
 {
 	unsigned int freq;
 	u32 start, end;
-	int i = HZ / 10;
-
+	int i = HZ / 8;
 
+	ds1287_timer_state();
 	while (!ds1287_timer_state())
 		;
 
@@ -55,7 +55,7 @@ void __init dec_ioasic_clocksource_init(
 
 	end = dec_ioasic_hpt_read(&clocksource_dec);
 
-	freq = (end - start) * 10;
+	freq = (end - start) * 8;
 	printk(KERN_INFO "I/O ASIC clock frequency %dHz\n", freq);
 
 	clocksource_dec.rating = 200 + freq / 10000000;

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Ralf Baechle]

On Wed, Sep 04, 2013 at 11:47:45PM +0100, Maciej W. Rozycki wrote:

> This change corrects DECstation HRT calibration, by removing the following 
> bugs:

Hmm...  Seems this needs to be applied to virtually every older kernel
as well almost back to the dawn?

Great to see you backon the DECstations!

  Ralf

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

On Thu, 5 Sep 2013, Ralf Baechle wrote:

> > This change corrects DECstation HRT calibration, by removing the following 
> > bugs:
> 
> Hmm...  Seems this needs to be applied to virtually every older kernel
> as well almost back to the dawn?

 Yeah, any version that's still considered maintained; regrettably I've 
lost track of what the current practices are.  What about the R4k timer 
erratum workaround fix I sent the other day?  I'll have another small fix 
in this area yet, for a hopefully better decision on what to select from 
the timers available on various DECstation systems that have small 
differences among them.

> Great to see you backon the DECstations!

 I'm glad to have some time to have a look.

 The platform seems to suffer from surprisingly little bitrot, there are 
some issues with the system clock as handled with these patches and with 
the RTC -- regrettably drivers/char/rtc.c isn't selectable anymore (any 
particular reason for that?) and the potential rtclib drivers 
(drivers/rtc/rtc-cmos.c or drivers/rtc/rtc-m48t86.c) refuse to cooperate 
with hardware.  Also the IRQ handling latency has become high enough now 
for the serial driver to lose characters received at 115200bps on the 
R3400 system (the R4400 one is fast enough to cope).  Other than these I 
haven't noticed any issues with the 32-bit kernel -- that in a proper 
classic style netboots smoothly albeit a bit slowly via MOP over FDDI and 
fully cooperates with the userland.

 Unfortunately that can't be said of the 64-bit kernel that hangs solidly 
(reset does not help, need to power-cycle) early on, after:

Linux version 3.11.0-rc2 (macro@tp) (gcc version 4.1.2) #1 Sun Sep 1 18:06:20 BST 2013
bootconsole [prom0] enabled

has been printed.  The next line should be:

This is a DECstation 5000/2x0

So the hang happens somewhere between register_prom_console and 
prom_identify_arch, both called from prom_init, very early indeed.  I'll 
have a look into it next; hopefully it's something silly rather than my 
old trusty compiler getting confused with something added somewhere 
meanwhile.  This is an R4400SC BTW, initial revision (PRId: 00000440).

 I'm not sure what to do about the RTC yet, and the Zilog SCC@115200bps 
case is probably lost even though the chip has been wired for DMA in the 
DECstation.

  Maciej

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Ralf Baechle]

On Fri, Sep 06, 2013 at 10:47:39PM +0100, Maciej W. Rozycki wrote:

> > > bugs:
> > 
> > Hmm...  Seems this needs to be applied to virtually every older kernel
> > as well almost back to the dawn?
> 
>  Yeah, any version that's still considered maintained; regrettably I've 
> lost track of what the current practices are.  What about the R4k timer 
> erratum workaround fix I sent the other day?  I'll have another small fix 
> in this area yet, for a hopefully better decision on what to select from 
> the timers available on various DECstation systems that have small 
> differences among them.

It's queued up to go to Linus after the weekend.

> > Great to see you backon the DECstations!
> 
>  I'm glad to have some time to have a look.
> 
>  The platform seems to suffer from surprisingly little bitrot, there are 
> some issues with the system clock as handled with these patches and with 
> the RTC -- regrettably drivers/char/rtc.c isn't selectable anymore (any 
> particular reason for that?) and the potential rtclib drivers 

The old RTC driver is bitrotting, not well integraed into modern kernels'
time and driver infrastructure.  So I eventually had to switch MIPS to
RTC_LIB.

However I actually was assuming DECstation to be just fine with RTC_LIB.

> (drivers/rtc/rtc-cmos.c or drivers/rtc/rtc-m48t86.c) refuse to cooperate 
> with hardware.  Also the IRQ handling latency has become high enough now 
> for the serial driver to lose characters received at 115200bps on the 
> R3400 system (the R4400 one is fast enough to cope).  Other than these I 
> haven't noticed any issues with the 32-bit kernel -- that in a proper 
> classic style netboots smoothly albeit a bit slowly via MOP over FDDI and 
> fully cooperates with the userland.

I'm not sure how many people still get to feel the pains of high
per interrupt overhead on their hardware.  You maybe want to talk to
Thomas Gleixner about this?

A *kludgy* solution would be "fast interrupt layer" to handle RX/TX
interrupts, and maintaining a software FIFO and only infrequently forward
interrupts through the normal Linux interrupt code.

>  Unfortunately that can't be said of the 64-bit kernel that hangs solidly 
> (reset does not help, need to power-cycle) early on, after:
> 
> Linux version 3.11.0-rc2 (macro@tp) (gcc version 4.1.2) #1 Sun Sep 1 18:06:20 BST 2013
> bootconsole [prom0] enabled
> 
> has been printed.  The next line should be:
> 
> This is a DECstation 5000/2x0

No idea why this might be hanging.  You might try git-bisect, if that's
not too painful?

> So the hang happens somewhere between register_prom_console and 
> prom_identify_arch, both called from prom_init, very early indeed.  I'll 
> have a look into it next; hopefully it's something silly rather than my 
> old trusty compiler getting confused with something added somewhere 
> meanwhile.  This is an R4400SC BTW, initial revision (PRId: 00000440).
> 
>  I'm not sure what to do about the RTC yet, and the Zilog SCC@115200bps 
> case is probably lost even though the chip has been wired for DMA in the 
> DECstation.

We could switch put a "select RTC_LIB if !MACH_DECSTATION" to buy you some
time.  But longer term RTC_LIB is really the issue.

  Ralf

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

On Sat, 7 Sep 2013, Ralf Baechle wrote:

> >  Yeah, any version that's still considered maintained; regrettably I've 
> > lost track of what the current practices are.  What about the R4k timer 
> > erratum workaround fix I sent the other day?  I'll have another small fix 
> > in this area yet, for a hopefully better decision on what to select from 
> > the timers available on various DECstation systems that have small 
> > differences among them.
> 
> It's queued up to go to Linus after the weekend.

 Excellent, thanks!  I should have the third patch mentioned ready on 
Monday too -- I want to make yet another check to make sure it does the 
right thing as I tweaked it a little bit from its original version.

> >  The platform seems to suffer from surprisingly little bitrot, there are 
> > some issues with the system clock as handled with these patches and with 
> > the RTC -- regrettably drivers/char/rtc.c isn't selectable anymore (any 
> > particular reason for that?) and the potential rtclib drivers 
> 
> The old RTC driver is bitrotting, not well integraed into modern kernels'
> time and driver infrastructure.  So I eventually had to switch MIPS to
> RTC_LIB.
> 
> However I actually was assuming DECstation to be just fine with RTC_LIB.

 I'm not sure what's going on, I haven't looked into it in detail.  The 
wiring in the DECstation is a little bit different, there's no separate 
address and data port and the chip is mapped linearly in the memory 
address space -- the chipset does all the magic to split accesses into 
separate cycles on the DS1287A bus.  Also the interrupt is taken for the 
system timer and the handling of the century byte needs special-casing.  
Any of that might be the cause -- except from the latter, that needs to be 
implemented, but does not explain a total failure.  Or maybe something 
else yet.  There isn't much in the kernel log, only:

drivers/rtc/hctosys.c: unable to open rtc device (rtc0)

A major rtc device is installed at 254 according to /proc/devices, but any 
accesses to (254, 0) fail (ENXIO, IIRC).

 BTW, is it normal these days that /proc entries like /proc/iomem, 
/proc/net or /proc/sys are omitted from the /proc directory listing?  They
can still be opened if you know the name.  I've been wondering if it's 
been a change made sometime or an odd effect of a possible compiler bug.

> > (drivers/rtc/rtc-cmos.c or drivers/rtc/rtc-m48t86.c) refuse to cooperate 
> > with hardware.  Also the IRQ handling latency has become high enough now 
> > for the serial driver to lose characters received at 115200bps on the 
> > R3400 system (the R4400 one is fast enough to cope).  Other than these I 
> > haven't noticed any issues with the 32-bit kernel -- that in a proper 
> > classic style netboots smoothly albeit a bit slowly via MOP over FDDI and 
> > fully cooperates with the userland.
> 
> I'm not sure how many people still get to feel the pains of high
> per interrupt overhead on their hardware.  You maybe want to talk to
> Thomas Gleixner about this?
> 
> A *kludgy* solution would be "fast interrupt layer" to handle RX/TX
> interrupts, and maintaining a software FIFO and only infrequently forward
> interrupts through the normal Linux interrupt code.

 Thanks for the hint, but I think there will be no need to resort to 
kludges after all, even though the SCC only has a tiny 3-stage FIFO (the 
first DECstation models had a different, genuine DEC-brand UART coming 
from the VAXen, with a comparatively impressive 64-stage FIFO).  I have 
read through I/O ASIC documentation and IIUC there is a way to utilise its 
DMA engine even for asynchronous reception (DMA would normally be used in 
synchronous modes such as SDLC; the SCC does support them as does the 
wiring in the DECstations).  Also transmits can be DMA'd in a block manner 
if there's more than one character to send available, offloading the CPU 
(there's little sense in arranging DMA for a single character only even 
though, indeed, it appears possible ;) ).

 Sounds like a plan to me and some fun stuff to do -- let's make the old 
gears spin. :)

 Speaking of interrupts -- I've noticed that all handlers are now started 
with interrupts locally disabled (IRQF_DISABLED semantics is in force for 
all handlers now and the flag itself has been scheduled for removal).  So 
what's the practice been these days for the lower priority services that 
can run with interrupts enabled just fine and take a little bit more than 
a poke at MMIO here or there?  Can they simply enable interrupts 
temporarily or is splitting the handler into a bottom half and a top half 
like in the old days, and handling the stuff beyond poking at MMIO in a 
softirq the only way?

> >  Unfortunately that can't be said of the 64-bit kernel that hangs solidly 
> > (reset does not help, need to power-cycle) early on, after:
> > 
> > Linux version 3.11.0-rc2 (macro@tp) (gcc version 4.1.2) #1 Sun Sep 1 18:06:20 BST 2013
> > bootconsole [prom0] enabled
> > 
> > has been printed.  The next line should be:
> > 
> > This is a DECstation 5000/2x0
> 
> No idea why this might be hanging.  You might try git-bisect, if that's
> not too painful?

 Given that printk works and it's just a couple of lines to examine I 
think I'll do it in the old fashion. ;)

> >  I'm not sure what to do about the RTC yet, and the Zilog SCC@115200bps 
> > case is probably lost even though the chip has been wired for DMA in the 
> > DECstation.
> 
> We could switch put a "select RTC_LIB if !MACH_DECSTATION" to buy you some
> time.  But longer term RTC_LIB is really the issue.

 Yeah, although I think it's a bit of a shame a then perfectly working and 
as of now still included driver has been disabled while the alternative 
does not work.  But please don't haste reenabling it as we don't know if 
it still works as it is, and meanwhile I'll try to poke at the rtclib 
drivers instead.

  Maciej

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Geert Uytterhoeven]

On Sat, Sep 7, 2013 at 2:48 PM, Maciej W. Rozycki <macro@linux-mips.org> wrote:
>  BTW, is it normal these days that /proc entries like /proc/iomem,
> /proc/net or /proc/sys are omitted from the /proc directory listing?  They
> can still be opened if you know the name.  I've been wondering if it's
> been a change made sometime or an odd effect of a possible compiler bug.

That's a regression introduced in early v3.11-rcX, and fixed in v3.11-rc7.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

On Sat, 7 Sep 2013, Geert Uytterhoeven wrote:

> >  BTW, is it normal these days that /proc entries like /proc/iomem,
> > /proc/net or /proc/sys are omitted from the /proc directory listing?  They
> > can still be opened if you know the name.  I've been wondering if it's
> > been a change made sometime or an odd effect of a possible compiler bug.
> 
> That's a regression introduced in early v3.11-rcX, and fixed in v3.11-rc7.

 Good to know (-rc2 used here, will update sometime), thanks!

  Maciej

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

On Sat, 7 Sep 2013, Maciej W. Rozycki wrote:

> > >  Unfortunately that can't be said of the 64-bit kernel that hangs solidly 
> > > (reset does not help, need to power-cycle) early on, after:
> > > 
> > > Linux version 3.11.0-rc2 (macro@tp) (gcc version 4.1.2) #1 Sun Sep 1 18:06:20 BST 2013
> > > bootconsole [prom0] enabled
> > > 
> > > has been printed.  The next line should be:
> > > 
> > > This is a DECstation 5000/2x0
> > 
> > No idea why this might be hanging.  You might try git-bisect, if that's
> > not too painful?
> 
>  Given that printk works and it's just a couple of lines to examine I 
> think I'll do it in the old fashion. ;)

 So it turned out harder to debug than I anticipated and I resorted to 
`git-bisect' after all.  That took several hours, including the fun of 
figuring out how to actually make the states of the tree chosen by `git' 
(or any nearby candidates) build at all -- the period covered was clearly 
the dark age of DECstation support.

 Eventually I tracked it down to 231a35d37293ab88d325a9cb94e5474c156282c0 
that introduced an incompatible copy of arch/mips/dec/prom/call_o32.S in 
arch/mips/fw/lib/, built unconditionally.  The copy happens to land 
earlier of the two among the modules used in the link and is therefore 
chosen for the DECstation rather than the intended original.  As a result 
random kernel data is corrupted because a pointer to the "%s" formatted 
output template is used as a temporary stack pointer rather than being 
passed down to prom_printf.  This also explains why prom_printf still 
works, up to a point -- the next argument is the actual string to output 
so it works just fine as the output template until enough kernel data has 
been corrupted to cause a crash.

 Now that the cause is known it is straightforward to correct one way or 
another -- the only question remaining is which one to choose.

 Thomas, what was the rationale behind arranging things in this way?  Did 
you mean to make this code shared among platforms needing it?  I would 
guess so, but then the copy in arch/mips/dec/prom/ would have to be 
removed and macros in <asm/dec/prom.h> adjusted according to the new API 
which you didn't do in your change.  Also why the need for stack 
switching?  It looks like an unnecessary complication to me, any firmware 
callbacks exported have to maintain stack integrity or they would be 
unusable.  Is that to work around some SNI firmware quirk?

 [And does it work for the SNI in the first place? -- it looks to me like 
`o32_stk' has an alignment problem (8 is required for the stack pointer in 
the o32 ABI though 4 will often be enough to satisfy hardware); but 
perhaps it just happens to get correct alignment by virtue of merely 
always following a data object that enforces it, hmm...]

 No wonder it was so weird to debug, and I guess I need to build the 
64-bit config a bit more often...

  Maciej

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Thomas Bogendoerfer]

On Mon, Sep 16, 2013 at 06:54:55AM +0100, Maciej W. Rozycki wrote:
>  Thomas, what was the rationale behind arranging things in this way?  Did 
> you mean to make this code shared among platforms needing it?  I would 

I really can't remember the reasoning any more, but I guess code sharing
was a reason.

> guess so, but then the copy in arch/mips/dec/prom/ would have to be 
> removed and macros in <asm/dec/prom.h> adjusted according to the new API 
> which you didn't do in your change.

because I didn't have a running DECstation handy.

> Also why the need for stack 
> switching?  It looks like an unnecessary complication to me, any firmware 
> callbacks exported have to maintain stack integrity or they would be 
> unusable.  Is that to work around some SNI firmware quirk?

a 64bit kernel with more than CKSEG0 addressable memory may end up having
a stack outside of CKSEG0, which is something the 32bit SNI firmware
doesn't like. I guess the same is true for DECstation, if there is a HW config
with enough memory.

>  [And does it work for the SNI in the first place? -- it looks to me like 
> `o32_stk' has an alignment problem (8 is required for the stack pointer in 
> the o32 ABI though 4 will often be enough to satisfy hardware); but 
> perhaps it just happens to get correct alignment by virtue of merely 
> always following a data object that enforces it, hmm...]

it worked, but nevertheless fixing the aligment isn't a bad thing.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Maciej W. Rozycki]

On Tue, 17 Sep 2013, Thomas Bogendoerfer wrote:

> >  Thomas, what was the rationale behind arranging things in this way?  Did 
> > you mean to make this code shared among platforms needing it?  I would 
> 
> I really can't remember the reasoning any more, but I guess code sharing
> was a reason.

 That was my guess too, so I guess two guesses are enough to be sure. ;)

> > guess so, but then the copy in arch/mips/dec/prom/ would have to be 
> > removed and macros in <asm/dec/prom.h> adjusted according to the new API 
> > which you didn't do in your change.
> 
> because I didn't have a running DECstation handy.

 Well, TBH, you could have made the same mechanical adjustments across the 
DEC files you made to your platform, or at the very least ping me (cc on 
your patch submission), especially if code sharing was your objective.

> > Also why the need for stack 
> > switching?  It looks like an unnecessary complication to me, any firmware 
> > callbacks exported have to maintain stack integrity or they would be 
> > unusable.  Is that to work around some SNI firmware quirk?
> 
> a 64bit kernel with more than CKSEG0 addressable memory may end up having
> a stack outside of CKSEG0, which is something the 32bit SNI firmware
> doesn't like. I guess the same is true for DECstation, if there is a HW config
> with enough memory.

 Nope, 480MB is the maximum the most capable in this respect DECstation 
can ever take, so it never exceeds the KSEG0 space (the remaining 32MB of 
KSEG0-mapped space is taken for MMIO; some systems also take a further 
1.5GB for aliased TURBOchannel MMIO).  Thanks for confirming.

> >  [And does it work for the SNI in the first place? -- it looks to me like 
> > `o32_stk' has an alignment problem (8 is required for the stack pointer in 
> > the o32 ABI though 4 will often be enough to satisfy hardware); but 
> > perhaps it just happens to get correct alignment by virtue of merely 
> > always following a data object that enforces it, hmm...]
> 
> it worked, but nevertheless fixing the aligment isn't a bad thing.

 OK, I think I've got a plan now.  I don't want to burden the DECstation 
with stack switching or the extra memory use it requires, so I'll reshape 
arch/mips/fw/lib/call_o32.S a bit to fit both platforms.  Will you be able 
and willing to test my code once it's ready with a SNI system?

  Maciej

Re: [PATCH] MIPS: DECstation HRT calibration bug fixes

[Thomas Bogendoerfer]

On Tue, Sep 17, 2013 at 04:10:54PM +0100, Maciej W. Rozycki wrote:
>  OK, I think I've got a plan now.  I don't want to burden the DECstation 
> with stack switching or the extra memory use it requires, so I'll reshape 
> arch/mips/fw/lib/call_o32.S a bit to fit both platforms.  Will you be able 
> and willing to test my code once it's ready with a SNI system?

sure no problem. Thanks for fixing that.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]