[PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[David Howells]

These patches add the MEI/Panasonic MN10300/AM33 architecture to the Linux
kernel.

The first patch makes it possible to suppress AOUT support in the ELF binfmt.
MN10300 does not support the AOUT binfmt, so the ELF binfmt should not be
permitted to go looking for AOUT libraries to load.

The second patch adds the architecture itself, to be selected by ARCH=mn10300
on the make command line.

The patches can also be downloaded from:

	http://people.redhat.com/~dhowells/mn10300/mn10300-arch.tar.bz2


A suitable toolchain can be downloaded from:

	ftp://ftp.redhat.com/pub/redhat/gnupro/AM33/

The latest is currently:

	am33-04r2-5/tools/i686-pc-linux-gnulibc2.3-x-am33_2.0-linux-gnu.tar.bz2

David

[PATCH 1/2] MN10300: Suppress AOUT library support in ELF binfmt if !CONFIG_BINFMT_AOUT [try #2]

[David Howells]

Suppress AOUT library support in ELF binfmt if CONFIG_BINFMT_AOUT is not set.

The MN10300 architecture does not support the AOUT binfmt, so the ELF binfmt
should not be permitted to go looking for AOUT libraries to load.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/binfmt_elf.c       |   30 ++++++++++++++++++++++--------
 include/linux/a.out.h |    6 ++++++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index ba8de7c..b734848 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -464,6 +464,7 @@ out:
 	return error;
 }
 
+#ifdef CONFIG_BINFMT_AOUT
 static unsigned long load_aout_interp(struct exec *interp_ex,
 		struct file *interpreter)
 {
@@ -509,6 +510,10 @@ static unsigned long load_aout_interp(struct exec *interp_ex,
 out:
 	return elf_entry;
 }
+#else
+extern unsigned long load_aout_interp(struct exec *interp_ex,
+				      struct file *interpreter);
+#endif
 
 /*
  * These are the functions used to load ELF style executables and shared
@@ -516,9 +521,15 @@ out:
  */
 
 #define INTERPRETER_NONE 0
-#define INTERPRETER_AOUT 1
 #define INTERPRETER_ELF 2
 
+#ifdef CONFIG_BINFMT_AOUT
+#define INTERPRETER_AOUT 1
+#define IS_AOUT_INTERP(x) ((x) == INTERPRETER_AOUT)
+#else
+#define IS_AOUT_INTERP(x) (0)
+#endif
+
 #ifndef STACK_RND_MASK
 #define STACK_RND_MASK (0x7ff >> (PAGE_SHIFT - 12))	/* 8MB of VA */
 #endif
@@ -734,6 +745,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	/* Some simple consistency checks for the interpreter */
 	if (elf_interpreter) {
 		static int warn;
+#ifdef CONFIG_BINFMT_AOUT
 		interpreter_type = INTERPRETER_ELF | INTERPRETER_AOUT;
 
 		/* Now figure out which format our binary is */
@@ -741,11 +753,13 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 		    (N_MAGIC(loc->interp_ex) != ZMAGIC) &&
 		    (N_MAGIC(loc->interp_ex) != QMAGIC))
 			interpreter_type = INTERPRETER_ELF;
-
+#else
+		interpreter_type = INTERPRETER_ELF;
+#endif
 		if (memcmp(loc->interp_elf_ex.e_ident, ELFMAG, SELFMAG) != 0)
 			interpreter_type &= ~INTERPRETER_ELF;
 
-		if (interpreter_type == INTERPRETER_AOUT && warn < 10) {
+		if (IS_AOUT_INTERP(interpreter_type) && warn < 10) {
 			printk(KERN_WARNING "a.out ELF interpreter %s is "
 				"deprecated and will not be supported "
 				"after Linux 2.6.25\n", elf_interpreter);
@@ -774,7 +788,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 
 	/* OK, we are done with that, now set up the arg stuff,
 	   and then start this sucker up */
-	if ((!bprm->sh_bang) && (interpreter_type == INTERPRETER_AOUT)) {
+	if (IS_AOUT_INTERP(interpreter_type) && !bprm->sh_bang) {
 		char *passed_p = passed_fileno;
 		sprintf(passed_fileno, "%d", elf_exec_fileno);
 
@@ -961,7 +975,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	}
 
 	if (elf_interpreter) {
-		if (interpreter_type == INTERPRETER_AOUT)
+		if (IS_AOUT_INTERP(interpreter_type))
 			elf_entry = load_aout_interp(&loc->interp_ex,
 						     interpreter);
 		else
@@ -990,7 +1004,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 
 	kfree(elf_phdata);
 
-	if (interpreter_type != INTERPRETER_AOUT)
+	if (!IS_AOUT_INTERP(interpreter_type))
 		sys_close(elf_exec_fileno);
 
 	set_binfmt(&elf_format);
@@ -1006,14 +1020,14 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	compute_creds(bprm);
 	current->flags &= ~PF_FORKNOEXEC;
 	retval = create_elf_tables(bprm, &loc->elf_ex,
-			  (interpreter_type == INTERPRETER_AOUT),
+			  IS_AOUT_INTERP(interpreter_type),
 			  load_addr, interp_load_addr);
 	if (retval < 0) {
 		send_sig(SIGKILL, current, 0);
 		goto out;
 	}
 	/* N.B. passed_fileno might not be initialized? */
-	if (interpreter_type == INTERPRETER_AOUT)
+	if (IS_AOUT_INTERP(interpreter_type))
 		current->mm->arg_start += strlen(passed_fileno) + 1;
 	current->mm->end_code = end_code;
 	current->mm->start_code = start_code;
diff --git a/include/linux/a.out.h b/include/linux/a.out.h
index f913cc3..7a890e8 100644
--- a/include/linux/a.out.h
+++ b/include/linux/a.out.h
@@ -1,6 +1,8 @@
 #ifndef __A_OUT_GNU_H__
 #define __A_OUT_GNU_H__
 
+#ifdef CONFIG_BINFMT_AOUT
+
 #define __GNU_EXEC_MACROS__
 
 #ifndef __STRUCT_EXEC_OVERRIDE__
@@ -265,4 +267,8 @@ struct relocation_info
 #endif /* no N_RELOCATION_INFO_DECLARED.  */
 
 
+#else /* CONFIG_BINFMT_AOUT */
+struct exec {
+};
+#endif /* CONFIG_BINFMT_AOUT */
 #endif /* __A_OUT_GNU_H__ */

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[Suzuki Takashi]

On 10/29/07, David Howells <dhowells@redhat.com> wrote:
> The second patch adds the architecture itself, to be selected by ARCH=mn10300
> on the make command line.

Why don't you use `am33' for the arch name?
Do you have any plans to support am30 (mn10300) or any pre-am33 processor cores?

I know the mn10300-* target configurations are also available for toolchains,
but it is for historical reasons.
Even the GNUPro packages have the release name am33-04r2-5 instead of mn10300-*.

The same arch name would be better for the same architecture
among the toolchains, the kernel and the library, not to confuse contributors.
GNU libc has used am33 for the machine name since the architecture was
supported.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> Why don't you use `am33' for the arch name?

History and the fact that 'mn10300' is what MEI called their arch when they
originally gave us (RH) their kernel.  There are kernels already out there
running on consumer devices for which it is the mn10300 arch.

David

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[Suzuki Takashi]

On 10/30/07, David Howells <dhowells@redhat.com> wrote:
> > Why don't you use `am33' for the arch name?
>
> History and the fact that 'mn10300' is what MEI called their arch when they
> originally gave us (RH) their kernel.  There are kernels already out there
> running on consumer devices for which it is the mn10300 arch.

I see.
But the arch name change does not affect running kernels on consumer devices.
And it wouldn't have so much impact on developers.
Drastic changes in the directory and file structures will have
a much greater impact than that.

This submission is a good chance for the new arch name.
New contributors cannot recognize the am33 glibc is for the mn10300 Linux.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> But the arch name change does not affect running kernels on consumer devices.

uname says mn10300.

> Drastic changes in the directory and file structures will have
> a much greater impact than that.

Like renaming the arch/mn10300 and include/asm-mn10300 directories and having
to tell people already using ARCH=mn10300 that they've got to change.

> This submission is a good chance for the new arch name.

It's also a good time to leave it as is, IMO.  This isn't precisely a new
arch.  It's been around a few years.  It just hasn't been posted upstream till
now.  People are using it, however.

> New contributors cannot recognize the am33 glibc is for the mn10300 Linux.

They can if they're told so, plus if they've got the CPU docs to hand, it
should be more obvious.  The AM33 is a variant of the MN10300 arch as far as I
can tell.

David

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[DJ Delorie]

> They can if they're told so, plus if they've got the CPU docs to
> hand, it should be more obvious.  The AM33 is a variant of the
> MN10300 arch as far as I can tell.

Right, there's also the AM30, AM31, AM32, and AM34.  Panasonic calls
the whole series the "AM3" or "MN103 series" although they also have
MN103E and MN103S series.  MN103E is the AM33/AM34 microprocessors
(MMU), MN103S is the AM3[012] microcontrollers (no MMU).

http://www.semicon.panasonic.co.jp/e-micom/MicomFamily/roadmap.html

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[Suzuki Takashi]

On 10/31/07, David Howells <dhowells@redhat.com> wrote:
> > But the arch name change does not affect running kernels on consumer devices.
>
> uname says mn10300.

Oh, I forgot.
But is there any utility that depends on that uname says mn10300?

> > Drastic changes in the directory and file structures will have
> > a much greater impact than that.
>
> Like renaming the arch/mn10300 and include/asm-mn10300 directories and having
> to tell people already using ARCH=mn10300 that they've got to change.

I know existing developers should learn the arch name change.
But how many developers?

> > New contributors cannot recognize the am33 glibc is for the mn10300 Linux.
>
> They can if they're told so, plus if they've got the CPU docs to hand, it
> should be more obvious.  The AM33 is a variant of the MN10300 arch as far as I
> can tell.

That's the point. They must always be told or consult the documentations.
Don't you think an arch name should be natural?
People need not to be told on other, at least major, archs.

It's
- growing new developers will have difficulties in understanding arch
name difference,
versus
- fewer people have to learn the arch name change.
I vote the latter now.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> But is there any utility that depends on that uname says mn10300?

autoconf:-)

> I know existing developers should learn the arch name change.
> But how many developers?

I don't know.  MEI probably does.

> That's the point. They must always be told or consult the documentations.
> Don't you think an arch name should be natural?

Can you guarantee there won't be any non-AM33 variants of mn10300 that need
supporting?  An AM34, perhaps?

So basically, the toolchain name should change from am33 to mn10300 by your
argument.

> People need not to be told on other, at least major, archs.

There are plenty of other complications for other archs, such as 32-bit vs
64-bit, LE vs BE and other more subtle variants, so your argument isn't that
compelling.

> - growing new developers will have difficulties in understanding arch
> name difference,

You tell them, they then know.  It doesn't take long.  Standard practice, I
suspect, will involve giving them the toolchains and basic root filesystems.

The discrepancy has been around for years, and I suspect it's not going to
change now.  If MEI ask, I will change it.

David

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[Alan Cox]

> Can you guarantee there won't be any non-AM33 variants of mn10300 that need
> supporting?  An AM34, perhaps?
> 
> So basically, the toolchain name should change from am33 to mn10300 by your
> argument.

Given the "i686" toolchain name is wrong (its actually i686 + cmov) there
is a ton of history for the tool chain name not matching up and its even
happened on PC - and there are tools like rpm that have to know about the
mismatches and already cope.

Re: [Linux-am33-list] [PATCH 0/2] MN10300: Add the MN10300 architecture to Linux kernel [try #2]

[Suzuki Takashi]

Sorry for the late.

On 10/31/07, David Howells <dhowells@redhat.com> wrote:
> > But is there any utility that depends on that uname says mn10300?
>
> autoconf:-)

I forgot again the one as an host environment. :-)
AFAIK, mn10300 series are for embedded systems and I've never heard as
development hosts.
I had noticed the new machine name needs to be added / changed, though.

> Can you guarantee there won't be any non-AM33 variants of mn10300 that need
> supporting?  An AM34, perhaps?

I think `am33' is a reasonable name if  it is the first variant of
mn10300 series
that is supported by Linux, like `am33' is for glibc and `i386' is
(was) for Linux and others.
People don't misunderstand that an AM30, AM31 or AM32 is supported by Linux.
AM3n seems to be upper compatible with AM3(n-1), so far.
MEI (and only they) know if AM34, and any newer core in that series
are compatible with AM33.

> The discrepancy has been around for years, and I suspect it's not going to
> change now.  If MEI ask, I will change it.

I see.
I know there is no technical problem if the arch name continues to be mn10300.
It is an issue of some branding strategy, or a designing policy.
I understand you (RH) are not in a position to decide to change, nor
to refuse to change.

I just noticed and became worried about the mismatch.
If MEI ask you to change the toolchain name from am33 to mn10300, I
have no objection to that.
I'd be glad to hear any comments from MEI.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[Suzuki Takashi]

On 10/29/07, David Howells <dhowells@redhat.com> wrote:
> diff --git a/Documentation/mn10300/compartmentalisation.txt b/Documentation/mn10300/compartmentalisation.txt
(snip)
> + (1) CPU level
> +
> +     This involves support for a specific set of registers and instructions and
> +     some very low-level peripherals.
> +
> +     CPU-specific constants, definitions and sources are segregated by having
> +     the header files divided into CPU specific directories under the asm
> +     headers directory:
> +
> +       (*) include/asm-mn10300/cpu-am33v2/
> +
> +           Support for the AM33v2 CPU core.
> +
> +     The appropriate CPU is selected by a CONFIG_MN10300_CPU_XXXX option from
> +     the "Processor core support" choice menu in the arch/mn10300/Kconfig file.

Does this mean cpu-am33v3, cpu-am33v4, etc. will be created
when a new core comes up?

Isn't it possible to split codes by features, instead of target cores?

If the AM33v2 is the base architecture and a new core feature comes up
on AM33v3,
that core feature should be enabled by a new CONFIG_* or a dynamic switch.
Generally, drastic changes won't come up
as long as the new CPU core is on the same architecture.
Whole directory separation would be inappropriate for the CPU cores on
the same arch.
If similar codes were in multiple files and directories, it would be
hard to maintain them.

> + (2) Processor level
> +
> +     The "processor level" is a CPU core plus the other on-silicon
> +     peripherals.
> +
> +     Processor-specific header files are divided among directories in a similar
> +     way to the CPU level:
> +
> +       (*) include/asm-mn10300/proc-mn103e010/
> +
> +           Support for the AM33v2 CPU core.
> +
> +     The appropriate processor is selected by a CONFIG_MN10300_PROC_YYYY option
> +     from the "Processor support" choice menu in the arch/mn10300/Kconfig file.

Ditto. Splitting by features would be better.

> diff --git a/arch/mn10300/kernel/fpu-low.S b/arch/mn10300/kernel/fpu-low.S
(snip)
> +       .globl  fpu_init_state
> +       .type   fpu_init_state,@function
> +fpu_init_state:
> +       mov     epsw,d0
> +       or      EPSW_FE,epsw
> +
> +#ifdef CONFIG_MN10300_PROC_MN103E010
> +       nop
> +       nop
> +       nop
> +#endif

This should be conditioned by a feature.
Isn't it a feature or a limitation that several non-FPU instructions are
necessary just after the EPSW_FE is set?

> diff --git a/arch/mn10300/kernel/mn103e010-debug.c b/arch/mn10300/kernel/mn103e010-debug.c
(snip)
> diff --git a/arch/mn10300/kernel/mn103e010-serial-low.S b/arch/mn10300/kernel/mn103e010-serial-low.S
(snip)
> diff --git a/arch/mn10300/kernel/mn103e010-serial.c b/arch/mn10300/kernel/mn103e010-serial.c
(snip)
> diff --git a/arch/mn10300/kernel/mn103e010-serial.h b/arch/mn10300/kernel/mn103e010-serial.h
(snip)
> diff --git a/arch/mn10300/kernel/mn103e010-watchdog-low.S b/arch/mn10300/kernel/mn103e010-watchdog-low.S
(snip)
> diff --git a/arch/mn10300/kernel/mn103e010-watchdog.c b/arch/mn10300/kernel/mn103e010-watchdog.c

Are these specific to MN103E010, or applicable to some other LSIs?
If they are not specific to MN103E010, the file names aren't good.

> diff --git a/include/asm-mn10300/cpu-am33v2/dmactl-regs.h b/include/asm-mn10300/cpu-am33v2/dmactl-regs.h
(snip)
> +struct mn103e010_dmactl_regs {
> +       u32             ctr;
> +       const void      *src;
> +       void            *dst;
> +       u32             siz;
> +       u32             cyc;
> +} __attribute__((aligned(0x100)));
> +
> +extern volatile struct mn103e010_dmactl_regs mn103e010_dmactl_regs[4];

Stranger names here.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> Does this mean cpu-am33v3, cpu-am33v4, etc. will be created
> when a new core comes up?

Yes.  Note that the headers in a later one can 'inherit' from those of an
earlier one simply by #including that earlier one, much like archs do with
asm-generic headers.

> Isn't it possible to split codes by features, instead of target cores?

Perhaps, but I personally am not in a position to judge what features are
common to what CPUs.  I'll have to let someone from MEI answer that one.  I'll
discuss it with them.

> If similar codes were in multiple files and directories, it would be
> hard to maintain them.

#include is a wonderful thing.

> > +#ifdef CONFIG_MN10300_PROC_MN103E010
> > +       nop
> > +       nop
> > +       nop
> > +#endif
> 
> This should be conditioned by a feature.
> Isn't it a feature or a limitation that several non-FPU instructions are
> necessary just after the EPSW_FE is set?

Or it could just be a h/w bug in this particular processor.  I can always
patch it later to change the name.  Whatever it is, I suspect it will always
be controlled implicitly.

> Are these specific to MN103E010, or applicable to some other LSIs?

Yes.

> If they are not specific to MN103E010, the file names aren't good.

I realise that, but I haven't been able to get any better names for them.

> Stranger names here.

How so?

David

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[Suzuki Takashi]

On 10/31/07, David Howells <dhowells@redhat.com> wrote:
> > Does this mean cpu-am33v3, cpu-am33v4, etc. will be created
> > when a new core comes up?
>
> Yes.  Note that the headers in a later one can 'inherit' from those of an
> earlier one simply by #including that earlier one, much like archs do with
> asm-generic headers.

GCC is willing to include the earlier one.
But people aren't, are they?

If cpu-am33v3/cpu-regs.h #includes cpu-am33v2/cpu-regs.h,
people have to
1) open cpu-am33v3/cpu-regs.h,
saying #include <asm/cpu-am33v2/cpu-regs.h> (and sigh).
2) open cpu-am33v2/cpu-regs.h.

This decreases development efficiency.
If there is no difference and they are all #including only headers,
there is nothing but development efficiency decrease.
It is not too late to split into directories
only after some large difference to the earlier one comes on.

And at that time asm/cpu-regs.h should #include cpu/cpu-regs.h and
people would #include asm/cpu-regs.h for the general purpose so that
they aren't confused which is cpu-specific and which is proc-specific.

> > Isn't it possible to split codes by features, instead of target cores?
>
> Perhaps, but I personally am not in a position to judge what features are
> common to what CPUs.  I'll have to let someone from MEI answer that one.  I'll
> discuss it with them.

Ok. I'll wait.

> > Stranger names here.
>
> How so?

The file is under cpu/, but the names are mn103e010_* which is proc-specific.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> But people aren't, are they?

On the other hand, these particular header files are of interest to arch
maintainers.

If there are variances between CPUs that mandate different sets of registers
or different usage protocols for equivalent modules, then you will end up with
something you'll object to.

Anyway, I've discussed this with MEI, and they're willing for some flattening
to take place.  The problem is how much?  I could just move all the
cpu-am33v2/ files into the dir above, but what happens if an incompatible CPU
core is introduced?

> If cpu-am33v3/cpu-regs.h #includes cpu-am33v2/cpu-regs.h,
> people have to
> 1) open cpu-am33v3/cpu-regs.h,
> saying #include <asm/cpu-am33v2/cpu-regs.h> (and sigh).
> 2) open cpu-am33v2/cpu-regs.h.

It's an awkward situation, yes, but not one most people will be concerned
with.  There needs to be some way to multiplex alternate register sets.  The
main ways of doing that are:

 (1) #include hell

 (2) #ifdef hell

 (3) Both

Which do you prefer?

Besides, you should use emacs tags:-)

However, I'll concede that the various versions of AM33 processor should
perhaps be represented by the same CPU subdirectory (cpu-am33), using #ifdef
to add extra features.  However, should, say, an AM34 be produced that's
effectively a variant of the AM33, then that scheme falls down too.

> This decreases development efficiency.

Like I said, it'll make very little difference to most people because few
people go poking around in the arch code.

> It is not too late to split into directories

It is already split up into directories.

> And at that time asm/cpu-regs.h should #include cpu/cpu-regs.h and
> people would #include asm/cpu-regs.h for the general purpose so that
> they aren't confused which is cpu-specific and which is proc-specific.

I'm confused as to what you're talking about.  asm/cpu-regs.h would not for
general purpose at all.  It would be an arch specific header file and should
not be used by code outside of the arch.

And if all asm/cpu-regs.h does is #include asm/cpu/cpu-regs.h, then what's the
point in having it?

> > > Stranger names here.
> >
> > How so?
> 
> The file is under cpu/, but the names are mn103e010_* which is proc-specific.

Good point.  That bears moving then.

David

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[Suzuki Takashi]

Thank you for your replies.

On 10/31/07, David Howells <dhowells@redhat.com> wrote:
> Anyway, I've discussed this with MEI, and they're willing for some flattening
> to take place.

I'm very annoyed and a bit surprised to hear they admitted such a big
change so easily.

You and I know there are consumer devices already out running the am33
port of the kernel.
I guess they have some policy for the port, including the directory
structure under arch/,
if they have already shipped several versions of the kernel on their
commercial products.
If so, they would take objection to the change, partly or entirely, I think.

I suspect that your port is not a version of the line of the proven,
running kernels
on the shipped products and that your client is not, or doesn't have
contact with,
the developers of the running kernels.

I have taken a look at the kernels for am33 on www.am-linux.jp (*).
What triggered me to comment on your patch was
that its directory and file structure is very different from the ones there.
There are no cpu-, proc- and unit- directories and no names with
mn103e10_* there.

The kernels there seem to be already running on several processors,
some of which are with AM34 cores that you concern about, according to
the #ifdefs there.

I thought at first your port was the revised one after refactoring,
but it has turned out not to be.
You should let your client to discuss with all the developers
concerned in the whole company.

(*) Only a version of an old model is linked from the top page.
Newer models appear to have their unique URIs, and there are v2.6
kernels there, too.

> The problem is how much?  I could just move all the
> cpu-am33v2/ files into the dir above, but what happens if an incompatible CPU
> core is introduced?

How does your client say?
Ok, I agree it's the preparation for incompatible CPU cores.
There might be no incompatible CPU core, currently, though.
But I have become worried about the incompatibility with the running kernels
that have no cpu/ directories.

> It's an awkward situation, yes, but not one most people will be concerned
> with.  There needs to be some way to multiplex alternate register sets.  The
> main ways of doing that are:
>
>  (1) #include hell
>
>  (2) #ifdef hell
>
>  (3) Both
>
> Which do you prefer?

Do the company accept my personal preference?
I prefer (2), as long as the difference is only an addition or the
difference is small enough.

> However, I'll concede that the various versions of AM33 processor should
> perhaps be represented by the same CPU subdirectory (cpu-am33), using #ifdef
> to add extra features.  However, should, say, an AM34 be produced that's
> effectively a variant of the AM33, then that scheme falls down too.

As I said in the other thread,
am33 is a reasonable name if  the AM33 is the first one that is
supported by the port
and the AM34 and newer ones are compatible with that.

> And if all asm/cpu-regs.h does is #include asm/cpu/cpu-regs.h, then what's the
> point in having it?

I meant if there is only an asm/cpu-regs.h and no cpu/ directory at first and
later cpu-regs.h comes up in the cpu/ directories for cpu variants,
you don't have to change the ``#include <asm/cpu-regs.h>'' in the
arch-dependent codes.
cpu-regs.h may not be good for an example.


Anyway, I have no doubt that your port is too immature to be merged
into the upstream kernel.
It doesn't mean there is any technical problem. I understand your position.


-- 
Suzuki Takashi
Japan

Re: [Linux-am33-list] [PATCH 2/2] MN10300: Add the MN10300/AM33 architecture to the kernel [try #2]

[David Howells]

Suzuki Takashi <suzuki.takashi@gmail.com> wrote:

> I'm very annoyed and a bit surprised to hear they admitted such a big
> change so easily.

Why be annoyed?  It's not actually a substantial change.

> You and I know there are consumer devices already out running the am33
> port of the kernel.

Yes, and have been for years, I suspect.

> What triggered me to comment on your patch was that its directory and file
> structure is very different from the ones there.  There are no cpu-, proc-
> and unit- directories and no names with mn103e10_* there.

Things are allowed to change.

> The kernels there seem to be already running on several processors,
> some of which are with AM34 cores that you concern about, according to
> the #ifdefs there.

Yes.

> > The problem is how much?  I could just move all the cpu-am33v2/ files into
> > the dir above, but what happens if an incompatible CPU core is introduced?
> 
> How does your client say?

MEI have agreed for me to do that.  I can always undo it later if sufficiently
incompatible CPU cores arise that warrant that level of segregation.

David