On Monday 27 April 2015 14:35:08 Yoshinori Sato wrote: > diff --git a/arch/h8300/include/asm/asm-offsets.h b/arch/h8300/include/asm/asm-offsets.h > new file mode 100644 > index 0000000..d370ee3 > --- /dev/null > +++ b/arch/h8300/include/asm/asm-offsets.h > @@ -0,0 +1 @@ > +#include <generated/asm-offsets.h> Could you add a file with these contents to include/asm-generic and use that instead of providing your own copy? > diff --git a/arch/h8300/include/asm/delay.h b/arch/h8300/include/asm/delay.h > new file mode 100644 > index 0000000..2bdde59 > --- /dev/null > +++ b/arch/h8300/include/asm/delay.h > @@ -0,0 +1,38 @@ > +#ifndef _H8300_DELAY_H > +#define _H8300_DELAY_H > + > +#include <asm/param.h> > + > +/* > + * Copyright (C) 2002 Yoshinori Sato <ysato@xxxxxxxxxxxxxx> > + * > + * Delay routines, using a pre-computed "loops_per_second" value. > + */ > + > +static inline void __delay(unsigned long loops) > +{ > + __asm__ __volatile__ ("1:\n\t" > + "dec.l #1,%0\n\t" > + "bne 1b" > + : "=r" (loops) : "0"(loops)); > +} > + This could be optimized by using the clocksource instead, if that is accurate enough. Doing so will speed up the boot as well, because you can avoid calibrating the delay loop. > +#endif /* _H8300_DELAY_H */ > diff --git a/arch/h8300/include/asm/device.h b/arch/h8300/include/asm/device.h > new file mode 100644 > index 0000000..06746c5 > --- /dev/null > +++ b/arch/h8300/include/asm/device.h > @@ -0,0 +1,6 @@ > +/* > + * Arch specific extensions to struct device > + * > + * This file is released under the GPLv2 > + */ > +#include <asm-generic/device.h> This one can obviously just use 'generic-y' isntead of including the file. > diff --git a/arch/h8300/include/asm/emergency-restart.h b/arch/h8300/include/asm/emergency-restart.h > new file mode 100644 > index 0000000..108d8c4 > --- /dev/null > +++ b/arch/h8300/include/asm/emergency-restart.h > @@ -0,0 +1,6 @@ > +#ifndef _ASM_EMERGENCY_RESTART_H > +#define _ASM_EMERGENCY_RESTART_H > + > +#include <asm-generic/emergency-restart.h> > + > +#endif /* _ASM_EMERGENCY_RESTART_H */ Same here. > diff --git a/arch/h8300/include/asm/io.h b/arch/h8300/include/asm/io.h > new file mode 100644 > index 0000000..51ee096 > --- /dev/null > +++ b/arch/h8300/include/asm/io.h > @@ -0,0 +1,314 @@ > +#ifndef _H8300_IO_H > +#define _H8300_IO_H > + > +#ifdef __KERNEL__ > + > +#include <linux/types.h> > + > +#define __raw_readb(addr) ({ u8 __v = *(volatile u8 *)(addr); __v; }) > + > +#define __raw_readw(addr) ({ u16 __v = *(volatile u16 *)(addr); __v; }) > + > +#define __raw_readl(addr) ({ u32 __v = *(volatile u32 *)(addr); __v; }) > + > +#define __raw_writeb(b, addr) (void)((*(volatile u8 *)(addr)) = (b)) > + > +#define __raw_writew(b, addr) (void)((*(volatile u16 *)(addr)) = (b)) > + > +#define __raw_writel(b, addr) (void)((*(volatile u32 *)(addr)) = (b)) > + > +#define readb __raw_readb > +#define readw __raw_readw > +#define readl __raw_readl > +#define writeb __raw_writeb > +#define writew __raw_writew > +#define writel __raw_writel We have recently changed this so you now have to provide readl_relaxed() and writel_relaxed() as well. As a side-note: The current definition here prevents you from ever using PCI devices, which are by definition little-endian. If there is any chance that you might need to support PCI devices later, better don't use the readl/writel family of accessors for platform specific drivers and use ioread_be32/iowrite_be32 (or an h8300-specific variant thereof) for any big-endian devices, and define read() etc to do a byte swap. > +#if defined(CONFIG_H83069) > +#define ABWCR 0xFEE020 > +#elif defined(CONFIG_H8S2678) > +#define ABWCR 0xFFFEC0 > +#endif > + > +#ifdef CONFIG_H8300_BUBSSWAP > +#define _swapw(x) __builtin_bswap16(x) > +#define _swapl(x) __builtin_bswap32(x) > +#else > +#define _swapw(x) (x) > +#define _swapl(x) (x) > +#endif Is this swapping configurable by software? The best way to do this is normally not to do any bus swapping in hardware at all but let the drivers take care of it, otherwise you will get things wrong in the end. > +static inline void io_outsw(unsigned int addr, const void *buf, int len) > +{ > + volatile unsigned short *ap = (volatile unsigned short *) addr; > + unsigned short *bp = (unsigned short *) buf; > + > + while (len--) > + *ap = _swapw(*bp++); > +} > + > +static inline void io_outsl(unsigned int addr, const void *buf, int len) > +{ > + volatile unsigned short *ap = (volatile unsigned short *) addr; > + unsigned short *bp = (unsigned short *) buf; > + > + while (len--) { > + *(ap + 1) = _swapw(*(bp + 0)); > + *(ap + 0) = _swapw(*(bp + 1)); > + bp += 2; > + } > +} In particular, the outsw/insw/readsw/writesw/iowrite16_rep/ioread16_rep() family of function is assumed to never perform any byte swapping, because they are used to access FIFO registers from a lot of the drivers. These FIFOs normally contain byte streams in memory order, and Linux drivers are written to assume that normal mmio registers may need byte swaps while fifo registers don't. What you have here is basically doing the opposite: you assume that mmio registers are configured to be the same endianess as the CPU by using an extra hardware bus swap, while the fifos will need to be swapped back as a side effect of the hardware swap. This can work if none of your drivers are shared with other architectures, but the more you share, the more problems it will cause. > + > +#define inb(addr) ((h8300_buswidth(addr)) ? \ > + __raw_readw((addr) & ~1) & 0xff:__raw_readb(addr)) > +#define inw(addr) _swapw(__raw_readw(addr)) > +#define inl(addr) (_swapw(__raw_readw(addr) << 16 | \ > + _swapw(__raw_readw(addr + 2)))) > +#define outb(x, addr) ((void)((h8300_buswidth(addr) && ((addr) & 1)) ? \ > + __raw_writeb(x, (addr) & ~1) : \ > + __raw_writeb(x, addr))) > +#define outw(x, addr) ((void) __raw_writew(_swapw(x), addr)) > +#define outl(x, addr) \ > + ((void) __raw_writel(_swapw(x & 0xffff) | \ > + _swapw(x >> 16) << 16, addr)) > + > +#define inb_p(addr) inb(addr) > +#define inw_p(addr) inw(addr) > +#define inl_p(addr) inl(addr) > +#define outb_p(x, addr) outb(x, addr) > +#define outw_p(x, addr) outw(x, addr) > +#define outl_p(x, addr) outl(x, addr) > + > +#define outsb(a, b, l) io_outsb(a, b, l) > +#define outsw(a, b, l) io_outsw(a, b, l) > +#define outsl(a, b, l) io_outsl(a, b, l) > + > +#define insb(a, b, l) io_insb(a, b, l) > +#define insw(a, b, l) io_insw(a, b, l) > +#define insl(a, b, l) io_insl(a, b, l) It's probably better not to define these macros if you do not have PCI devices. They have a very specific meaning on PCI, and you don't seem to use them this way. > +#define IO_SPACE_LIMIT 0xffffff In particular, you don't enforce the IO_SPACE_LIMIT for the addresses: What you normally have is one part of the physical address space that maps to PCI I/O ports, so your inb would look like #define inb(addr) (readl(IO_SPACE_START + (addr & IO_SPACE_LIMIT)) > +#define ioread8(a) __raw_readb(a) > +#define ioread16(a) __raw_readw(a) > +#define ioread32(a) __raw_readl(a) > + > +#define iowrite8(v, a) __raw_writeb((v), (a)) > +#define iowrite16(v, a) __raw_writew((v), (a)) > +#define iowrite32(v, a) __raw_writel((v), (a)) > + > +#define ioread8_rep(p, d, c) insb(p, d, c) > +#define ioread16_rep(p, d, c) insw(p, d, c) > +#define ioread32_rep(p, d, c) insl(p, d, c) > +#define iowrite8_rep(p, s, c) outsb(p, s, c) > +#define iowrite16_rep(p, s, c) outsw(p, s, c) > +#define iowrite32_rep(p, s, c) outsl(p, s, c) > diff --git a/arch/h8300/include/asm/smp.h b/arch/h8300/include/asm/smp.h > new file mode 100644 > index 0000000..9e9bd7e > --- /dev/null > +++ b/arch/h8300/include/asm/smp.h > @@ -0,0 +1 @@ > +/* nothing required here yet */ > diff --git a/arch/h8300/include/asm/spinlock.h b/arch/h8300/include/asm/spinlock.h > new file mode 100644 > index 0000000..d5407fa > --- /dev/null > +++ b/arch/h8300/include/asm/spinlock.h > @@ -0,0 +1,6 @@ > +#ifndef __H8300_SPINLOCK_H > +#define __H8300_SPINLOCK_H > + > +#error "H8/300 doesn't do SMP yet" > + > +#endif generic file? > diff --git a/arch/h8300/include/asm/topology.h b/arch/h8300/include/asm/topology.h > new file mode 100644 > index 0000000..fdc1219 > --- /dev/null > +++ b/arch/h8300/include/asm/topology.h > @@ -0,0 +1,6 @@ > +#ifndef _ASM_H8300_TOPOLOGY_H > +#define _ASM_H8300_TOPOLOGY_H > + > +#include <asm-generic/topology.h> > + > +#endif /* _ASM_H8300_TOPOLOGY_H */ same here Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html