AVR32 architecture patch against Linux 2.6.18-rc1 available

AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

Hi everyone,

I've put up an updated set of patches for AVR32 support at
http://avr32linux.org/twiki/bin/view/Main/LinuxPatches

The most interesting patch probably is
http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch

which, at 544K, is too large to attach here. Please let me know if you
want me to do it anyway.

Anyone want to have a look at this? I understand that a full review is
a huge job, but I'd appreciate a pointer or two in the general
direction that I need to take this in order to get it acceptable for
mainline.

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Andrew Morton]

On Thu, 6 Jul 2006 10:52:27 +0200
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> Hi everyone,
> 
> I've put up an updated set of patches for AVR32 support at
> http://avr32linux.org/twiki/bin/view/Main/LinuxPatches
> 
> The most interesting patch probably is
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch
> 
> which, at 544K, is too large to attach here. Please let me know if you
> want me to do it anyway.
> 
> Anyone want to have a look at this? I understand that a full review is
> a huge job, but I'd appreciate a pointer or two in the general
> direction that I need to take this in order to get it acceptable for
> mainline.
> 

Looks pretty sane from a quick scan.

- request_irq() can use GFP_KERNEL?

- show_interrupts() should use for_each_online_cpu()

<wow, kprobes support>

- do you really need __udivdi3() and friends?  We struggle hard to avoid
  the necessity on x86 and you should be able to leverage that advantage.

- What are these for?

	+EXPORT_SYMBOL(register_dma_controller);
	+EXPORT_SYMBOL(find_dma_controller);

	+EXPORT_SYMBOL(clk_get);
	+EXPORT_SYMBOL(clk_put);
	+EXPORT_SYMBOL(clk_enable);
	+EXPORT_SYMBOL(clk_disable);
	+EXPORT_SYMBOL(clk_get_rate);
	+EXPORT_SYMBOL(clk_round_rate);
	+EXPORT_SYMBOL(clk_set_rate);
	+EXPORT_SYMBOL(clk_set_parent);
	+EXPORT_SYMBOL(clk_get_parent);

- Was there a ./MAINTAINERS patch?  I didn't see one.

- Who stands behind this port?  How do we know this isn't a patch-n-run
  exercise?  How do we know that the code won't rot?

- How does one build a something->avr32 cross-toolchain?

Thanks.

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Russell King]

On Thu, Jul 06, 2006 at 02:19:06AM -0700, Andrew Morton wrote:
> - What are these for?
> 
> 	+EXPORT_SYMBOL(clk_get);
> 	+EXPORT_SYMBOL(clk_put);
> 	+EXPORT_SYMBOL(clk_enable);
> 	+EXPORT_SYMBOL(clk_disable);
> 	+EXPORT_SYMBOL(clk_get_rate);
> 	+EXPORT_SYMBOL(clk_round_rate);
> 	+EXPORT_SYMBOL(clk_set_rate);
> 	+EXPORT_SYMBOL(clk_set_parent);
> 	+EXPORT_SYMBOL(clk_get_parent);

Part of the clock framework.  No, not the one that you've probably
heard about (which is in connection with time, and IMHO incorrectly
named).

This one is to do with controlling the clock sources for peripherals
in systems, to allow drivers to be shared between platforms with the
minimum of crap in drivers.

For example, the ARM Ltd AMBA "devices" end up being embedded into ARM
SoCs, MIPS SoCs and now probably AVR32 SoCs.  These drivers need a way
to be told, eg, their clock source rate in the case of a UART, and be
able to control the clock rate in the case of an LCD controller.  It's
also advantageous for power saving to be able to turn clock sources
on and off.

You can have hardware situations where the clock source for UARTs
depend on where they are on the bus, possibly clocked at different
rates, or all UARTs are clocked from one source.

How that is achieved is extremely SoC or platform specific, and as such
I put together a well defined API (include/linux/clk.h) which gives:

1. an API for drivers to follow to achieve achieve inter-operability and
   re-use with the minimum of fuss.
2. complete flexibility to the platform code to implement the backend
   how they see fit, using whatever structures they see fit.

It's great that the AVR32 folk are reusing this.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Arjan van de Ven]

> 
> - do you really need __udivdi3() and friends?  We struggle hard to avoid
>   the necessity on x86 and you should be able to leverage that advantage.
> 
> - What are these for?
> 
> 	+EXPORT_SYMBOL(register_dma_controller);
> 	+EXPORT_SYMBOL(find_dma_controller);
> 
> 	+EXPORT_SYMBOL(clk_get);
> 	+EXPORT_SYMBOL(clk_put);
> 	+EXPORT_SYMBOL(clk_enable);
> 	+EXPORT_SYMBOL(clk_disable);
> 	+EXPORT_SYMBOL(clk_get_rate);
> 	+EXPORT_SYMBOL(clk_round_rate);
> 	+EXPORT_SYMBOL(clk_set_rate);
> 	+EXPORT_SYMBOL(clk_set_parent);
> 	+EXPORT_SYMBOL(clk_get_parent);

probably wants to be _GPL exports anyway

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 06 Jul 2006 11:58:13 +0200
Arjan van de Ven <arjan@infradead.org> wrote:

> 
> > 
> > - do you really need __udivdi3() and friends?  We struggle hard to
> > avoid the necessity on x86 and you should be able to leverage that
> > advantage.
> > 
> > - What are these for?
> > 
> > 	+EXPORT_SYMBOL(register_dma_controller);
> > 	+EXPORT_SYMBOL(find_dma_controller);
> > 
> > 	+EXPORT_SYMBOL(clk_get);
> > 	+EXPORT_SYMBOL(clk_put);
> > 	+EXPORT_SYMBOL(clk_enable);
> > 	+EXPORT_SYMBOL(clk_disable);
> > 	+EXPORT_SYMBOL(clk_get_rate);
> > 	+EXPORT_SYMBOL(clk_round_rate);
> > 	+EXPORT_SYMBOL(clk_set_rate);
> > 	+EXPORT_SYMBOL(clk_set_parent);
> > 	+EXPORT_SYMBOL(clk_get_parent);
> 
> probably wants to be _GPL exports anyway

If so, ARM should probably be converted as well. On SH, they are
actually _GPL exports.

I'll convert dma_controller stuff anyway.

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Russell King]

On Thu, Jul 06, 2006 at 12:43:15PM +0200, Haavard Skinnemoen wrote:
> On Thu, 06 Jul 2006 11:58:13 +0200
> Arjan van de Ven <arjan@infradead.org> wrote:
> > > 	+EXPORT_SYMBOL(clk_get);
> > > 	+EXPORT_SYMBOL(clk_put);
> > > 	+EXPORT_SYMBOL(clk_enable);
> > > 	+EXPORT_SYMBOL(clk_disable);
> > > 	+EXPORT_SYMBOL(clk_get_rate);
> > > 	+EXPORT_SYMBOL(clk_round_rate);
> > > 	+EXPORT_SYMBOL(clk_set_rate);
> > > 	+EXPORT_SYMBOL(clk_set_parent);
> > > 	+EXPORT_SYMBOL(clk_get_parent);
> > 
> > probably wants to be _GPL exports anyway
> 
> If so, ARM should probably be converted as well. On SH, they are
> actually _GPL exports.

That depends if you consider them to be a low level API or not.  I don't.
They're staying as non-GPL on ARM, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 6 Jul 2006 02:19:06 -0700
Andrew Morton <akpm@osdl.org> wrote:

> On Thu, 6 Jul 2006 10:52:27 +0200
> Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:
> 
> > Hi everyone,
> > 
> > I've put up an updated set of patches for AVR32 support at
> > http://avr32linux.org/twiki/bin/view/Main/LinuxPatches
> > 
> > The most interesting patch probably is
> > http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch
> > 
> > which, at 544K, is too large to attach here. Please let me know if
> > you want me to do it anyway.
> > 
> > Anyone want to have a look at this? I understand that a full review
> > is a huge job, but I'd appreciate a pointer or two in the general
> > direction that I need to take this in order to get it acceptable for
> > mainline.
> > 
> 
> Looks pretty sane from a quick scan.
> 
> - request_irq() can use GFP_KERNEL?

Probably, but the genirq implementation also uses GFP_ATOMIC.

> - show_interrupts() should use for_each_online_cpu()

Indeed. Haven't really touched that code for a while.

> <wow, kprobes support>
> 
> - do you really need __udivdi3() and friends?  We struggle hard to
> avoid the necessity on x86 and you should be able to leverage that
> advantage.

__avr32_udiv64 has only one caller, in time_init(). I'll see if I can
eliminate it.

The 64-bit shift functions are called from architecture-independent
code, so that will be harder.

> - What are these for?
> 
> 	+EXPORT_SYMBOL(register_dma_controller);
> 	+EXPORT_SYMBOL(find_dma_controller);

An attempt at a DMA controller framework, which is used by our MMC
driver as well as a couple of sound drivers. I can remove it from the
patchset if you want, as I haven't forward-ported any of the users yet.

> 	+EXPORT_SYMBOL(clk_get);
> 	+EXPORT_SYMBOL(clk_put);
> 	+EXPORT_SYMBOL(clk_enable);
> 	+EXPORT_SYMBOL(clk_disable);
> 	+EXPORT_SYMBOL(clk_get_rate);
> 	+EXPORT_SYMBOL(clk_round_rate);
> 	+EXPORT_SYMBOL(clk_set_rate);
> 	+EXPORT_SYMBOL(clk_set_parent);
> 	+EXPORT_SYMBOL(clk_get_parent);

Implementation of the linux/clk.h API. David Brownell convinced me to
implement it for AVR32, as it is used by a lot of ARM drivers,
especially the AT91 ones which could mostly be shared with AVR32.

> - Was there a ./MAINTAINERS patch?  I didn't see one.

Sorry, I missed that one. Appended below.

> - Who stands behind this port?  How do we know this isn't a
> patch-n-run exercise?  How do we know that the code won't rot?

Atmel, or more precisely Atmel Norway, which is the design center
that designed the AVR and AVR32 architectures, stands behind it. Atmel
Applications Engineers have received a fair amount of Linux training
already, so I don't think we'll be dropping this on the floor any time
soon.

> - How does one build a something->avr32 cross-toolchain?

I've started writing up a "Getting Started" guide at
http://avr32linux.org/twiki/bin/view/Main/GettingStarted. It's mostly
complete, although I haven't actually uploaded the toolchain patches.
I'll do that in a couple of minutes.

Håvard

diff --git a/MAINTAINERS b/MAINTAINERS
index 196a31c..f7ab4ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -422,6 +422,23 @@ W:	http://people.redhat.com/sgrubb/audit
 T:	git kernel.org:/pub/scm/linux/kernel/git/dwmw2/audit-2.6.git
 S:	Maintained
 
+AVR32 ARCHITECTURE
+P:	Atmel AVR32 Support Team
+M:	avr32@atmel.com
+P:	Haavard Skinnemoen
+M:	hskinnemoen@atmel.com
+W:	http://www.atmel.com/products/AVR32/
+W:	http://avr32linux.org/
+W:	http://avrfreaks.net/
+S:	Supported
+
+AVR32/AT32AP MACHINE SUPPORT
+P:	Atmel AVR32 Support Team
+M:	avr32@atmel.com
+P:	Haavard Skinnemoen
+M:	hskinnemoen@atmel.com
+S:	Supported
+
 AX.25 NETWORK LAYER
 P:	Ralf Baechle
 M:	ralf@linux-mips.org

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Andrew Morton]

On Thu, 6 Jul 2006 12:03:19 +0200
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> 
> [stuff]
>

OK, thanks.  Send me the whole lot when you think it's ready and we'll get
it into the pipeline.  Not for 2.6.18 though - we need to give people time
to look through it and send you nastygrams ;)

> > - How does one build a something->avr32 cross-toolchain?
> 
> I've started writing up a "Getting Started" guide at
> http://avr32linux.org/twiki/bin/view/Main/GettingStarted. It's mostly
> complete, although I haven't actually uploaded the toolchain patches.
> I'll do that in a couple of minutes.

OK, well the more you can do there the better - that way crazy people
cross-compile for your architecture and save you work.

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 6 Jul 2006 03:14:16 -0700
Andrew Morton <akpm@osdl.org> wrote:

> On Thu, 6 Jul 2006 12:03:19 +0200
> Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:
> 
> > 
> > [stuff]
> >
> 
> OK, thanks.  Send me the whole lot when you think it's ready and
> we'll get it into the pipeline.  Not for 2.6.18 though - we need to
> give people time to look through it and send you nastygrams ;)

Great. I'll send you a patch when I get most/all the suggested changes
implemented.

> > > - How does one build a something->avr32 cross-toolchain?
> > 
> > I've started writing up a "Getting Started" guide at
> > http://avr32linux.org/twiki/bin/view/Main/GettingStarted. It's
> > mostly complete, although I haven't actually uploaded the toolchain
> > patches. I'll do that in a couple of minutes.
> 
> OK, well the more you can do there the better - that way crazy people
> cross-compile for your architecture and save you work.

I've uploaded patches for the basic stuff like binutils and gcc.
I've also updated the Getting Started page so that it should at least be
possible to build the kernel by following the instructions there.

So, all you crazy people should be able to start breaking stuff now ;)

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 6 Jul 2006 03:14:16 -0700
Andrew Morton <akpm@osdl.org> wrote:

> OK, thanks.  Send me the whole lot when you think it's ready and
> we'll get it into the pipeline.  Not for 2.6.18 though - we need to
> give people time to look through it and send you nastygrams ;)

I've put up an updated patch at
http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-3.patch

which fixes most of the issues people have pointed out. Thanks a lot
to everyone for taking the time to look through this.

I've appended the changelog below, but first I'll mention the things I
didn't fix and why:

There are three libgcc functions left, which handle the three possible
variants of 64-bit shift. There's no easy way to eliminate these, but
maybe our gcc maintainer can get gcc to emit the instructions inline
instead. However, these functions are actually specified by the AVR32
ABI, so they should be the same no matter which compiler you use.

The clk API is still exported as non-GPL. I don't feel very strongly
one way or another, but since Russell is keeping them non-GPL on ARM,
it makes most sense to keep them non-GPL on AVR32 as well.

I've kept the volatiles in the arguments to the bitops functions as
they are. I'm not sure if they're really needed, but as I understood
from reading the recent thread about spinlocks, this doesn't fall in
the category of "obviously bad" usage of volatile.

Here's the shortlog:
      Add MAINTAINERS entries for AVR32 and AT32AP
      Remove EARLY_PRINTK support
      Remove CONFIG_DW_DMAC symbol
      Remove DMA controller framework
      Add Kbuild file for 'make headers_install' on AVR32
      Remove #ifdef __KERNEL__ from asm/atomic.h
      Remove #ifdef __KERNEL__ from asm/bitops.h
      Remove #ifdef __KERNEL__ from asm/dma-mapping.h
      Remove #ifdef __KERNEL__ from asm/mmu_context.h
      Remove #ifdef __KERNEL__ from asm/semaphore.h
      Remove #ifdef __KERNEL__ from asm/thread_info.h
      Move PAGE_SIZE and friends inside #ifdef __KERNEL__
      Wrap __FD_SET and friends inside __KERNEL__
      Kill _syscall[0123456]() and hand-code execve() instead
      Remove obsolete #include <linux/config.h>
      Replace MB -> MiB and KB -> KiB
      Wire up 39 new syscalls
      Use do_div() instead of open-coded div64 in time_init()
      Use for_each_online_cpu in show_interrupts()
      Add asm/futex.h
      Ensure kprobes compiles again
      Switch to genirq framework
      Fix cpu_idle preempt bug
      Process softirqs the usual way
      Handle preempt in the interrupt- and exception handlers

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Andrew Morton]

On Mon, 10 Jul 2006 11:03:25 +0200
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> On Thu, 6 Jul 2006 03:14:16 -0700
> Andrew Morton <akpm@osdl.org> wrote:
> 
> > OK, thanks.  Send me the whole lot when you think it's ready and
> > we'll get it into the pipeline.  Not for 2.6.18 though - we need to
> > give people time to look through it and send you nastygrams ;)
> 
> I've put up an updated patch at
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-3.patch

Thanks, I'll grab it.

> which fixes most of the issues people have pointed out. Thanks a lot
> to everyone for taking the time to look through this.
> 
> I've appended the changelog below, but first I'll mention the things I
> didn't fix and why:
> 
> There are three libgcc functions left, which handle the three possible
> variants of 64-bit shift. There's no easy way to eliminate these, but
> maybe our gcc maintainer can get gcc to emit the instructions inline
> instead. However, these functions are actually specified by the AVR32
> ABI, so they should be the same no matter which compiler you use.

Sure, if the compier doesn't inline 64-bit shift then you'll certainly need
the library function.

> The clk API is still exported as non-GPL. I don't feel very strongly
> one way or another, but since Russell is keeping them non-GPL on ARM,
> it makes most sense to keep them non-GPL on AVR32 as well.

OK.

> I've kept the volatiles in the arguments to the bitops functions as
> they are. I'm not sure if they're really needed, but as I understood
> from reading the recent thread about spinlocks, this doesn't fall in
> the category of "obviously bad" usage of volatile.

Nope.  i386 does it too, as does include/asm-generic/bitops/atomic.h
(wrongly, I suspect.  The volatile gets typecast away.)

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Andrew Morton]

On Mon, 10 Jul 2006 11:03:25 +0200
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> I've put up an updated patch at
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-3.patch

That diff doesn't update ./MAINTAINERS

Please prepare a nice changelog describing the architecture (what's an
avr?), who supports it, useful web pages, how to build cross-tools, etc. 
The sort of things which Linus and kernel developers should know when
someone sends in a half-megabyte patch.

And a signed-off-by:, as per section 11 of Documentation/SubmittingPatches.

Thanks.

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Mon, 10 Jul 2006 02:37:58 -0700
Andrew Morton <akpm@osdl.org> wrote:

> On Mon, 10 Jul 2006 11:03:25 +0200
> Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:
> 
> > I've put up an updated patch at
> > http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-3.patch
> 
> That diff doesn't update ./MAINTAINERS

Crap, I can't belive I left that out two times in a row...

> Please prepare a nice changelog describing the architecture (what's an
> avr?), who supports it, useful web pages, how to build cross-tools,
> etc. The sort of things which Linus and kernel developers should know
> when someone sends in a half-megabyte patch.
>
> And a signed-off-by:, as per section 11 of
> Documentation/SubmittingPatches.

Updated patch with MAINTAINERS, a much longer changelog entry and
Signed-off-by:

http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-4.patch

By the way, there's a separate patch #ifdef'ing out /dev/port available
from the same LinuxPatches page. Should I fold it into the avr32-arch
patch? I don't consider this a permanent solution anyway, so I made it
a separate patch in order to drop it more easily when we actually
implement port-based I/O...

Thanks,
Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[David Woodhouse]

On Mon, 2006-07-10 at 11:03 +0200, Haavard Skinnemoen wrote:
+	.global	__sys_pselect6
+	.type	__sys_pselect6,@function
+__sys_pselect6:
+	pushm	lr
+	st.w	--sp, ARG6
+	rcall	sys_pselect6
+	sub	sp, -4
+	popm	pc


sys_pselect6() is just a hackish workaround for the fact that most
architectures don't allow seven-argument syscalls. Having your own
workaround in assembly, which then calls sys_pselect6(), seems a little
odd -- why not call sys_pselect7() directly instead from your assembly
wrapper?

Stop a moment and work out precisely what the best way to pass the
arguments _is_ if you have only 5 registers and the stack, perhaps.

-- 
dwmw2

¹ Note that I'm just _asking_ -- the answer "Uli doesn't want to support
anything but the basic 6-argument sys_pselect6() in glibc" is an
acceptable answer on your part.

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Mon, 10 Jul 2006 10:57:33 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Mon, 2006-07-10 at 11:03 +0200, Haavard Skinnemoen wrote:
> +	.global	__sys_pselect6
> +	.type	__sys_pselect6,@function
> +__sys_pselect6:
> +	pushm	lr
> +	st.w	--sp, ARG6
> +	rcall	sys_pselect6
> +	sub	sp, -4
> +	popm	pc
> 
> 
> sys_pselect6() is just a hackish workaround for the fact that most
> architectures don't allow seven-argument syscalls. Having your own
> workaround in assembly, which then calls sys_pselect6(), seems a
> little odd -- why not call sys_pselect7() directly instead from your
> assembly wrapper?

Good point. The reason I did it this way is because I haven't written a
_syscall7() stub for uClibc yet, but I guess I should probably get that
done. I did a quick grep through include/asm-* and didn't find any
other architectures defining __NR_pselect7...

> Stop a moment and work out precisely what the best way to pass the
> arguments _is_ if you have only 5 registers and the stack, perhaps.

The regular AVR32 call convention uses r12-r8 to pass up to 5 arguments
in registers, while any additional arguments are pushed on the stack.

For syscalls, I've modified this to pass the syscall number in r8 and
no arguments on the stack. arg5 is passed in r5 and moved to r8 by the
low-level syscall handler, while arg6 is passed through r3 and pushed
on the stack only if necessary.

So the best way to implement pselect7 is probably to pass the seventh
argument through r2 -- or maybe r4.

> ¹ Note that I'm just _asking_ -- the answer "Uli doesn't want to
> support anything but the basic 6-argument sys_pselect6() in glibc" is
> an acceptable answer on your part.

Well, we haven't actually ported glibc yet. But if 7-argument syscalls
is going to cause problems later, we should probably not do it...

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Thomas Gleixner]

On Thu, 2006-07-06 at 12:03 +0200, Haavard Skinnemoen wrote: 
> > Looks pretty sane from a quick scan.
> > 
> > - request_irq() can use GFP_KERNEL?
> 
> Probably, but the genirq implementation also uses GFP_ATOMIC.

Is there a good reason, why AVR32 needs its own interrupt handling
implementation ?

>From a short glance there's nothing which can not be handled by the
generic code. Also there are a couple of things missing -e.g. recursive
enable/disable_irq() handling. 

	tglx

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 06 Jul 2006 13:30:25 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Thu, 2006-07-06 at 12:03 +0200, Haavard Skinnemoen wrote: 
> > > Looks pretty sane from a quick scan.
> > > 
> > > - request_irq() can use GFP_KERNEL?
> > 
> > Probably, but the genirq implementation also uses GFP_ATOMIC.
> 
> Is there a good reason, why AVR32 needs its own interrupt handling
> implementation ?

No, not really. At least not after the genirq stuff went in. I used to
be a bit concerned about the generic irq code being too heavyweight,
but I think handle_simple_irq() might be just what we need for
chip-internal interrupt handling.

> >From a short glance there's nothing which can not be handled by the
> generic code. Also there are a couple of things missing -e.g.
> recursive enable/disable_irq() handling. 

You're probably right. I'll see if I can get it converted to genirq
one of the next days.

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Thomas Gleixner]

On Thu, 2006-07-06 at 13:50 +0200, Haavard Skinnemoen wrote:
> On Thu, 06 Jul 2006 13:30:25 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Thu, 2006-07-06 at 12:03 +0200, Haavard Skinnemoen wrote: 
> > > > Looks pretty sane from a quick scan.
> > > > 
> > > > - request_irq() can use GFP_KERNEL?
> > > 
> > > Probably, but the genirq implementation also uses GFP_ATOMIC.
> > 
> > Is there a good reason, why AVR32 needs its own interrupt handling
> > implementation ?
> 
> No, not really. At least not after the genirq stuff went in. I used to
> be a bit concerned about the generic irq code being too heavyweight,
> but I think handle_simple_irq() might be just what we need for
> chip-internal interrupt handling.
> 
> > >From a short glance there's nothing which can not be handled by the
> > generic code. Also there are a couple of things missing -e.g.
> > recursive enable/disable_irq() handling. 
> 
> You're probably right. I'll see if I can get it converted to genirq
> one of the next days.

Good. If there are any questions or things you find missing, don't
hesitate to ask.

	tglx

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[David Woodhouse]

On Thu, 2006-07-06 at 10:52 +0200, Haavard Skinnemoen wrote:
> Hi everyone,
> 
> I've put up an updated set of patches for AVR32 support at
> http://avr32linux.org/twiki/bin/view/Main/LinuxPatches
> 
> The most interesting patch probably is
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch

Please add include/asm-avr32/Kbuild which lists those files which need
to be present in /usr/include/asm, over and above those listed in
asm-generic/Kbuild.asm. Then run 'make headers_install' and review the
exported headers to make sure they're suitable for building glibc, etc.

Should probably drop the #ifdef __KERNEL__ from asm/atomic.h. There's no
excuse for anything non-kernel to be using atomic.h, so by the time you
merge that'll probably have been taken out of asm-generic/Kbuild.asm.
Likewise bitops.h and anything else which isn't, or shouldn't be,
exported (including dma-mapping.h, probably also io.h, etc.)

You define PAGE_SIZE in asm/page.h where userspace can see it. No need
for that -- portable userspace must use sysconf(_SC_PAGE_SIZE) anyway --
you can move it inside the #ifdef __KERNEL__ you already have there.

Your posix_types.h wraps __FD_SET et al in 
#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
I suspect that's not at all needed, so it can be fixed to just
__KERNEL__ as we did on s390 recently iirc.

Kill syscall[012456] from your unistd.h -- they aren't used anywhere. In
fact, you might as well kill syscall3() too, since you only use it in
one place for execve. Just do execve 'manually' with the appropriate
inline asm.

Do you really need the EARLY_PRINTK crap? Can't you just register your
proper console nice and early? There's no need to wait for
console_init() and use console_initcall(). You can do it right at the
beginning of setup_arch(), as long as you parse the command line early
enough for console= options. 

You're including <linux/config.h> in a few places -- kill them all.

"DMA controller framework".... isn't that what drivers/dma was recently
invented for? If appropriate, you should probably use that. If not, you
should explain why, and perhaps we should get it fixed.

You're a bit behind on syscall support -- I note you have
TIF_RESTORE_SIGMASK (which means you're ahead of x86_64) but you haven't
wired up ppoll() and pselect(), amongst others.

You say 'MB' in a few places where you actually mean 'MiB', probably
copied from sloppy code elsewhere.

-- 
dwmw2

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 06 Jul 2006 12:58:03 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> [lots of stuff I'll get fixed]
> 
> Do you really need the EARLY_PRINTK crap? Can't you just register your
> proper console nice and early? There's no need to wait for
> console_init() and use console_initcall(). You can do it right at the
> beginning of setup_arch(), as long as you parse the command line early
> enough for console= options. 

I didn't know that. But since the "proper console" would usually mean
at91_serial, I'd have to remove console_initcall() from there and call
it directly from setup_arch().

I suppose I could just remove the EARLY_PRINTK crap now and worry
about driver modifications later.

> You're including <linux/config.h> in a few places -- kill them all.

Will do.

> "DMA controller framework".... isn't that what drivers/dma was
> recently invented for? If appropriate, you should probably use that.
> If not, you should explain why, and perhaps we should get it fixed.

It was written some time before the drivers/dma stuff. I suppose I
should try to use the DMA subsystem instead.

As I understand it, though, drivers/dma is mostly for memory-to-memory
to transfers, while what I really need is memory-to-hardware and
hardware-to-memory transfers.

I'll just drop the DMA controller stuff for now as it doesn't have any
users yet anyway.

> You're a bit behind on syscall support -- I note you have
> TIF_RESTORE_SIGMASK (which means you're ahead of x86_64) but you
> haven't wired up ppoll() and pselect(), amongst others.

I'll sync up with i386. By the way, are there any syscalls I
_shouldn't_ have wired up? It's probably too late to remove any of them
at this point, but if we get it sorted out quickly, we might get away
with a shorter grace period than usual...

> You say 'MB' in a few places where you actually mean 'MiB', probably
> copied from sloppy code elsewhere.

Will do.

Thanks,
Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[David Woodhouse]

On Thu, 2006-07-06 at 16:13 +0200, Haavard Skinnemoen wrote:
> > "DMA controller framework".... isn't that what drivers/dma was
> > recently invented for? If appropriate, you should probably use that.
> > If not, you should explain why, and perhaps we should get it fixed.
> 
> It was written some time before the drivers/dma stuff. I suppose I
> should try to use the DMA subsystem instead.
> 
> As I understand it, though, drivers/dma is mostly for memory-to-memory
> to transfers, while what I really need is memory-to-hardware and
> hardware-to-memory transfers.

With MMIO those are just a not-so-special case of memory-memory, surely?
If the new framework doesn't support that, it probably _should_.

> > You're a bit behind on syscall support -- I note you have
> > TIF_RESTORE_SIGMASK (which means you're ahead of x86_64) but you
> > haven't wired up ppoll() and pselect(), amongst others.
> 
> I'll sync up with i386. By the way, are there any syscalls I
> _shouldn't_ have wired up? It's probably too late to remove any of them
> at this point, but if we get it sorted out quickly, we might get away
> with a shorter grace period than usual... 

Looks OK -- I don't see any of the obvious ones like oldstat, oldfstat,
etc.

-- 
dwmw2

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 06 Jul 2006 15:34:52 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Thu, 2006-07-06 at 16:13 +0200, Haavard Skinnemoen wrote:

> > As I understand it, though, drivers/dma is mostly for
> > memory-to-memory to transfers, while what I really need is
> > memory-to-hardware and hardware-to-memory transfers.
> 
> With MMIO those are just a not-so-special case of memory-memory,
> surely? If the new framework doesn't support that, it probably
> _should_.

Yes, but there are at least two important differences:

   * Hanshaking. The DMA controller must know when the peripheral has
     new data available/is able to accept more data. Thus, you need to
     specify which set of handshaking signals to use as well as which
     direction the data is moved.
   * One of the pointers often stays the same during the whole transfer.

I'm willing to give the drivers/dma framework a try, though, when the
time comes to forward-port the drivers that need such infrastructure.

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[David Woodhouse]

On Thu, 2006-07-06 at 20:48 +0200, Haavard Skinnemoen wrote:
> > With MMIO those are just a not-so-special case of memory-memory,
> > surely? If the new framework doesn't support that, it probably
> > _should_.
> 
> Yes, but there are at least two important differences:
> 
>    * Hanshaking. The DMA controller must know when the peripheral has
>      new data available/is able to accept more data. Thus, you need to
>      specify which set of handshaking signals to use as well as which
>      direction the data is moved.
>    * One of the pointers often stays the same during the whole transfer. 

Those are hardly esoteric features -- the same goes for just about every
sane DMA controller on every architecture already. Any "generic DMA
framework" which isn't entirely crack-inspired is surely going to handle
it properly.

-- 
dwmw2

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Thu, 06 Jul 2006 12:58:03 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> Please add include/asm-avr32/Kbuild which lists those files which need
> to be present in /usr/include/asm, over and above those listed in
> asm-generic/Kbuild.asm. Then run 'make headers_install' and review the
> exported headers to make sure they're suitable for building glibc,
> etc.

uClibc fails because it tries to grep for UTS_RELEASE in
linux/version.h and doesn't find it because it's been moved to
linux/utsrelease.h. I can update the script so that it checks
linux/utsrelease.h first, but that file is not installed by
make headers_install.

Should utsrelease.h be added to objhdr-y or should uClibc handle this
some other way?

Håvard

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Arjan van de Ven]

On Fri, 2006-07-07 at 12:39 +0200, Haavard Skinnemoen wrote:
> On Thu, 06 Jul 2006 12:58:03 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > Please add include/asm-avr32/Kbuild which lists those files which need
> > to be present in /usr/include/asm, over and above those listed in
> > asm-generic/Kbuild.asm. Then run 'make headers_install' and review the
> > exported headers to make sure they're suitable for building glibc,
> > etc.
> 
> uClibc fails because it tries to grep for UTS_RELEASE in
> linux/version.h and doesn't find it because it's been moved to
> linux/utsrelease.h. I can update the script so that it checks
> linux/utsrelease.h first, but that file is not installed by
> make headers_install.

I would say that version.h should most certainly not be exposed to
userspace. utsrelease.h.. since it's exposing a string... MAYBE. But
really.. how can userspace use this at all in any useful way?

Greetings,
    Arjan van de Ven

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Nick Piggin]

Haavard Skinnemoen wrote:
> Hi everyone,
> 
> I've put up an updated set of patches for AVR32 support at
> http://avr32linux.org/twiki/bin/view/Main/LinuxPatches
> 
> The most interesting patch probably is
> http://avr32linux.org/twiki/pub/Main/LinuxPatches/avr32-arch-2.patch
> 
> which, at 544K, is too large to attach here. Please let me know if you
> want me to do it anyway.
> 
> Anyone want to have a look at this? I understand that a full review is
> a huge job, but I'd appreciate a pointer or two in the general
> direction that I need to take this in order to get it acceptable for
> mainline.
> 

Hi,

+void cpu_idle(void)
+{
+	/* endless idle loop with no priority at all */
+	while (1) {
+		/* TODO: Enter sleep mode */
+		if (need_resched())
+			schedule();
+	}
+}

AFAIKS, this is buggy.

need_resched() translates to a test_bit, which doesn't have any barriers,
so it could be optimised away completely. And if you're intending to use
preempt, you need to have preemption disabled in the idle loop.

Documentation/sched-arch.txt attempts to explain, and  something like
arm26's cpu_idle() is a nice, simple example to follow.

Actually, I'm wrong about the test_bit. It casts to volatile there, which
is probably why you don't end up with infinite loops. Still, it would be
nicer to have an explicit barrier (eg. cpu_relax()).

Why do we cast to volatile in places like this? Linus? I don't see why
test_bit() should be any more "special" than the & operator. What's more,
some architectures do cast and others don't, which is just insane.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Nick Piggin]

Nick Piggin wrote:

> I don't see why test_bit() should be any more "special" than the & operator.

OTOH, I guess test_bit isn't a double underscore variant, and so perhaps
it is. What are the semantics of these things supposed to be? It gets
a bit complex if you're just trying to decide whether need_resched()
needs a barrier() or not :(

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: AVR32 architecture patch against Linux 2.6.18-rc1 available

[Haavard Skinnemoen]

On Fri, 07 Jul 2006 18:08:48 +1000
Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> need_resched() translates to a test_bit, which doesn't have any
> barriers, so it could be optimised away completely. And if you're
> intending to use preempt, you need to have preemption disabled in the
> idle loop.

Actually, preemption seems to be disabled when entering cpu_idle(), but
it must be enabled around the call to schedule(). That explains the
"scheduling while atomic" storm we saw when enabling CONFIG_PREEMPT
anyway.

CONFIG_PREEMPT support is one of the things still missing, btw. I'll
implement that next, now that genirq seems to be working.

> Documentation/sched-arch.txt attempts to explain, and  something like
> arm26's cpu_idle() is a nice, simple example to follow.

I'll do that. Thanks.

Håvard