From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: fbdev: goldfishfb: Request memory region Date: Thu, 23 Apr 2026 09:34:41 +1000 Message-ID: In-Reply-To: <20260420134424.77494-4-amit.barzilai22@gmail.com> References: <20260420134424.77494-1-amit.barzilai22@gmail.com> <20260420134424.77494-4-amit.barzilai22@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Status: Good, with one observation** Converts from bare `ioremap()` to `devm_ioremap_resource()`, and correctly = removes the manual `iounmap()` calls from both the error unwind path and `g= oldfish_fb_remove()`. ```c fb->reg_base =3D devm_ioremap_resource(&pdev->dev, r); if (IS_ERR(fb->reg_base)) { ret =3D PTR_ERR(fb->reg_base); goto err_no_io_base; } ``` **Observation on mapping size:** The original code maps exactly `PAGE_SIZE`= bytes: ```c fb->reg_base =3D ioremap(r->start, PAGE_SIZE); ``` The new `devm_ioremap_resource()` maps `resource_size(r)` bytes. The actual= register space only extends to offset `0x20` (36 bytes), so both PAGE_SIZE= and the platform-defined resource size are more than sufficient. Mapping e= xactly what the resource describes is the right thing to do, so this is a c= orrect change =E2=80=94 but it's worth the reviewer being aware that the ma= pped size is now determined by the platform resource rather than hardcoded. **Cleanup correctness:** The `iounmap()` removal is safe. The `kfree(fb)` c= alls in both the error path and `goldfish_fb_remove()` are retained, which = is correct =E2=80=94 freeing the struct that holds the `reg_base` pointer d= oesn't conflict with the devm-managed mapping lifetime (the mapping is tied= to the device, not the struct). No issues. --- Generated by Claude Code Patch Reviewer