Re: [XEN PATCH v3] drivers/video: make declarations of defined functions available

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 05/09/2023 16:42, Jan Beulich wrote:
> On 01.09.2023 09:02, Nicola Vetrini wrote:
> > The declarations for 'vesa_{init,early_init,endboot}' needed by
> > 'xen/drivers/video/vesa.c' and 'fill_console_start_info' in 'vga.c'
> > are now available by moving the relative code inside 'vga.h'.
> >
> > While moving the code, the alternative definitions are now guarded by
> > CONFIG_VGA, since it implies all previously used constants.
> But such an implication doesn't mean said adjustment is correct. 
> Easiest
> would likely be to simply drop that half sentence.
Maybe I didn't phrase that correctly, I'm ok with just dropping the last 
part
(it can be traced to the Kconfig anyway).

> > --- a/xen/include/xen/vga.h
> > +++ b/xen/include/xen/vga.h
> > @@ -13,6 +13,14 @@
> >
> >  #ifdef CONFIG_VGA
> >  extern struct xen_vga_console_info vga_console_info;
> > +int fill_console_start_info(struct dom0_vga_console_info *);
> > +void vesa_init(void);
> > +void vesa_early_init(void);
> > +void vesa_endboot(bool_t keep);
> Nit: Just "bool" please.
Right, I missed it.

> > +#else
> > +#define vesa_early_init() ((void)0)
> > +#define vesa_endboot(x)   ((void)0)
> > +static inline void vesa_init(void) {};
> So why two #define-s and one inline function? Just because it was that
> way originally doesn't mean it needs to stay that way. Then again are
> the two #define-s actually needed at all? It looks like they were added
> to vga.c just out of precaution, covering the "VGA but no VESA" case.
> Now that things are properly moved to headers (and keyed to CONFIG_VGA)
> I think we'd better omit those. They can be introduced again when we
> gain a VGA/VESA split (and a CONFIG_VESA).
Ok on uniforming them to inline functions.
I don't have an opinion on the second matter. If you're otherwise okay 
with the patch
and no one objects I can drop them.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)