Hi Anirudh, On 2026-07-19T18:03:31, Anirudh Srinivasan <[email protected]> wrote: > video: Add Aspeed VGA driver > > Add a video driver for the Aspeed VGA device that relys on the vesa > video implementation and VBIOS emulation. The VBIOS requires a custom > INT15 handler, so that is added in this commit. > > This commit isn't usable as is, and depends on the next commit for the > device to actually work. And I'm not sure if the next commit is suitable > for upstream, so I've marked this one as such too.
Typo: 'relies'. Please move the upstreamability commentary below the '---' cut (or drop it once 7/8 are sorted); it should not stay in the permanent commit log. > > Signed-off-by: Anirudh Srinivasan <[email protected]> > > drivers/video/Kconfig | 10 ++++++ > drivers/video/Makefile | 1 + > drivers/video/aspeed_vga.c | 81 > ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 92 insertions(+) > diff --git a/drivers/video/aspeed_vga.c b/drivers/video/aspeed_vga.c > @@ -0,0 +1,81 @@ > +#include <bios_emul.h> > +#include <dm.h> > +#include <log.h> > +#include <pci.h> > +#include <vesa.h> > +#include <video.h> > +#include <linux/delay.h> Please add '#define LOG_CATEGORY UCLASS_VIDEO' at the top (see bochs.c) so log output is categorised correctly. > diff --git a/drivers/video/aspeed_vga.c b/drivers/video/aspeed_vga.c > @@ -0,0 +1,81 @@ > + /* ASPEED exposes a 16 MiB VRAM BAR */ > + uc_plat->size = 16 << 20; Please derive this from the BAR size rather than hard-coding — query BAR0 via dm_pci_read_bar32() at bind time, or at minimum use a #define (e.g. ASPEED_VRAM_SIZE). SZ_16M from <linux/sizes.h> also reads better. > diff --git a/drivers/video/aspeed_vga.c b/drivers/video/aspeed_vga.c > @@ -0,0 +1,81 @@ > +static const struct video_ops aspeed_vga_ops = { > +}; You can drop the empty video_ops and the .ops assignment if you like, as it's optional at present. Regards, Simon
