2024-05-14 13:09:46

by Niklas Schnelle

[permalink] [raw]
Subject: [PATCH v2 0/1] video: Handle HAS_IOPORT dependencies

Hi,

This is a follow up in my ongoing effort of making inb()/outb() and
similar I/O port accessors compile-time optional. Previously I sent this
as a treewide series titled "treewide: Remove I/O port accessors for
HAS_IOPORT=n" with the latest being its 5th version[0]. With a significant
subset of patches merged I've changed over to per-subsystem series. These
series are stand alone and should be merged via the relevant tree such
that with all subsystems complete we can follow this up with the final
patch that will make the I/O port accessors compile-time optional.

The current state of the full series with changes to the remaining subsystems
and the aforementioned final patch can be found for your convenience on my
git.kernel.org tree in the has_ioport branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Changes since v1:
- Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
to use them in with or without I/O port variants.
- Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
to get rid of in-code #ifdef (Arnd)
- Got rid of if (regbase) logic inversion needed for in-code #ifdef

Thanks,
Niklas

[0] https://lore.kernel.org/all/[email protected]/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/

Niklas Schnelle (1):
video: Handle HAS_IOPORT dependencies

include/video/vga.h | 58 ++++++++++++++++++++++++++++++++-------------
1 file changed, 42 insertions(+), 16 deletions(-)

--
2.40.1



2024-05-14 13:09:54

by Niklas Schnelle

[permalink] [raw]
Subject: [PATCH v2 1/1] video: Handle HAS_IOPORT dependencies

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.

Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
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.

v1 -> v2:
- Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
to use them in with or without I/O port variants.
- Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
to get rid of in-code #ifdef (Arnd)
- Got rid of if (regbase) logic inversion needed for in-code #ifdef

include/video/vga.h | 58 ++++++++++++++++++++++++++++++++-------------
1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..468764d6727a 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -197,9 +197,26 @@ struct vgastate {
extern int save_vga(struct vgastate *state);
extern int restore_vga(struct vgastate *state);

+static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short port)
+{
+ return readb (regbase + port);
+}
+
+static inline void vga_mm_w (void __iomem *regbase, unsigned short port, unsigned char val)
+{
+ writeb (val, regbase + port);
+}
+
+static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ writew (VGA_OUT16VAL (val, reg), regbase + port);
+}
+
/*
* generic VGA port read/write
*/
+#ifdef CONFIG_HAS_IOPORT

static inline unsigned char vga_io_r (unsigned short port)
{
@@ -217,22 +234,6 @@ static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
outw(VGA_OUT16VAL (val, reg), port);
}

-static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short port)
-{
- return readb (regbase + port);
-}
-
-static inline void vga_mm_w (void __iomem *regbase, unsigned short port, unsigned char val)
-{
- writeb (val, regbase + port);
-}
-
-static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
- unsigned char reg, unsigned char val)
-{
- writew (VGA_OUT16VAL (val, reg), regbase + port);
-}
-
static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
{
if (regbase)
@@ -258,7 +259,24 @@ static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
else
vga_io_w_fast (port, reg, val);
}
+#else /* CONFIG_HAS_IOPORT */
+static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
+{
+ return vga_mm_r (regbase, port);
+}

+static inline void vga_w (void __iomem *regbase, unsigned short port, unsigned char val)
+{
+ vga_mm_w (regbase, port, val);
+}
+
+
+static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ vga_mm_w_fast (regbase, port, reg, val);
+}
+#endif /* CONFIG_HAS_IOPORT */

/*
* VGA CRTC register read/write
@@ -280,6 +298,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 +314,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 +353,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 +369,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 +407,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 +423,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 +458,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 +470,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


2024-05-15 07:48:10

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] video: Handle HAS_IOPORT dependencies

On Tue, May 14, 2024, at 13:08, Niklas Schnelle wrote:
> 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.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> 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.
>
> v1 -> v2:
> - Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
> to use them in with or without I/O port variants.
> - Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
> to get rid of in-code #ifdef (Arnd)
> - Got rid of if (regbase) logic inversion needed for in-code #ifdef

Thanks for preparing the new version!

> diff --git a/include/video/vga.h b/include/video/vga.h
> index 947c0abd04ef..468764d6727a 100644
> --- a/include/video/vga.h
> +++ b/include/video/vga.h
> @@ -197,9 +197,26 @@ struct vgastate {
> extern int save_vga(struct vgastate *state);
> extern int restore_vga(struct vgastate *state);
>
> +static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned
> short port)
> +{
> + return readb (regbase + port);
> +}

My first thought was that this should use the normal whitespace,
but I guess the file is pretty consistent about the style here,
so I agree with your choice here.

Arnd