Re: [PATCH] exynos: serial clear and set big endian fix

[Arnd Bergmann <arnd@arndb.de>]

On Monday 21 July 2014 16:50:32 Mark Brown wrote:
> +#ifndef CONFIG_CPU_BIG_ENDIAN /* little endian */
> +static inline void __hw_set_bit(int nr, volatile unsigned long *addr)
> +{
> +       __set_bit(nr, addr);
> +}
> +
> +static inline void __hw_clear_bit(int nr, volatile unsigned long *addr)
> +{
> +       __clear_bit(nr, addr);
> +}
> +#else
> +static inline void __hw_set_bit(int nr, volatile unsigned long *addr)
> +{
> +       unsigned long mask = BIT_MASK(nr);
> +       unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> +       unsigned long val = le32_to_cpu(*p);
> +
> +       val  |= mask;
> +
> +       *p = cpu_to_le32(val);
> +}

This is still wrong, because there is an associated bug: you must never
access pointers to MMIO registers from C code.

The correct way to do it is to use the readl()/writel() functions, or
readl_relaxed()/writel_relaxed() in case of drivers that don't need
to synchronize with DMA transfers.

I think what you want is something like

static inline void __hw_set_bit(int nr, unsigned long __iomem *addr)
{
       addr += BIT_WORD(nr);
       writel_relaxed(readl_relaxed(addr) | BIT_MASK(nr), addr);
}

which is also endian-safe.

	Arnd