[PATCH 1/9] iosys-map: add new accessor interfaces and use them internally.

[Lucas De Marchi]

On Thu, May 22, 2025 at 01:58:54PM +0200, Thomas Zimmermann wrote:
>
>
>Am 22.05.25 um 08:52 schrieb Dave Airlie:
>>From: Dave Airlie <airlied at redhat.com>
>>
>>This adds accessors inlines to the iosys-map. The intent is to
>>roll the iomem flag into the lower bits of the vaddr eventually.
>>
>>First just add accessors to move all current in-tree users over to.
>>
>>Signed-off-by: Dave Airlie <airlied at redhat.com>
>>---
>>  include/linux/iosys-map.h | 53 +++++++++++++++++++++++++--------------
>>  1 file changed, 34 insertions(+), 19 deletions(-)
>>
>>diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>>index 4696abfd311c..5ce5df1db60a 100644
>>--- a/include/linux/iosys-map.h
>>+++ b/include/linux/iosys-map.h
>>@@ -114,6 +114,21 @@ struct iosys_map {
>>  	bool is_iomem;
>>  };
>>+static inline bool iosys_map_is_iomem(const struct iosys_map *map)
>>+{
>>+	return map->is_iomem;
>>+}
>>+
>>+static inline void __iomem *iosys_map_ioptr(const struct iosys_map *map)
>>+{
>>+	return map->vaddr_iomem;
>>+}
>>+
>>+static inline void *iosys_map_ptr(const struct iosys_map *map)
>>+{
>>+       return map->vaddr;
>>+}
>>+
>
>These helpers need documentation.

agreed

>
>We should encourage users to the other helpers for interacting with 
>iosys-map structures instead of decoding them manually. OTOH there are 
>cases where decoding them by hand is clearly better. I'd suggest to 
>prefix the new helpers with __ so mark them an internal/special.

 From the other patches there are quite a few cases that would be using
"internal"  API. From those there are just a few cases in which we'd
have a direct translation to existing API... so I wouldn't make this
internal when they are clearly needed externally.

Lucas De Marchi

>
>Best regards
>Thomas
>
>>  /**
>>   * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an address in system memory
>>   * @vaddr_:	A system-memory address
>>@@ -234,9 +249,9 @@ static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
>>   */
>>  static inline bool iosys_map_is_null(const struct iosys_map *map)
>>  {
>>-	if (map->is_iomem)
>>-		return !map->vaddr_iomem;
>>-	return !map->vaddr;
>>+	if (iosys_map_is_iomem(map))
>>+		return !iosys_map_ioptr(map);
>>+	return !iosys_map_ptr(map);
>>  }
>>  /**
>>@@ -286,10 +301,10 @@ static inline void iosys_map_clear(struct iosys_map *map)
>>  static inline void iosys_map_memcpy_to(struct iosys_map *dst, size_t dst_offset,
>>  				       const void *src, size_t len)
>>  {
>>-	if (dst->is_iomem)
>>-		memcpy_toio(dst->vaddr_iomem + dst_offset, src, len);
>>+	if (iosys_map_is_iomem(dst))
>>+		memcpy_toio(iosys_map_ioptr(dst) + dst_offset, src, len);
>>  	else
>>-		memcpy(dst->vaddr + dst_offset, src, len);
>>+		memcpy(iosys_map_ptr(dst) + dst_offset, src, len);
>>  }
>>  /**
>>@@ -306,10 +321,10 @@ static inline void iosys_map_memcpy_to(struct iosys_map *dst, size_t dst_offset,
>>  static inline void iosys_map_memcpy_from(void *dst, const struct iosys_map *src,
>>  					 size_t src_offset, size_t len)
>>  {
>>-	if (src->is_iomem)
>>-		memcpy_fromio(dst, src->vaddr_iomem + src_offset, len);
>>+	if (iosys_map_is_iomem(src))
>>+		memcpy_fromio(dst, iosys_map_ioptr(src) + src_offset, len);
>>  	else
>>-		memcpy(dst, src->vaddr + src_offset, len);
>>+		memcpy(dst, iosys_map_ptr(src) + src_offset, len);
>>  }
>>  /**
>>@@ -322,7 +337,7 @@ static inline void iosys_map_memcpy_from(void *dst, const struct iosys_map *src,
>>   */
>>  static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
>>  {
>>-	if (map->is_iomem)
>>+	if (iosys_map_is_iomem(map))
>>  		map->vaddr_iomem += incr;
>>  	else
>>  		map->vaddr += incr;
>>@@ -341,10 +356,10 @@ static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
>>  static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,
>>  				    int value, size_t len)
>>  {
>>-	if (dst->is_iomem)
>>-		memset_io(dst->vaddr_iomem + offset, value, len);
>>+	if (iosys_map_is_iomem(dst))
>>+		memset_io(iosys_map_ioptr(dst) + offset, value, len);
>>  	else
>>-		memset(dst->vaddr + offset, value, len);
>>+		memset(iosys_map_ptr(dst) + offset, value, len);
>>  }
>>  #ifdef CONFIG_64BIT
>>@@ -393,10 +408,10 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,
>>   */
>>  #define iosys_map_rd(map__, offset__, type__) ({					\
>>  	type__ val_;									\
>>-	if ((map__)->is_iomem) {							\
>>-		__iosys_map_rd_io(val_, (map__)->vaddr_iomem + (offset__), type__);	\
>>+	if (iosys_map_is_iomem(map__)) {						\
>>+		__iosys_map_rd_io(val_, iosys_map_ioptr(map__) + (offset__), type__);	\
>>  	} else {									\
>>-		__iosys_map_rd_sys(val_, (map__)->vaddr + (offset__), type__);		\
>>+		__iosys_map_rd_sys(val_, iosys_map_ptr(map__) + (offset__), type__);	\
>>  	}										\
>>  	val_;										\
>>  })
>>@@ -415,10 +430,10 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,
>>   */
>>  #define iosys_map_wr(map__, offset__, type__, val__) ({					\
>>  	type__ val_ = (val__);								\
>>-	if ((map__)->is_iomem) {							\
>>-		__iosys_map_wr_io(val_, (map__)->vaddr_iomem + (offset__), type__);	\
>>+	if (iosys_map_is_iomem(map__)) {						\
>>+		__iosys_map_wr_io(val_, iosys_map_ioptr(map__) + (offset__), type__);	\
>>  	} else {									\
>>-		__iosys_map_wr_sys(val_, (map__)->vaddr + (offset__), type__);		\
>>+		__iosys_map_wr_sys(val_, iosys_map_ptr(map__) + (offset__), type__);	\
>>  	}										\
>>  })
>
>-- 
>--
>Thomas Zimmermann
>Graphics Driver Developer
>SUSE Software Solutions Germany GmbH
>Frankenstrasse 146, 90461 Nuernberg, Germany
>GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>HRB 36809 (AG Nuernberg)
>