All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>
Cc: linux-arch@vger.kernel.org, "Arnd Bergmann" <arnd@kernel.org>,
	linux-fbdev@vger.kernel.org, "Albert Ou" <aou@eecs.berkeley.edu>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Helge Deller" <deller@gmx.de>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	"Alan Stern" <stern@rowland.harvard.edu>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>
Subject: Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
Date: Thu, 23 Mar 2023 15:17:38 +0100	[thread overview]
Message-ID: <917b95c9af1b80843b8a361d1b7fa337a25105e7.camel@linux.ibm.com> (raw)
In-Reply-To: <ZBGbxDWEhqr8hhgU@intel.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>
Cc: linux-arch@vger.kernel.org, "Arnd Bergmann" <arnd@kernel.org>,
	linux-fbdev@vger.kernel.org, "Albert Ou" <aou@eecs.berkeley.edu>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Helge Deller" <deller@gmx.de>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	"Alan Stern" <stern@rowland.harvard.edu>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>
Subject: Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
Date: Thu, 23 Mar 2023 15:17:38 +0100	[thread overview]
Message-ID: <917b95c9af1b80843b8a361d1b7fa337a25105e7.camel@linux.ibm.com> (raw)
In-Reply-To: <ZBGbxDWEhqr8hhgU@intel.com>

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


  reply	other threads:[~2023-03-23 14:18 UTC|newest]

Thread overview: 163+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-14 12:11 [PATCH v3 00/38] Kconfig: Introduce HAS_IOPORT config option Niklas Schnelle
2023-03-14 12:11 ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-14 12:37   ` Johannes Berg
2023-03-14 12:37     ` Johannes Berg
2023-03-14 12:37     ` Johannes Berg
2023-03-14 12:37     ` Johannes Berg
2023-03-14 12:37     ` Johannes Berg
2023-03-14 12:37     ` Johannes Berg
2023-03-23 13:23     ` Niklas Schnelle
2023-03-23 13:23       ` Niklas Schnelle
2023-03-23 13:23       ` Niklas Schnelle
2023-03-23 13:23       ` Niklas Schnelle
2023-03-23 13:23       ` Niklas Schnelle
2023-03-23 13:23       ` Niklas Schnelle
2023-03-14 12:48   ` Geert Uytterhoeven
2023-03-14 12:48     ` Geert Uytterhoeven
2023-03-14 12:48     ` Geert Uytterhoeven
2023-03-14 12:48     ` Geert Uytterhoeven
2023-03-14 12:48     ` Geert Uytterhoeven
2023-03-14 12:48     ` Geert Uytterhoeven
2023-03-14 13:29   ` Arnd Bergmann
2023-03-14 13:29     ` Arnd Bergmann
2023-03-14 13:29     ` Arnd Bergmann
2023-03-14 13:29     ` Arnd Bergmann
2023-03-14 13:29     ` Arnd Bergmann
2023-03-14 13:29     ` Arnd Bergmann
2023-03-14 16:11     ` Niklas Schnelle
2023-03-14 16:11       ` Niklas Schnelle
2023-03-14 16:11       ` Niklas Schnelle
2023-03-14 16:11       ` Niklas Schnelle
2023-03-14 16:11       ` Niklas Schnelle
2023-03-14 16:11       ` Niklas Schnelle
2023-03-14 16:37   ` Niklas Schnelle
2023-03-14 16:37     ` Niklas Schnelle
2023-03-14 16:37     ` Niklas Schnelle
2023-03-14 16:37     ` Niklas Schnelle
2023-03-14 16:37     ` Niklas Schnelle
2023-03-14 16:37     ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 02/38] ata: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-15  1:23   ` Damien Le Moal
2023-03-15  6:46     ` Arnd Bergmann
2023-03-15  7:15       ` Damien Le Moal
2023-03-15  7:15   ` Damien Le Moal
2023-03-15  8:39   ` Geert Uytterhoeven
2023-03-15  9:12     ` Damien Le Moal
2023-03-15 11:36       ` Geert Uytterhoeven
2023-03-15 23:57         ` Damien Le Moal
2023-03-16 15:21           ` Arnd Bergmann
2023-04-28 13:32             ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 03/38] char: impi, tpm: depend on HAS_IOPORT Niklas Schnelle
2023-03-14 12:20   ` Jarkko Sakkinen
2023-03-14 14:01     ` Arnd Bergmann
2023-03-14 13:52   ` Corey Minyard
2023-03-14 14:17   ` Geert Uytterhoeven
2023-03-14 15:08     ` Arnd Bergmann
2023-03-21 14:33   ` Jason Gunthorpe
2023-03-14 12:11 ` [PATCH v3 04/38] comedi: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 05/38] counter: " Niklas Schnelle
2023-03-14 12:41   ` William Breathitt Gray
2023-04-28 13:27     ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 06/38] /dev/port: don't compile file operations without CONFIG_DEVPORT Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 07/38] drm: handle HAS_IOPORT dependencies Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-15 11:54   ` Jani Nikula
2023-03-15 11:54     ` Jani Nikula
2023-03-15 11:54     ` Jani Nikula
2023-03-23 15:05     ` Niklas Schnelle
2023-03-23 15:05       ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 08/38] firmware: dmi-sysfs: handle HAS_IOPORT=n Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 09/38] gpio: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-15  8:42   ` Linus Walleij
2023-04-28 14:41     ` Niklas Schnelle
2023-05-01 12:29       ` Linus Walleij
2023-03-14 12:11 ` [PATCH v3 10/38] hwmon: " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 11/38] i2c: " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 12/38] iio: ad7606: Kconfig: " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 13/38] Input: " Niklas Schnelle
2023-03-15  8:22   ` Geert Uytterhoeven
2023-04-28 14:50     ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 14/38] Input: gameport: add ISA and " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 15/38] leds: add " Niklas Schnelle
2023-03-16 16:14   ` Lee Jones
2023-03-23 12:42     ` Niklas Schnelle
2023-03-23 13:32       ` Arnd Bergmann
2023-03-23 14:02         ` Niklas Schnelle
2023-03-23 14:05           ` Arnd Bergmann
2023-03-23 16:11         ` Geert Uytterhoeven
2023-03-23 16:25           ` Arnd Bergmann
2023-03-23 14:53       ` Lee Jones
2023-03-23 15:33         ` Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 16/38] media: " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 17/38] misc: " Niklas Schnelle
2023-03-14 12:11 ` [PATCH v3 18/38] mISDN: " Niklas Schnelle
2023-03-15  5:41   ` Jakub Kicinski
2023-03-14 12:11 ` [PATCH v3 19/38] mpt fusion: " Niklas Schnelle
2023-03-14 12:11 ` [Intel-wired-lan] [PATCH v3 20/38] net: handle " Niklas Schnelle
2023-03-14 12:11   ` Niklas Schnelle
2023-03-15  5:41   ` Jakub Kicinski
2023-03-15  5:41     ` Jakub Kicinski
2023-03-15  5:41     ` [Intel-wired-lan] " Jakub Kicinski
2023-03-15  9:47   ` Maciej W. Rozycki
2023-03-15  9:47     ` Maciej W. Rozycki
2023-03-14 12:11 ` [PATCH v3 21/38] parport: PC style parport depends on HAS_IOPORT Niklas Schnelle
2023-03-14 14:12   ` Arnd Bergmann
2023-05-02 13:40     ` Niklas Schnelle
2023-05-02 15:30   ` Bjorn Helgaas
2023-03-14 12:12 ` [PATCH v3 22/38] PCI: Make quirk using inw() depend " Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 23/38] PCI/sysfs: Make I/O resource " Niklas Schnelle
2023-03-14 16:05   ` Bjorn Helgaas
2023-03-14 12:12 ` [PATCH v3 24/38] pcmcia: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 25/38] platform: " Niklas Schnelle
2023-03-15  2:21   ` Tzung-Bi Shih
2023-03-14 12:12 ` [PATCH v3 26/38] pnp: " Niklas Schnelle
2023-03-20 17:37   ` Rafael J. Wysocki
2023-03-21 13:56     ` Rafael J. Wysocki
2023-03-23 12:55       ` Niklas Schnelle
2023-03-24 18:45         ` Wysocki, Rafael J
2023-03-14 12:12 ` [PATCH v3 27/38] power: " Niklas Schnelle
2023-03-29 23:01   ` Sebastian Reichel
2023-03-14 12:12 ` [PATCH v3 28/38] rtc: " Niklas Schnelle
2023-03-14 12:52   ` Alexandre Belloni
2023-05-08 15:36     ` Niklas Schnelle
2023-05-08 20:01       ` Arnd Bergmann
2023-05-09  6:38         ` Geert Uytterhoeven
2023-05-09  8:22           ` Arnd Bergmann
2023-05-09  8:51             ` Geert Uytterhoeven
2023-05-09  9:46               ` Arnd Bergmann
2023-03-14 12:12 ` [PATCH v3 29/38] scsi: " Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 30/38] sound: " Niklas Schnelle
2023-03-14 12:33   ` Takashi Iwai
2023-03-14 12:33     ` Takashi Iwai
2023-05-08 16:41     ` Niklas Schnelle
2023-05-08 16:41       ` Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 31/38] speakup: add HAS_IOPORT dependency for SPEAKUP_SERIALIO Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 32/38] staging: add HAS_IOPORT dependencies Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 33/38] tty: serial: handle " Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 34/38] usb: " Niklas Schnelle
2023-03-14 14:56   ` Arnd Bergmann
2023-03-14 12:12 ` [PATCH v3 35/38] video: " Niklas Schnelle
2023-03-14 12:12   ` Niklas Schnelle
2023-03-15  8:16   ` Geert Uytterhoeven
2023-03-15  8:16     ` Geert Uytterhoeven
2023-03-15 10:19     ` Ville Syrjälä
2023-03-15 10:19       ` Ville Syrjälä
2023-03-23 14:17       ` Niklas Schnelle [this message]
2023-03-23 14:17         ` Niklas Schnelle
2023-03-23 16:08         ` Ville Syrjälä
2023-03-23 16:08           ` Ville Syrjälä
2023-05-08 17:09           ` Niklas Schnelle
2023-05-08 17:09             ` Niklas Schnelle
2023-03-14 12:12 ` [PATCH v3 36/38] watchdog: add " Niklas Schnelle
2023-03-14 14:21   ` Guenter Roeck
2023-03-14 12:12 ` [PATCH v3 37/38] wireless: " Niklas Schnelle
2023-03-15  5:26   ` Kalle Valo
2023-03-14 12:12 ` [PATCH v3 38/38] asm-generic/io.h: drop inb() etc for HAS_IOPORT=n Niklas Schnelle
2023-03-14 14:05 ` [PATCH v3 00/38] Kconfig: Introduce HAS_IOPORT config option Arnd Bergmann
2023-03-14 14:05   ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=917b95c9af1b80843b8a361d1b7fa337a25105e7.camel@linux.ibm.com \
    --to=schnelle@linux.ibm.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rafael@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.