linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 32/38] staging: add HAS_IOPORT dependencies
       [not found] <20230314121216.413434-1-schnelle@linux.ibm.com>
@ 2023-03-14 12:12 ` Niklas Schnelle
  2023-03-14 12:12 ` [PATCH v3 35/38] video: handle " Niklas Schnelle
  1 sibling, 0 replies; 7+ messages in thread
From: Niklas Schnelle @ 2023-03-14 12:12 UTC (permalink / raw)
  To: Arnd Bergmann, Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
	Forest Bond
  Cc: Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab,
	Alan Stern, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, linux-fbdev, linux-staging

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/staging/sm750fb/Kconfig | 2 +-
 drivers/staging/vt6655/Kconfig  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/Kconfig b/drivers/staging/sm750fb/Kconfig
index 1461c89701c3..ab3d9b057d56 100644
--- a/drivers/staging/sm750fb/Kconfig
+++ b/drivers/staging/sm750fb/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 config FB_SM750
 	tristate "Silicon Motion SM750 framebuffer support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_MODE_HELPERS
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
diff --git a/drivers/staging/vt6655/Kconfig b/drivers/staging/vt6655/Kconfig
index d1cd5de46dcf..077f62ebe80c 100644
--- a/drivers/staging/vt6655/Kconfig
+++ b/drivers/staging/vt6655/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 config VT6655
    tristate "VIA Technologies VT6655 support"
-   depends on PCI && MAC80211 && m
+   depends on PCI && HAS_IOPORT && MAC80211 && m
    help
      This is a vendor-written driver for VIA VT6655.
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
       [not found] <20230314121216.413434-1-schnelle@linux.ibm.com>
  2023-03-14 12:12 ` [PATCH v3 32/38] staging: add HAS_IOPORT dependencies Niklas Schnelle
@ 2023-03-14 12:12 ` Niklas Schnelle
  2023-03-15  8:16   ` Geert Uytterhoeven
  1 sibling, 1 reply; 7+ messages in thread
From: Niklas Schnelle @ 2023-03-14 12:12 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller
  Cc: Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab,
	Alan Stern, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, linux-fbdev, dri-devel

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/video/console/Kconfig |  1 +
 drivers/video/fbdev/Kconfig   | 25 +++++++++++++------------
 include/video/vga.h           |  8 ++++++++
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 22cea5082ac4..64974eaa3ac5 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
 	depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC &&  !SUPERH && \
 		(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) && \
 		!ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
+	depends on HAS_IOPORT
 	select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
 	default y
 	help
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index ff3646c30d0d..b21a37497d22 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -338,7 +338,7 @@ config FB_IMX
 
 config FB_CYBER2000
 	tristate "CyberPro 2000/2010/5000 support"
-	depends on FB && PCI && (BROKEN || !SPARC64)
+	depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -432,6 +432,7 @@ config FB_FM2
 config FB_ARC
 	tristate "Arc Monochrome LCD board support"
 	depends on FB && (X86 || COMPILE_TEST)
+	depends on HAS_IOPORT
 	select FB_SYS_FILLRECT
 	select FB_SYS_COPYAREA
 	select FB_SYS_IMAGEBLIT
@@ -1260,7 +1261,7 @@ config FB_RADEON_DEBUG
 
 config FB_ATY128
 	tristate "ATI Rage128 display support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
 
 config FB_ATY
 	tristate "ATI Mach64 display support" if PCI || ATARI
-	depends on FB && !SPARC32
+	depends on FB && HAS_IOPORT && !SPARC32
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1335,7 +1336,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
 	tristate "S3 Trio/Virge support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1396,7 +1397,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
 	tristate "SiS/XGI display support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1427,7 +1428,7 @@ config FB_SIS_315
 
 config FB_VIA
 	tristate "VIA UniChrome (Pro) and Chrome9 display support"
-	depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+	depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || COMPILE_TEST)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1466,7 +1467,7 @@ endif
 
 config FB_NEOMAGIC
 	tristate "NeoMagic display support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_MODE_HELPERS
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
@@ -1496,7 +1497,7 @@ config FB_KYRO
 
 config FB_3DFX
 	tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_IMAGEBLIT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
@@ -1546,7 +1547,7 @@ config FB_VOODOO1
 
 config FB_VT8623
 	tristate "VIA VT8623 support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1561,7 +1562,7 @@ config FB_VT8623
 
 config FB_TRIDENT
 	tristate "Trident/CyberXXX/CyberBlade support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1584,7 +1585,7 @@ config FB_TRIDENT
 
 config FB_ARK
 	tristate "ARK 2000PV support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -2198,7 +2199,7 @@ config FB_SSD1307
 
 config FB_SM712
 	tristate "Silicon Motion SM712 framebuffer support"
-	depends on FB && PCI
+	depends on FB && PCI && HAS_IOPORT
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..f4b806b85c86 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -203,18 +203,26 @@ extern int restore_vga(struct vgastate *state);
 
 static inline unsigned char vga_io_r (unsigned short port)
 {
+#ifdef CONFIG_HAS_IOPORT
 	return inb_p(port);
+#else
+	return 0xff;
+#endif
 }
 
 static inline void vga_io_w (unsigned short port, unsigned char val)
 {
+#ifdef CONFIG_HAS_IOPORT
 	outb_p(val, port);
+#endif
 }
 
 static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
 				  unsigned char val)
 {
+#ifdef CONFIG_HAS_IOPORT
 	outw(VGA_OUT16VAL (val, reg), port);
+#endif
 }
 
 static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short port)
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
  2023-03-14 12:12 ` [PATCH v3 35/38] video: handle " Niklas Schnelle
@ 2023-03-15  8:16   ` Geert Uytterhoeven
  2023-03-15 10:19     ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2023-03-15  8:16 UTC (permalink / raw)
  To: Niklas Schnelle
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller, Bjorn Helgaas,
	Uwe Kleine-König, Mauro Carvalho Chehab, Alan Stern,
	Rafael J. Wysocki, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-fbdev,
	dri-devel

Hi Niklas,

On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to add HAS_IOPORT as dependency for
> those drivers using them and guard inline code in headers.
>
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>

Thanks for your patch!

> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig

> @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
>
>  config FB_ATY
>         tristate "ATI Mach64 display support" if PCI || ATARI
> -       depends on FB && !SPARC32
> +       depends on FB && HAS_IOPORT && !SPARC32

On Atari, this works without ATARI_ROM_ISA, hence it must not depend
on HAS_IOPORT.
The only call to inb() is inside a section protected by #ifdef
CONFIG_PCI. So:

    depends on FB && !SPARC32
    depends on ATARI || HAS_IOPORT

>         select FB_CFB_FILLRECT
>         select FB_CFB_COPYAREA
>         select FB_CFB_IMAGEBLIT

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
  2023-03-15  8:16   ` Geert Uytterhoeven
@ 2023-03-15 10:19     ` Ville Syrjälä
  2023-03-23 14:17       ` Niklas Schnelle
  0 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2023-03-15 10:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Schnelle, linux-arch, Arnd Bergmann, linux-fbdev,
	Albert Ou, Arnd Bergmann, Rafael J. Wysocki, Greg Kroah-Hartman,
	Helge Deller, Paul Walmsley, linux-pci, linux-kernel, dri-devel,
	Alan Stern, Uwe Kleine-König, Bjorn Helgaas, Palmer Dabbelt,
	Mauro Carvalho Chehab

On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add HAS_IOPORT as dependency for
> > those drivers using them and guard inline code in headers.
> >
> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/video/fbdev/Kconfig
> > +++ b/drivers/video/fbdev/Kconfig
> 
> > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> >
> >  config FB_ATY
> >         tristate "ATI Mach64 display support" if PCI || ATARI
> > -       depends on FB && !SPARC32
> > +       depends on FB && HAS_IOPORT && !SPARC32
> 
> On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> on HAS_IOPORT.
> The only call to inb() is inside a section protected by #ifdef
> CONFIG_PCI. So:

That piece of code is a nop anyway. We immediately overwrite
clk_wr_offset with a hardcoded selection after the register reads.
So if you nuke that nop code then no IOPORT dependency required
at all.

> 
>     depends on FB && !SPARC32
>     depends on ATARI || HAS_IOPORT
> 
> >         select FB_CFB_FILLRECT
> >         select FB_CFB_COPYAREA
> >         select FB_CFB_IMAGEBLIT
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
  2023-03-15 10:19     ` Ville Syrjälä
@ 2023-03-23 14:17       ` Niklas Schnelle
  2023-03-23 16:08         ` Ville Syrjälä
  0 siblings, 1 reply; 7+ messages in thread
From: Niklas Schnelle @ 2023-03-23 14:17 UTC (permalink / raw)
  To: Ville Syrjälä, Geert Uytterhoeven
  Cc: linux-arch, Arnd Bergmann, linux-fbdev, Albert Ou, Arnd Bergmann,
	Rafael J. Wysocki, Greg Kroah-Hartman, Helge Deller,
	Paul Walmsley, linux-pci, linux-kernel, dri-devel, Alan Stern,
	Uwe Kleine-König, Bjorn Helgaas, Palmer Dabbelt,
	Mauro Carvalho Chehab

On Wed, 2023-03-15 at 12:19 +0200, Ville Syrjälä wrote:
> On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> > Hi Niklas,
> > 
> > On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > not being declared. We thus need to add HAS_IOPORT as dependency for
> > > those drivers using them and guard inline code in headers.
> > > 
> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > 
> > Thanks for your patch!
> > 
> > > --- a/drivers/video/fbdev/Kconfig
> > > +++ b/drivers/video/fbdev/Kconfig
> > 
> > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> > > 
> > >  config FB_ATY
> > >         tristate "ATI Mach64 display support" if PCI || ATARI
> > > -       depends on FB && !SPARC32
> > > +       depends on FB && HAS_IOPORT && !SPARC32
> > 
> > On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> > on HAS_IOPORT.
> > The only call to inb() is inside a section protected by #ifdef
> > CONFIG_PCI. So:
> 
> That piece of code is a nop anyway. We immediately overwrite
> clk_wr_offset with a hardcoded selection after the register reads.
> So if you nuke that nop code then no IOPORT dependency required
> at all.
> 

I agree this "looks" like a nop but are we sure the inb() doesn't have
side effects? 
(for reference drivers/video/fbdev/aty/aty/atyfb_base.c:
atyfb_setup_generc() towards the end)

It does feel a bit out of scope for this series but if it's really a
nop nuking it surely is the cleaner solution.

Thanks,
Niklas


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
  2023-03-23 14:17       ` Niklas Schnelle
@ 2023-03-23 16:08         ` Ville Syrjälä
  2023-05-08 17:09           ` Niklas Schnelle
  0 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2023-03-23 16:08 UTC (permalink / raw)
  To: Niklas Schnelle
  Cc: Geert Uytterhoeven, linux-arch, Arnd Bergmann, linux-fbdev,
	Albert Ou, Arnd Bergmann, Rafael J. Wysocki, Greg Kroah-Hartman,
	Helge Deller, Paul Walmsley, linux-pci, linux-kernel, dri-devel,
	Alan Stern, Uwe Kleine-König, Bjorn Helgaas, Palmer Dabbelt,
	Mauro Carvalho Chehab

On Thu, Mar 23, 2023 at 03:17:38PM +0100, Niklas Schnelle wrote:
> On Wed, 2023-03-15 at 12:19 +0200, Ville Syrjälä wrote:
> > On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> > > Hi Niklas,
> > > 
> > > On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > > not being declared. We thus need to add HAS_IOPORT as dependency for
> > > > those drivers using them and guard inline code in headers.
> > > > 
> > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > 
> > > Thanks for your patch!
> > > 
> > > > --- a/drivers/video/fbdev/Kconfig
> > > > +++ b/drivers/video/fbdev/Kconfig
> > > 
> > > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> > > > 
> > > >  config FB_ATY
> > > >         tristate "ATI Mach64 display support" if PCI || ATARI
> > > > -       depends on FB && !SPARC32
> > > > +       depends on FB && HAS_IOPORT && !SPARC32
> > > 
> > > On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> > > on HAS_IOPORT.
> > > The only call to inb() is inside a section protected by #ifdef
> > > CONFIG_PCI. So:
> > 
> > That piece of code is a nop anyway. We immediately overwrite
> > clk_wr_offset with a hardcoded selection after the register reads.
> > So if you nuke that nop code then no IOPORT dependency required
> > at all.
> > 
> 
> I agree this "looks" like a nop but are we sure the inb() doesn't have
> side effects? 

Yes. It's just trying to check which PLL dividers/etc. are currently
used. In VGA mode it gets it from a the GENMO and in non-VGA mode from
CLOCK_CNTL. And then it says "screw that" and just uses index 3 instead.

Though I must say that mach64 GX seems to use that clk_wr_offset
very differently so I'm not sure the PCI+GX combo is even working
currently, assuming those even exist. I don't think I have anything
older than a PCI mach64 VT myself.

> (for reference drivers/video/fbdev/aty/aty/atyfb_base.c:
> atyfb_setup_generc() towards the end)
> 
> It does feel a bit out of scope for this series but if it's really a
> nop nuking it surely is the cleaner solution.
> 
> Thanks,
> Niklas

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
  2023-03-23 16:08         ` Ville Syrjälä
@ 2023-05-08 17:09           ` Niklas Schnelle
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Schnelle @ 2023-05-08 17:09 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Geert Uytterhoeven, linux-arch, Arnd Bergmann, linux-fbdev,
	Albert Ou, Arnd Bergmann, Rafael J. Wysocki, Greg Kroah-Hartman,
	Helge Deller, Paul Walmsley, linux-pci, linux-kernel, dri-devel,
	Alan Stern, Uwe Kleine-König, Bjorn Helgaas, Palmer Dabbelt,
	Mauro Carvalho Chehab

On Thu, 2023-03-23 at 18:08 +0200, Ville Syrjälä wrote:
> On Thu, Mar 23, 2023 at 03:17:38PM +0100, Niklas Schnelle wrote:
> > On Wed, 2023-03-15 at 12:19 +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> > > > Hi Niklas,
> > > > 
> > > > On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > > > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > > > not being declared. We thus need to add HAS_IOPORT as dependency for
> > > > > those drivers using them and guard inline code in headers.
> > > > > 
> > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > > 
> > > > Thanks for your patch!
> > > > 
> > > > > --- a/drivers/video/fbdev/Kconfig
> > > > > +++ b/drivers/video/fbdev/Kconfig
> > > > 
> > > > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> > > > > 
> > > > >  config FB_ATY
> > > > >         tristate "ATI Mach64 display support" if PCI || ATARI
> > > > > -       depends on FB && !SPARC32
> > > > > +       depends on FB && HAS_IOPORT && !SPARC32
> > > > 
> > > > On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> > > > on HAS_IOPORT.
> > > > The only call to inb() is inside a section protected by #ifdef
> > > > CONFIG_PCI. So:
> > > 
> > > That piece of code is a nop anyway. We immediately overwrite
> > > clk_wr_offset with a hardcoded selection after the register reads.
> > > So if you nuke that nop code then no IOPORT dependency required
> > > at all.
> > > 
> > 
> > I agree this "looks" like a nop but are we sure the inb() doesn't have
> > side effects? 
> 
> Yes. It's just trying to check which PLL dividers/etc. are currently
> used. In VGA mode it gets it from a the GENMO and in non-VGA mode from
> CLOCK_CNTL. And then it says "screw that" and just uses index 3 instead.
> 

Ok, I've added a patch to remove this part of the code and with that
the driver actually builds on s390 (no HAS_IOPORT) so I also removed
the HAS_IOPORT dependency. Both will be in my v4.

Thanks,
Niklas



^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-08 17:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20230314121216.413434-1-schnelle@linux.ibm.com>
2023-03-14 12:12 ` [PATCH v3 32/38] staging: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 35/38] video: handle " Niklas Schnelle
2023-03-15  8:16   ` Geert Uytterhoeven
2023-03-15 10:19     ` Ville Syrjälä
2023-03-23 14:17       ` Niklas Schnelle
2023-03-23 16:08         ` Ville Syrjälä
2023-05-08 17:09           ` Niklas Schnelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).