linuxsh-dev

[linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[Mitch D. <mj...@al...>]

Hello,

As Greg indicated in his earlier email, here are the diffs we
have been working on.  There's a patch for the kernel code, and
a patch for the stub.

Preceding each patch is a list of changes.  The patches can
also be retrieved from here:

Kernel patch: (linux-sh-2000-07-18-gnb-mjd.diff.bz2)

  http://sourceforge.net/patch/?func=detailpatch&patch_id=100920&group_id=2682
  
Stub patch: (gdb-sh-stub-2000-06-22-gnb-mjd.diff.bz2)

  http://sourceforge.net/patch/?func=detailpatch&patch_id=100921&group_id=2682

We are posting them here for public comment.  All suggestions and
ideas are most welcome.

Regards,

Mitch.
--
mailto:mj...@al... | Certified Linux Evangelist!

[linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[NIIBE Y. <gn...@ch...>]

Mitch Davis wrote:
 > As Greg indicated in his earlier email, here are the diffs we
 > have been working on.  There's a patch for the kernel code, and
 > a patch for the stub.

Thanks for your effort.  It's great contribution.

Here're some comments:

 > x  Link script sets Load Memory Addresses so that kernel image
 >    load bypasses the cache, to avoid cache problems.

No, this is backward.  If you have any problems, please send us bug
report.  The changes like this just paper the bugs, and hide them.
It's not the solution.  Major problem is it loses performance.
Besides, I don't know the problem around cache issues, which is
solved with this change.

It seems for me that what to change is GDB stub, if needed.

 > --- kernel-orig/arch/sh/kernel/sh_bios.c	Thu Jan  1 10:00:00 1970
 > +++ kernel/arch/sh/kernel/sh_bios.c	Tue Jul 18 03:57:22 2000
 > @@ -0,0 +1,70 @@
 > +/* $Id$
 > + *
 > + *  linux/arch/sh/kernel/sh-bios.c
 > + *  C interface for trapping into the standard LinuxSH BIOS.
 > + *
 > + *  Copyright (C) 2000 Greg Banks, Mitch Davis
 > + *
 > + */

It would be good if we have license notice here (which I sometimes
forgot).  Please look arch/sh/kernel/entry.S for example.

Well done.  Please check them (except the Load Memory Addresses thing)
in.  May I ask you to incorporate changes of sh-sci.c,h done by
Stuart?  It seems that he's off-line nowadays.  I'm thinking send our
update to Linus.  I'll incorporate his other changes, if he won't.

 > This is a patch against the gdb stub version 2000-06-22.  The
 > following is a description of changes.

I'll incorporate this changes.  Thanks.
-- 

[linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[NIIBE Y. <gn...@ch...>]

NIIBE Yutaka wrote:
 > Well done.  Please check them (except the Load Memory Addresses thing)
 > in.  May I ask you to incorporate changes of sh-sci.c,h done by
 > Stuart?

I've misunderstood the situation.  The patch is by Takeshi.  I've just
checked in following patch.

2000-07-20  YAEGASHI Takeshi  <yae...@ma...>

	* sh-sci.h (PORT_IRDA, SH3_IRDA_IRQS): New definition.
	(SCI_INIT, SCI_NPORTS): Fixed for CONFIG_CPU_SUBTYPE_SH7708.

	* sh-sci.c (sci_init_pins_irda): New Function.

Index: drivers/char/sh-sci.c
===================================================================
RCS file: /cvsroot/linuxsh/kernel/drivers/char/sh-sci.c,v
retrieving revision 1.11
diff -u -r1.11 sh-sci.c
--- drivers/char/sh-sci.c	2000/07/04 09:45:19	1.11
+++ drivers/char/sh-sci.c	2000/07/20 05:30:57
@@ -62,6 +62,7 @@
 #ifndef SCI_ONLY
 static void sci_init_pins_scif(struct sci_port* port, unsigned int cflag);
 #endif
+static void sci_init_pins_irda(struct sci_port* port, unsigned int cflag);
 static void sci_disable_tx_interrupts(void *ptr);
 static void sci_enable_tx_interrupts(void *ptr);
 static void sci_disable_rx_interrupts(void *ptr);
@@ -138,6 +139,16 @@
 		/* Set /RTS2 (bit6) = 0 */
 		ctrl_outb(data&0xbf, SCPDR);
 	}
+	sci_out(port, SCFCR, fcr_val);
+}
+
+static void sci_init_pins_irda(struct sci_port* port, unsigned int cflag)
+{
+	unsigned int fcr_val = 0;
+
+	if (cflag & CRTSCTS)
+		fcr_val |= SCFCR_MCE;
+
 	sci_out(port, SCFCR, fcr_val);
 }
 
Index: drivers/char/sh-sci.h
===================================================================
RCS file: /cvsroot/linuxsh/kernel/drivers/char/sh-sci.h,v
retrieving revision 1.9
diff -u -r1.9 sh-sci.h
--- drivers/char/sh-sci.h	2000/06/22 17:52:03	1.9
+++ drivers/char/sh-sci.h	2000/07/20 05:30:57
@@ -13,6 +13,7 @@
 /* Values for sci_port->type */
 #define PORT_SCI  0
 #define PORT_SCIF 1
+#define PORT_IRDA 1		/* XXX: temporary assignment */
 
 /* Offsets into the sci_port->irqs array */
 #define SCIx_ERI_IRQ 0
@@ -22,6 +23,7 @@
 /*                     ERI, RXI, TXI,   INTC reg, INTC pos */
 #define SCI_IRQS      { 23,  24,  25 }, INTC_IPRB, 1
 #define SH3_SCIF_IRQS { 56,  57,  59 }, INTC_IPRE, 1
+#define SH3_IRDA_IRQS { 52,  53,  55 }, INTC_IPRE, 2
 #define SH4_SCIF_IRQS { 40,  41,  43 }, INTC_IPRC, 1
 
 #if defined(CONFIG_CPU_SUBTYPE_SH7708)
@@ -33,21 +35,11 @@
 # define SCSCR_INIT(port)          0x30 /* TIE=0,RIE=0,TE=1,RE=1 */
 # define SCI_ONLY
 #elif defined(CONFIG_CPU_SUBTYPE_SH7709)
-# define SCI_NPORTS 2
-# define SCI_INIT { \
-  { {}, PORT_SCI,  0xfffffe80, SCI_IRQS,      sci_init_pins_sci  }, \
-  { {}, PORT_SCIF, 0xA4000150, SH3_SCIF_IRQS, sci_init_pins_scif }  \
-}
-# define SCPCR  0xA4000116 /* 16 bit SCI and SCIF */
-# define SCPDR  0xA4000136 /* 8  bit SCI and SCIF */
-# define SCSCR_INIT(port)          0x30 /* TIE=0,RIE=0,TE=1,RE=1 */
-# define SCI_AND_SCIF
-#elif defined(CONFIG_CPU_SUBTYPE_SH7709A)
 # define SCI_NPORTS 3
 # define SCI_INIT { \
   { {}, PORT_SCI,  0xfffffe80, SCI_IRQS,        sci_init_pins_sci  }, \
-  { {}, PORT_SCIF, 0xA4000150, 7709A_SCIF_IRQS, sci_init_pins_scif }  \
-  { {}, PORT_SCIF, 0xA4000140, 7709A_IRDA_IRQS, sci_init_pins_irda }  \
+  { {}, PORT_SCIF, 0xA4000150, SH3_SCIF_IRQS, sci_init_pins_scif },   \
+  { {}, PORT_SCIF, 0xA4000140, SH3_IRDA_IRQS, sci_init_pins_irda }    \
 }
 # define SCPCR  0xA4000116 /* 16 bit SCI and SCIF */
 # define SCPDR  0xA4000136 /* 8  bit SCI and SCIF */

Re: [linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[Mitch D. <mj...@al...>]

NIIBE Yutaka wrote:
> 
> Mitch Davis wrote:
>  > As Greg indicated in his earlier email, here are the diffs we
>  > have been working on.  There's a patch for the kernel code, and
>  > a patch for the stub.
>
> Well done.  Please check them (except the Load Memory Addresses thing)
> in.  

Hello everyone,

I have now checked these changes in, except for the changes in
kernel/arch/sh/kernel.  CVS has left a lock there that needs 
clearing before the checkin can proceed.  (I've asked the
SourceForge people to fix it).

So as to not leave people with an uncompilable kernel, I am
attaching the changes from kernel/arch/sh/kernel in a patch
file.  Until the lock issue is resolved, please use this patch
to get a good copy of the most recent kernel source.

(Sorry about this necessary inconvenience)

Regards,

Mitch.
--
mailto:mj...@al... | Certified Linux Evangelist!

Re: [linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[Mitch D. <mj...@al...>]

Mitch Davis wrote:
> 
> I have now checked these changes in, except for the changes in
> kernel/arch/sh/kernel.  CVS has left a lock there that needs
> clearing before the checkin can proceed.  (I've asked the
> SourceForge people to fix it).

Hello everybody,

The problem has been fixed, and the files checked in.  It is
no longer necessary to apply the patch I sent out yesterday.

Regards,

Mitch.
--
mailto:mj...@al... | Certified Linux Evangelist!

Re: [linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[Mitch D. <mj...@al...>]

NIIBE Yutaka wrote:
> 
> Mitch Davis wrote:
>  > As Greg indicated in his earlier email, here are the diffs we
>  > have been working on.  There's a patch for the kernel code, and
>  > a patch for the stub.
> 
>  > x  Link script sets Load Memory Addresses so that kernel image
>  >    load bypasses the cache, to avoid cache problems.
> 
> No, this is backward.  If you have any problems, please send us bug
> report.  The changes like this just paper the bugs, and hide them.
> It's not the solution.  Major problem is it loses performance.
> Besides, I don't know the problem around cache issues, which is
> solved with this change.

Hi Niibe-san,

Please note that it is NOT the purpose of this patch to run the
kernel in the uncached P2 area (A0000000) - with this patch, the
kernel is still linked against and will still run in the cached
P1 area (80000000).  This patch only changes where the kernel is
loaded by a stub.

Running the kernel in the P2 area would surely compromise 
performance, and would indeed be a bad fix for a cache-related
problem! :-)

The problem we have faced a number of times was that kernel
downloading was not successful - even small programs would sometimes
not load properly.  (I had this problem at HP too, but not
involving Linux or our toolchain).  We traced this to the bootloader
not flushing the cache after the load.

As you know, using the "AT" linker directive makes it possible
link a program to run at a certain address (in this case, 80000000)
but be loaded at another address.  (In this case, A0000000).
Doing this ALWAYS fixed our problems.

> It seems for me that what to change is GDB stub, if needed.

It would be possible to change the stub so it has explicit code
to flush the cache after a transfer.  But the stub does not
know whether gdb on the host is sending a program, or setting
a variable.  Should it flush the whole cache every time
a variable is set?  Is it safe to do so without the kernel
knowing?  And also, the details of flushing the cache
are rather processor-specific, something it would be best to
keep out of the stub.

The other solution would be for the stub to mask incoming data so
they fall in the P2 area.  But I think this will not only 
mislead people, but may cause cache consistency problems
half-way through the run, because the variable is set past the
cached copy.

IMO, changing the Linux link file is a better way of determining
how the stub puts a loaded program into memory, and it avoids
these other problems.

Yes, storing into P2 will be a little slower than storing into
P1, but I believe the delay in waiting for data from a serial port
will be several orders of magnitude slower, so there won't be
any noticeable performance hit when loading the kernel.  (And
as I mentioned, none when running it).

Well, that's how we've been looking at it, but I'm only a beginner
with these things.  What do you think?  Would you prefer to see the
problem solved in another way?  Is there some issue which will
bite us with this approach, which we are not seeing?

I look forward to hearing your opinion.

Regards,

Mitch.
--
mailto:mj...@al... | Certified Linux Evangelist!

Re: [linuxsh-dev] Assorted patches for Linux kernel, and the stub.

[NIIBE Y. <gn...@ch...>]

Mitch Davis wrote:
 > Please note that it is NOT the purpose of this patch to run the
 > kernel in the uncached P2 area (A0000000) - with this patch, the
 > kernel is still linked against and will still run in the cached
 > P1 area (80000000).  This patch only changes where the kernel is
 > loaded by a stub.
[...]
 > The problem we have faced a number of times was that kernel
 > downloading was not successful - even small programs would sometimes
 > not load properly.  (I had this problem at HP too, but not
 > involving Linux or our toolchain).  We traced this to the bootloader
 > not flushing the cache after the load.
 > 
 > As you know, using the "AT" linker directive makes it possible
 > link a program to run at a certain address (in this case, 80000000)
 > but be loaded at another address.  (In this case, A0000000).
 > Doing this ALWAYS fixed our problems.

OK, I didn't understand the issues, now have understood.  I don't
understand stub hits this problem, as it disables caches.

If there's problem, it's the bug of stub, I think.

I think that what you should change is the loader (stub or other).
Yes, you can work around the problem with this patch, but I don't
think this is solution.  If I were at the problem, I fix the loader so
that it flush the cache at the point needed.  For GDB stub (with cache
enabled), the point would be continue command handling where we need
to flush data cache so that instruction loaded from memory works fine.

 > And also, the details of flushing the cache are rather
 > processor-specific, something it would be best to keep out of the
 > stub.

Please look at the code, it already has the cache flushing code, as
breakpoints need this.

My point is: kernel image (vmlinux) is not just for the environment of
GDB stub (with cache enabled).  It could be used as other loader, used
by building zImage, or used when we wrote it into Flash ROM.  I think
that it's better not introducing other artifact here.
--