From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Schwierzeck Date: Sat, 16 Jan 2016 17:26:37 +0100 Subject: [U-Boot] [PATCH v6 06/10] ns16550: add support for mips In-Reply-To: References: <1451906101-9801-2-git-send-email-wills.wang@live.com> <1452270226.14012.36.camel@gmail.com> <1452349847.6612.3.camel@gmail.com> Message-ID: <1452961597.4119.18.camel@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am Sonntag, den 17.01.2016, 00:15 +0800 schrieb Wills Wang: > > On Saturday, January 09, 2016 10:30 PM, Daniel Schwierzeck wrote: > > Am Samstag, den 09.01.2016, 18:46 +0800 schrieb Wills Wang: > > > On 01/09/2016 12:23 AM, Daniel Schwierzeck wrote: > > > > Am Montag, den 04.01.2016, 19:14 +0800 schrieb Wills Wang: > > > > > MIPS archtecture have no "in_le32/in_be32/out_le32/out_be32" > > > > > macro, > > > > > but usually define CONFIG_SYS_BIG_ENDIAN, this patch use > > > > > readl/writel > > > > > for register operation in mips when define > > > > > CONFIG_SYS_NS16550_MEM32. > > > > > > > > > > Signed-off-by: Wills Wang > > > > > --- > > > > > > > > > > Changes in v6: None > > > > > Changes in v5: None > > > > > Changes in v4: None > > > > > Changes in v3: None > > > > > Changes in v2: None > > > > > > > > > > drivers/serial/ns16550.c | 24 ++++++++++++++++-------- > > > > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/drivers/serial/ns16550.c > > > > > b/drivers/serial/ns16550.c > > > > > index 3fab3f1..3b24af0 100644 > > > > > --- a/drivers/serial/ns16550.c > > > > > +++ b/drivers/serial/ns16550.c > > > > > @@ -64,12 +64,16 @@ static inline void serial_out_shift(void > > > > > *addr, > > > > > int shift, int value) > > > > > { > > > > > #ifdef CONFIG_SYS_NS16550_PORT_MAPPED > > > > > outb(value, (ulong)addr); > > > > > -#elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > > !defined(CONFIG_SYS_BIG_ENDIAN) > > > > > - out_le32(addr, value); > > > > > -#elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > > defined(CONFIG_SYS_BIG_ENDIAN) > > > > > - out_be32(addr, value); > > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) > > > > > +#ifdef CONFIG_MIPS > > > > > writel(value, addr); > > > > > +#else > > > > > +#ifndef CONFIG_SYS_BIG_ENDIAN > > > > > + out_le32(addr, value); > > > > > +#else > > > > > + out_be32(addr, value); > > > > > +#endif > > > > > +#endif > > > > > #elif defined(CONFIG_SYS_BIG_ENDIAN) > > > > > writeb(value, addr + (1 << shift) - 1); > > > > > #else > > > > > @@ -81,12 +85,16 @@ static inline int serial_in_shift(void > > > > > *addr, > > > > > int > > > > > shift) > > > > > { > > > > > #ifdef CONFIG_SYS_NS16550_PORT_MAPPED > > > > > return inb((ulong)addr); > > > > > -#elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > > !defined(CONFIG_SYS_BIG_ENDIAN) > > > > > - return in_le32(addr); > > > > > -#elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > > defined(CONFIG_SYS_BIG_ENDIAN) > > > > > - return in_be32(addr); > > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) > > > > > +#ifdef CONFIG_MIPS > > > > > return readl(addr); > > > > > +#else > > > > > +#ifndef CONFIG_SYS_BIG_ENDIAN > > > > > + return in_le32(addr); > > > > > +#else > > > > > + return in_be32(addr); > > > > > +#endif > > > > > +#endif > > > > > #elif defined(CONFIG_SYS_BIG_ENDIAN) > > > > > return readb(addr + (1 << shift) - 1); > > > > > #else > > > > Could you tell us, if you need port IO or MMIO. In case of MMIO > > > > do > > > > you > > > > need 8 bit or 32 bit I/O access? > > > > > > > > Becasue your CPU is running in Big Endian and the NS16550 is > > > > Little > > > > Endian, you need endianess conversion in the 32 bit case. > > > > > > > > The 8 bit access should already work without changing anything. > > > > The > > > > MIPS Malta board also uses NS16550 and already works in Bit > > > > Endian > > > > mode. > > > > > > > > To make the 32 bit case working, you need to select > > > > CONFIG_SWAP_IO_SPACE in your SoC Kconfig code. Then all > > > > readl/writel > > > > accessors do an endianess conversion to Little Endian if the > > > > CPU is > > > > running in Big Endian. > > > > > > > > Anyway I think the current implementation is wrong: > > > > > > > > static inline void serial_out_shift(void *addr, int shift, int > > > > value) > > > > { > > > > #ifdef CONFIG_SYS_NS16550_PORT_MAPPED > > > > outb(value, (ulong)addr); > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > !defined(CONFIG_SYS_BIG_ENDIAN) > > > > out_le32(addr, value); > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) && > > > > defined(CONFIG_SYS_BIG_ENDIAN) > > > > out_be32(addr, value); > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) > > > > writel(value, addr); > > > > #elif defined(CONFIG_SYS_BIG_ENDIAN) > > > > writeb(value, addr + (1 << shift) - 1); > > > > #else > > > > writeb(value, addr); > > > > #endif > > > > } > > > > > > > > The branch with writel() will never be taken because the two > > > > preceding > > > > branches already catch all conditions for > > > > CONFIG_SYS_NS16550_MEM32. > > > > Also if the NS16550 is Little Endian, the branch with out_be32 > > > > makes no > > > > sense. All IO accessors must convert from CPU endianess to > > > > NS16550's > > > > Little Endian, but out_be32 converts from CPU endianess to Big > > > > Endian. > > > > > > > > To handle port IO, 32 Bit MMIO and 8 Bit MMIO, following code > > > > should be > > > > enough: > > > > > > > > static inline void serial_out_shift(void *addr, int shift, int > > > > value) > > > > { > > > > #ifdef CONFIG_SYS_NS16550_PORT_MAPPED > > > > outb(value, (ulong)addr); > > > > #elif defined(CONFIG_SYS_NS16550_MEM32) > > > > writel(value, addr); > > > > #elif defined(CONFIG_SYS_BIG_ENDIAN) > > > > writeb(value, addr + (1 << shift) - 1); > > > > #else > > > > writeb(value, addr); > > > > #endif > > > > } > > > > > > > > The arch-specific implementation of readl/writel should be > > > > responsible > > > > for the endianess conversion and not the driver. What do you > > > > think? > > > > > > > I wholly agree with Daniel, driver should not care the > > > endianness, we > > > need a general mechanism to deal with this situation, no matter > > > the > > > endianness of CPU and peripheral IP Core. > > > But CONFIG_SWAP_IO_SPACE don't resolve this issue, NS16550 is > > > just > > > one of many peripherals for CPU. > > > NS16550 use only 8bit register field even if in 32 bit MMIO. In > > > actual, > > > driver may just concern the register offset beause the bit field > > > of > > > register in chip datasheet is coincident with CPU endianness, > > > even > > > though hardware support both big-endian and little-endian, so the > > > optimal code should be the following: > > > static inline void serial_out_shift(void *addr, int shift, int > > > value) > > > { > > > #ifdef CONFIG_SYS_NS16550_PORT_MAPPED > > > outb(value, (ulong)addr); > > > #elif defined(CONFIG_SYS_NS16550_MEM32) > > > writel(value, addr); > > > #else > > > writeb(value, addr); > > > #endif > > > } > > > I use 32bit MMIO to access uart currently, i think that 8 Bit > > > MMIO > > > should work fine if adjust the register offset. > > > > > then try to use 8 bit access and find the correct register offset. > > You > > can configure that offset with the DT property "reg-shift". > > > CONFIG_SYS_BIG_ENDIAN and CONFIG_SYS_LITTLE_ENDIAN are used > just for MIPS in u-boot? no, that is a generic config option. E.g. PowerPC also has BE, most other archs have LE, MIPS supports both variants. -- - Daniel