Re: [PATCH 5/6] MN10300: Add the MN10300/AM33 architecture to the kernel [try #5]

[David Howells <dhowells@redhat.com>]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> memory mapped registers should be read with readw and friends and that
> should contain the volatile not the public code.

So you would say change the global h/w register variables[*] to be addresses
instead, and change all the references to be readX and writeX?  I'm wary of
doing that in case some special logic needs to be inserted in there to handle
funny buses differently.  I used to have code like that in there, though
there's none at the moment.  On the other hand, since the CPU addresses are for
the most part constants, I can probably do something clever with
__builtin_const_p() if it comes to that.

 [*] This includes auxiliary CPU control registers, though I suppose that
     doesn't make any difference.

> Similarly spin_lock/unlock are store barriers so for ring buffers should
> be sufficient unless you have cache management requirements in which case
> the dma_* APIs will handle those bits.

I don't actually need locks, but sticking smp_rmb()/smp_wmb() is probably the
right thing to do now that I know how to use them.  This code was written five
or six years ago and I haven't really thought about changing that since.

I don't see how the dma_* APIs would help.  The buffer is filled by a higher
priority interrupt routine that does 'virtual DMA'.  It's not actually done by
real DMA.  Normal interrupt disablement doesn't really disable interrupts, it
justs excludes normal priority interrupts.

The virtual DMA is done is ASM as it has to be really quick.  It's unfortunate,
but, the on-chip serial ports don't have a FIFO.

David