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
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
