Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng <hidden>
Date: 2017-05-17 06:09:56
Also in:
linux-arm-kernel, lkml
From: Dong Aisheng <hidden>
Date: 2017-05-17 06:09:56
Also in:
linux-arm-kernel, lkml
On Wed, May 17, 2017 at 08:50:39AM +0300, Nikita Yushchenko wrote:
quoted
quoted
quoted
quoted
quoted
static u32 lpuart32_read(void __iomem *addr) { - return ioread32be(addr); + return lpuart_is_be ? ioread32be(addr) : readl(addr); } static void lpuart32_write(u32 val, void __iomem *addr) { - iowrite32be(val, addr); + if (lpuart_is_be) + iowrite32be(val, addr); + else + writel(val, addr); }What if this is ever executed on big endian system?Sorry, not catching the point... What issues will meet?Isn't writel() in host endian?On big endian systems, it is supposed to run iowrite32be.Your code states, "force BE if lpuart_is_be, don't care otherwise". This semantics looks questionable for code reviewer. If driver handles endian, should't it be explicit in both cases? And if indeed driver means handling BE explicitly, but don't caring otherwise, maybe variable name should suggest that (i.e. "force_be")?
For lpuart32, only two cases that LS platforms is Big endian while IMX is little endian. It's SoC IP native property, i don't think force_be is better. Regards Dong Aisheng
Although driver maintainer could think differently. I won't insist on this. Nikita