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: Wrap fbcon updates from vga-switcheroo in helper Date: Mon, 25 May 2026 21:30:31 +1000 Message-ID: In-Reply-To: <20260520160744.130841-4-tzimmermann@suse.de> References: <20260520160744.130841-1-tzimmermann@suse.de> <20260520160744.130841-4-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Bug =E2=80=94 `#if CONFIG_FB`**: The patch adds: ```c +#if CONFIG_FB if (new_client->fb_info) - fbcon_remap_all(new_client->fb_info); + fb_switch_outputs(new_client->fb_info); +#endif ``` This should be `#ifdef CONFIG_FB`. While `#if CONFIG_FB` happens to work fo= r bool Kconfig symbols (undefined macros evaluate to 0 in `#if` expressions= ), it's not standard kernel preprocessor style. The kernel overwhelmingly u= ses `#ifdef CONFIG_*` for presence checks. `#if CONFIG_FB` could also trigg= er compiler warnings with `-Wundef` if enabled. Moreover, looking at the Kconfig: ``` config VGA_SWITCHEROO depends on (FRAMEBUFFER_CONSOLE=3Dn || FB=3Dy) ``` When `VGA_SWITCHEROO` is enabled, either `FB=3Dy` (so CONFIG_FB is always d= efined) or `FRAMEBUFFER_CONSOLE=3Dn` (FB may or may not be set). The guard = is functionally needed for the `FRAMEBUFFER_CONSOLE=3Dn && FB=3Dn` case (th= ough that seems unlikely given the dependency implies you need *some* displ= ay path). **Design**: The `fb_switch_outputs()` function is a thin wrapper around `fb= con_remap_all()`. The commit message explains this is a stepping stone towa= rd moving the call into DRM's fbdev emulation, which justifies the indirect= ion. The function is exported with `EXPORT_SYMBOL` since `vga_switcheroo` i= s outside the fbdev core. --- Generated by Claude Code Patch Reviewer