1. Patchew
  2. linux
  3. View series

[PATCH v2] drm/sysfb: Fix efidrm error handling and memory type mismatch

Chen Ni posted 1 patch 3 weeks, 6 days ago 
drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 15 deletions(-)
[PATCH v2] drm/sysfb: Fix efidrm error handling and memory type mismatch
Posted by Chen Ni 3 weeks, 6 days ago 
Fix incorrect error checking and memory type confusion in
efidrm_device_create(). devm_memremap() returns error pointers, not
NULL, and returns system memory while devm_ioremap() returns I/O memory.
The code incorrectly passes system memory to iosys_map_set_vaddr_iomem().

Restructure to handle each memory type separately. Use devm_ioremap*()
with ERR_PTR(-ENXIO) for WC/UC, and devm_memremap() with ERR_CAST() for
WT/WB.

Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
Changes in v2:
- Split mapping logic per memory type
- Remove is_iomem flag
- Use correct error handling (ERR_PTR/ERR_CAST)
---
 drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 1114359a1e62..b183256cfcd0 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -157,7 +157,6 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	struct drm_sysfb_device *sysfb;
 	struct drm_device *dev;
 	struct resource *mem = NULL;
-	void __iomem *screen_base = NULL;
 	struct drm_plane *primary_plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
@@ -244,21 +243,34 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 
 	mem_flags = efidrm_get_mem_flags(dev, res->start, vsize);
 
-	if (mem_flags & EFI_MEMORY_WC)
-		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
-	else if (mem_flags & EFI_MEMORY_UC)
-		screen_base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
-	else if (mem_flags & EFI_MEMORY_WT)
-		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
-					    MEMREMAP_WT);
-	else if (mem_flags & EFI_MEMORY_WB)
-		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
-					    MEMREMAP_WB);
-	else
+	if (mem_flags & EFI_MEMORY_WC) {
+		void __iomem *screen_base = devm_ioremap_wc(&pdev->dev, mem->start,
+							    resource_size(mem));
+		if (!screen_base)
+			return ERR_PTR(-ENXIO);
+		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+	} else if (mem_flags & EFI_MEMORY_UC) {
+		void __iomem *screen_base = devm_ioremap(&pdev->dev, mem->start,
+							 resource_size(mem));
+		if (!screen_base)
+			return ERR_PTR(-ENXIO);
+		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+	} else if (mem_flags & EFI_MEMORY_WT) {
+		void *screen_base = devm_memremap(&pdev->dev, mem->start,
+						  resource_size(mem), MEMREMAP_WT);
+		if (IS_ERR(screen_base))
+			return ERR_CAST(screen_base);
+		iosys_map_set_vaddr(&sysfb->fb_addr, screen_base);
+	} else if (mem_flags & EFI_MEMORY_WB) {
+		void *screen_base = devm_memremap(&pdev->dev, mem->start,
+						  resource_size(mem), MEMREMAP_WB);
+		if (IS_ERR(screen_base))
+			return ERR_CAST(screen_base);
+		iosys_map_set_vaddr(&sysfb->fb_addr, screen_base);
+	} else {
 		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
-	if (!screen_base)
-		return ERR_PTR(-ENOMEM);
-	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+		return ERR_PTR(-EINVAL);
+	}
 
 	/*
 	 * Modesetting
-- 
2.25.1
Re: [PATCH v2] drm/sysfb: Fix efidrm error handling and memory type mismatch
Posted by Thomas Zimmermann 3 weeks, 6 days ago 
Hi

Am 11.03.26 um 07:46 schrieb Chen Ni:
> Fix incorrect error checking and memory type confusion in
> efidrm_device_create(). devm_memremap() returns error pointers, not
> NULL, and returns system memory while devm_ioremap() returns I/O memory.
> The code incorrectly passes system memory to iosys_map_set_vaddr_iomem().
>
> Restructure to handle each memory type separately. Use devm_ioremap*()
> with ERR_PTR(-ENXIO) for WC/UC, and devm_memremap() with ERR_CAST() for
> WT/WB.
>
> Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks a lot for the fix.

> ---
> Changes in v2:
> - Split mapping logic per memory type
> - Remove is_iomem flag
> - Use correct error handling (ERR_PTR/ERR_CAST)
> ---
>   drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++------------
>   1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
> index 1114359a1e62..b183256cfcd0 100644
> --- a/drivers/gpu/drm/sysfb/efidrm.c
> +++ b/drivers/gpu/drm/sysfb/efidrm.c
> @@ -157,7 +157,6 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   	struct drm_sysfb_device *sysfb;
>   	struct drm_device *dev;
>   	struct resource *mem = NULL;
> -	void __iomem *screen_base = NULL;
>   	struct drm_plane *primary_plane;
>   	struct drm_crtc *crtc;
>   	struct drm_encoder *encoder;
> @@ -244,21 +243,34 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   
>   	mem_flags = efidrm_get_mem_flags(dev, res->start, vsize);
>   
> -	if (mem_flags & EFI_MEMORY_WC)
> -		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> -	else if (mem_flags & EFI_MEMORY_UC)
> -		screen_base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
> -	else if (mem_flags & EFI_MEMORY_WT)
> -		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> -					    MEMREMAP_WT);
> -	else if (mem_flags & EFI_MEMORY_WB)
> -		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> -					    MEMREMAP_WB);
> -	else
> +	if (mem_flags & EFI_MEMORY_WC) {
> +		void __iomem *screen_base = devm_ioremap_wc(&pdev->dev, mem->start,
> +							    resource_size(mem));

IIRC coding style requires that there's an empty line after the 
declarations. I'll fix that when I merge the patch. No need to send 
another revision.

Best regards
Thomas

> +		if (!screen_base)
> +			return ERR_PTR(-ENXIO);
> +		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
> +	} else if (mem_flags & EFI_MEMORY_UC) {
> +		void __iomem *screen_base = devm_ioremap(&pdev->dev, mem->start,
> +							 resource_size(mem));
> +		if (!screen_base)
> +			return ERR_PTR(-ENXIO);
> +		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
> +	} else if (mem_flags & EFI_MEMORY_WT) {
> +		void *screen_base = devm_memremap(&pdev->dev, mem->start,
> +						  resource_size(mem), MEMREMAP_WT);
> +		if (IS_ERR(screen_base))
> +			return ERR_CAST(screen_base);
> +		iosys_map_set_vaddr(&sysfb->fb_addr, screen_base);
> +	} else if (mem_flags & EFI_MEMORY_WB) {
> +		void *screen_base = devm_memremap(&pdev->dev, mem->start,
> +						  resource_size(mem), MEMREMAP_WB);
> +		if (IS_ERR(screen_base))
> +			return ERR_CAST(screen_base);
> +		iosys_map_set_vaddr(&sysfb->fb_addr, screen_base);
> +	} else {
>   		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
> -	if (!screen_base)
> -		return ERR_PTR(-ENOMEM);
> -	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
> +		return ERR_PTR(-EINVAL);
> +	}
>   
>   	/*
>   	 * Modesetting

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.