Claude review: drm/sysfb: efidrm: Fix framebuffer mapping error handling

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Type mismatch on error return path (minor issue):**

```c
	} else if (mem_flags & EFI_MEMORY_WT) {
		sys_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
					 MEMREMAP_WT);
		is_iomem = false;
		if (IS_ERR(sys_base))
			return sys_base;
```

The function returns `struct efidrm_device *`, but `sys_base` is `void *`. While `ERR_PTR` values will work in practice (they're just error-encoding pointers), the correct pattern would be `return ERR_CAST(sys_base)` to avoid an implicit pointer type conversion. This applies to both the WT and WB cases (lines 264 and 270 in the patched file). The compiler may produce a warning about this with strict settings.

**Error handling improvement in the `else` branch — good:**

The original code would print an error for invalid `mem_flags` but then fall through to `if (!screen_base) return ERR_PTR(-ENOMEM)`, which would return the wrong error code (`-ENOMEM` instead of something more appropriate). The patch now correctly returns `-EINVAL` immediately:

```c
	} else {
		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
		return ERR_PTR(-EINVAL);
	}
```

**Correctness of `iosys_map` usage — good:**

The separation into `screen_base` (for ioremap paths) and `sys_base` (for memremap paths) with the corresponding `iosys_map_set_vaddr_iomem()` vs `iosys_map_set_vaddr()` is the right approach. The `iosys_map` abstraction specifically tracks whether the mapped region is I/O memory or system memory, and the original code was setting everything as I/O memory incorrectly.

**Summary:** The patch fixes real bugs and the approach is sound. The only nit is using `return sys_base` instead of `return ERR_CAST(sys_base)` for the type-mismatched error returns.

---
Generated by Claude Code Patch Reviewer