Re: [PATCH 05/18] asm-generic: split off typeconfused readl and friends

[Sascha Hauer <sha@pengutronix.de>]

On Fri, Nov 10, 2023 at 10:44:08PM +0100, Ahmad Fatoum wrote:
> The MMIO accessors in barebox like readl have the error prone
> peculiarity of accepting integer arguments to pointers automatically,
> which makes it easy to confuse address and data.
> 
> We will add an alternative less error-probe way for new code, so first

s/error-probe/error-prone/

> split off the typeconfused code into a separate file.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/asm-generic/io-typeconfused.h | 75 +++++++++++++++++++++++++++
>  include/asm-generic/io.h              | 57 +-------------------
>  2 files changed, 76 insertions(+), 56 deletions(-)
>  create mode 100644 include/asm-generic/io-typeconfused.h
> 
> diff --git a/include/asm-generic/io-typeconfused.h b/include/asm-generic/io-typeconfused.h
> new file mode 100644
> index 000000000000..d25ed7db2473
> --- /dev/null
> +++ b/include/asm-generic/io-typeconfused.h
> @@ -0,0 +1,75 @@
> +/* Generic I/O port emulation, based on MN10300 code
> + *
> + * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +#ifndef __ASM_GENERIC_IO_TYPECONFUSED_H
> +#define __ASM_GENERIC_IO_TYPECONFUSED_H
> +
> +#include <linux/string.h> /* for memset() and memcpy() */
> +#include <linux/compiler.h>
> +#include <linux/types.h>
> +#include <asm/byteorder.h>
> +
> +/*****************************************************************************/
> +/*
> + * Unlike the definitions in <asm-generic/io.h>, these macros don't complain
> + * about integer arguments and just silently cast them to pointers. This is
> + * a common cause of bugs, but lots of existing code depends on this, so
> + * this header is provided as a transitory measure.
> + */
> +
> +#ifndef __raw_readb
> +#define __raw_readb(a)		(__chk_io_ptr(a), *(volatile unsigned char __force  *)(a))
> +#endif
> +
> +#ifndef __raw_readw
> +#define __raw_readw(a)		(__chk_io_ptr(a), *(volatile unsigned short __force *)(a))
> +#endif
> +
> +#ifndef __raw_readl
> +#define __raw_readl(a)		(__chk_io_ptr(a), *(volatile unsigned int __force   *)(a))
> +#endif
> +
> +#ifndef readb
> +#define readb __raw_readb
> +#endif
> +
> +#ifndef readw
> +#define readw(addr) __le16_to_cpu(__raw_readw(addr))
> +#endif
> +
> +#ifndef readl
> +#define readl(addr) __le32_to_cpu(__raw_readl(addr))
> +#endif
> +
> +#ifndef __raw_writeb
> +#define __raw_writeb(v,a)	(__chk_io_ptr(a), *(volatile unsigned char __force  *)(a) = (v))
> +#endif
> +
> +#ifndef __raw_writew
> +#define __raw_writew(v,a)	(__chk_io_ptr(a), *(volatile unsigned short __force *)(a) = (v))
> +#endif
> +
> +#ifndef __raw_writel
> +#define __raw_writel(v,a)	(__chk_io_ptr(a), *(volatile unsigned int __force   *)(a) = (v))
> +#endif
> +
> +#ifndef writeb
> +#define writeb __raw_writeb
> +#endif
> +
> +#ifndef writew
> +#define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr)
> +#endif
> +
> +#ifndef writel
> +#define writel(b,addr) __raw_writel(__cpu_to_le32(b),addr)
> +#endif
> +
> +#endif /* __ASM_GENERIC_IO_TYPECONFUSED_H */
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index ab439026928a..6383d71746f1 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -15,62 +15,7 @@
>  #include <linux/compiler.h>
>  #include <linux/types.h>
>  #include <asm/byteorder.h>
> -
> -/*****************************************************************************/
> -/*
> - * readX/writeX() are used to access memory mapped devices. On some
> - * architectures the memory mapped IO stuff needs to be accessed
> - * differently. On the simple architectures, we just read/write the
> - * memory location directly.
> - */
> -
> -#ifndef __raw_readb
> -#define __raw_readb(a)		(__chk_io_ptr(a), *(volatile unsigned char __force  *)(a))
> -#endif
> -
> -#ifndef __raw_readw
> -#define __raw_readw(a)		(__chk_io_ptr(a), *(volatile unsigned short __force *)(a))
> -#endif
> -
> -#ifndef __raw_readl
> -#define __raw_readl(a)		(__chk_io_ptr(a), *(volatile unsigned int __force   *)(a))
> -#endif
> -
> -#ifndef readb
> -#define readb __raw_readb
> -#endif
> -
> -#ifndef readw
> -#define readw(addr) __le16_to_cpu(__raw_readw(addr))
> -#endif
> -
> -#ifndef readl
> -#define readl(addr) __le32_to_cpu(__raw_readl(addr))
> -#endif
> -
> -#ifndef __raw_writeb
> -#define __raw_writeb(v,a)	(__chk_io_ptr(a), *(volatile unsigned char __force  *)(a) = (v))
> -#endif
> -
> -#ifndef __raw_writew
> -#define __raw_writew(v,a)	(__chk_io_ptr(a), *(volatile unsigned short __force *)(a) = (v))
> -#endif
> -
> -#ifndef __raw_writel
> -#define __raw_writel(v,a)	(__chk_io_ptr(a), *(volatile unsigned int __force   *)(a) = (v))
> -#endif
> -
> -#ifndef writeb
> -#define writeb __raw_writeb
> -#endif
> -
> -#ifndef writew
> -#define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr)
> -#endif
> -
> -#ifndef writel
> -#define writel(b,addr) __raw_writel(__cpu_to_le32(b),addr)
> -#endif
> +#include <asm-generic/io-typeconfused.h>
>  
>  #ifdef CONFIG_64BIT
>  static inline u64 __raw_readq(const volatile void __iomem *addr)
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |