[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <a25086c4-e2fc-4ffc-bc20-afa50e560d96@app.fastmail.com>
Date: Mon, 21 Oct 2024 10:08:19 +0000
From: "Arnd Bergmann" <arnd@...db.de>
To: "Thomas Zimmermann" <tzimmermann@...e.de>,
"Niklas Schnelle" <schnelle@...ux.ibm.com>, "Brian Cain" <bcain@...cinc.com>,
"Marcel Holtmann" <marcel@...tmann.org>,
"Luiz Augusto von Dentz" <luiz.dentz@...il.com>,
"Patrik Jakobsson" <patrik.r.jakobsson@...il.com>,
"Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>,
"Maxime Ripard" <mripard@...nel.org>, "Dave Airlie" <airlied@...il.com>,
"Simona Vetter" <simona@...ll.ch>, "Dave Airlie" <airlied@...hat.com>,
"Gerd Hoffmann" <kraxel@...hat.com>,
"Lucas De Marchi" <lucas.demarchi@...el.com>,
Thomas Hellström <thomas.hellstrom@...ux.intel.com>,
"Rodrigo Vivi" <rodrigo.vivi@...el.com>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
"Jiri Slaby" <jirislaby@...nel.org>, "Maciej W. Rozycki" <macro@...am.me.uk>,
"Heiko Carstens" <hca@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, linux-hexagon@...r.kernel.org,
linux-bluetooth@...r.kernel.org, dri-devel@...ts.freedesktop.org,
virtualization@...ts.linux.dev, spice-devel@...ts.freedesktop.org,
intel-xe@...ts.freedesktop.org, linux-serial@...r.kernel.org,
Linux-Arch <linux-arch@...r.kernel.org>, "Arnd Bergmann" <arnd@...nel.org>
Subject: Re: [PATCH v8 3/5] drm: handle HAS_IOPORT dependencies
On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote:
> Am 08.10.24 um 14:39 schrieb Niklas Schnelle:
d 100644
>> --- a/drivers/gpu/drm/qxl/Kconfig
>> +++ b/drivers/gpu/drm/qxl/Kconfig
>> @@ -2,6 +2,7 @@
>> config DRM_QXL
>> tristate "QXL virtual GPU"
>> depends on DRM && PCI && MMU
>> + depends on HAS_IOPORT
>
> Is there a difference between this style (multiple 'depends on') and the
> one used for gma500 (&& && &&)?
No, it's the same. Doing it in one line is mainly useful
if you have some '||' as well.
>> @@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
>>
>> writeb(val, bochs->mmio + offset);
>> } else {
>> +#ifdef CONFIG_HAS_IOPORT
>> outb(val, ioport);
>> +#endif
>
> Could you provide empty defines for the out() interfaces at the top of
> the file?
That no longer works since there are now __compiletime_error()
versions of these funcitons. However we can do it more nicely like:
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 9b337f948434..034af6e32200 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
return;
- if (bochs->mmio) {
+ if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
int offset = ioport - 0x3c0 + 0x400;
writeb(val, bochs->mmio + offset);
} else {
-#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
-#endif
}
}
@@ -128,16 +126,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport)
if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
return 0xff;
- if (bochs->mmio) {
+ if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
int offset = ioport - 0x3c0 + 0x400;
return readb(bochs->mmio + offset);
} else {
-#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
-#else
- return 0xff;
-#endif
}
}
@@ -145,32 +139,26 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg)
{
u16 ret = 0;
- if (bochs->mmio) {
+ if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
int offset = 0x500 + (reg << 1);
ret = readw(bochs->mmio + offset);
} else {
-#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
-#else
- ret = 0xffff;
-#endif
}
return ret;
}
static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val)
{
- if (bochs->mmio) {
+ if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
int offset = 0x500 + (reg << 1);
writew(val, bochs->mmio + offset);
} else {
-#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
-#endif
}
}
> And the in() interfaces could be defined to 0xff[ff].
>
> I assume that you don't want to provide such empty macros in the
> kernel's io.h header?
That was the original idea many years ago, but Linus rejected
my pull request for it, so Niklas worked through all drivers
individually to add the dependencies instead.
Arnd
Powered by blists - more mailing lists