Re: [PATCH 1/1] video: Handle HAS_IOPORT dependencies
From: Helge Deller
Date: Thu Apr 11 2024 - 10:01:27 EST
* Niklas Schnelle <schnelle@xxxxxxxxxxxxx>:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to #ifdef functions and their callsites which
> unconditionally use these I/O accessors. In the include/video/vga.h
> these are conveniently all those functions with the vga_io_* prefix.
Why don't you code it like in the patch below?
inb_p(), outb_p() and outw() would then need to be defined externally
without an implementation so that they would generate link time errors
(instead of compile time errors).
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..32c915e109fa 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -203,18 +203,20 @@ extern int restore_vga(struct vgastate *state);
static inline unsigned char vga_io_r (unsigned short port)
{
- return inb_p(port);
+ return IS_ENABLED(CONFIG_HAS_IOPORT) ? inb_p(port) : 0;
}
static inline void vga_io_w (unsigned short port, unsigned char val)
{
- outb_p(val, port);
+ if (IS_ENABLED(CONFIG_HAS_IOPORT))
+ outb_p(val, port);
}
static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
unsigned char val)
{
- outw(VGA_OUT16VAL (val, reg), port);
+ if (IS_ENABLED(CONFIG_HAS_IOPORT))
+ outw(VGA_OUT16VAL (val, reg), port);
}
static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short port)
>
> Co-developed-by: Arnd Bergmann <arnd@xxxxxxxxxx>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxxxx>
> Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> ---
> Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
> and may be merged via subsystem specific trees at your earliest
> convenience.
>
> include/video/vga.h | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/include/video/vga.h b/include/video/vga.h
> index 947c0abd04ef..ed89295941c4 100644
> --- a/include/video/vga.h
> +++ b/include/video/vga.h
> @@ -201,6 +201,7 @@ extern int restore_vga(struct vgastate *state);
> * generic VGA port read/write
> */
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_r (unsigned short port)
> {
> return inb_p(port);
> @@ -210,12 +211,12 @@ static inline void vga_io_w (unsigned short port, unsigned char val)
> {
> outb_p(val, port);
> }
> -
> static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
> unsigned char val)
> {
> outw(VGA_OUT16VAL (val, reg), port);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short port)
> {
> @@ -235,28 +236,34 @@ static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
>
> static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
> {
> - if (regbase)
> - return vga_mm_r (regbase, port);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> return vga_io_r (port);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + return vga_mm_r (regbase, port);
> }
>
> static inline void vga_w (void __iomem *regbase, unsigned short port, unsigned char val)
> {
> - if (regbase)
> - vga_mm_w (regbase, port, val);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> vga_io_w (port, val);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + vga_mm_w (regbase, port, val);
> }
>
>
> static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
> unsigned char reg, unsigned char val)
> {
> - if (regbase)
> - vga_mm_w_fast (regbase, port, reg, val);
> - else
> +#ifdef CONFIG_HAS_IOPORT
> + if (!regbase)
> vga_io_w_fast (port, reg, val);
> + else
> +#endif /* CONFIG_HAS_IOPORT */
> + vga_mm_w_fast (regbase, port, reg, val);
> }
>
>
> @@ -280,6 +287,7 @@ static inline void vga_wcrt (void __iomem *regbase, unsigned char reg, unsigned
> #endif /* VGA_OUTW_WRITE */
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rcrt (unsigned char reg)
> {
> vga_io_w (VGA_CRT_IC, reg);
> @@ -295,6 +303,7 @@ static inline void vga_io_wcrt (unsigned char reg, unsigned char val)
> vga_io_w (VGA_CRT_DC, val);
> #endif /* VGA_OUTW_WRITE */
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_rcrt (void __iomem *regbase, unsigned char reg)
> {
> @@ -333,6 +342,7 @@ static inline void vga_wseq (void __iomem *regbase, unsigned char reg, unsigned
> #endif /* VGA_OUTW_WRITE */
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rseq (unsigned char reg)
> {
> vga_io_w (VGA_SEQ_I, reg);
> @@ -348,6 +358,7 @@ static inline void vga_io_wseq (unsigned char reg, unsigned char val)
> vga_io_w (VGA_SEQ_D, val);
> #endif /* VGA_OUTW_WRITE */
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_rseq (void __iomem *regbase, unsigned char reg)
> {
> @@ -385,6 +396,7 @@ static inline void vga_wgfx (void __iomem *regbase, unsigned char reg, unsigned
> #endif /* VGA_OUTW_WRITE */
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rgfx (unsigned char reg)
> {
> vga_io_w (VGA_GFX_I, reg);
> @@ -400,6 +412,7 @@ static inline void vga_io_wgfx (unsigned char reg, unsigned char val)
> vga_io_w (VGA_GFX_D, val);
> #endif /* VGA_OUTW_WRITE */
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_rgfx (void __iomem *regbase, unsigned char reg)
> {
> @@ -434,6 +447,7 @@ static inline void vga_wattr (void __iomem *regbase, unsigned char reg, unsigned
> vga_w (regbase, VGA_ATT_W, val);
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static inline unsigned char vga_io_rattr (unsigned char reg)
> {
> vga_io_w (VGA_ATT_IW, reg);
> @@ -445,6 +459,7 @@ static inline void vga_io_wattr (unsigned char reg, unsigned char val)
> vga_io_w (VGA_ATT_IW, reg);
> vga_io_w (VGA_ATT_W, val);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static inline unsigned char vga_mm_rattr (void __iomem *regbase, unsigned char reg)
> {
> --
> 2.40.1
>