All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Garry <john.garry@huawei.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	linux-pci <linux-pci@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	Niklas Schnelle <schnelle@linux.ibm.com>
Subject: Re: [GIT PULL 1/2] asm-generic: rework PCI I/O space access
Date: Tue, 3 Aug 2021 12:23:55 +0100	[thread overview]
Message-ID: <a74dfb1f-befd-92ce-4c30-233cb08e04d3@huawei.com> (raw)
In-Reply-To: <CAK8P3a3HHeP+Gw_k2P7Qtig0OmErf0HN30G22+qHic_uZTh11Q@mail.gmail.com>

>> Anyway, one thing I mentioned earlier was that we could solve the
>> problem of drivers accessing unmapped IO ports and crashing systems on
>> archs which define PCI_IOBASE by building them under some "native port
>> IO support" flag.
> Right, that was part of the goal here.

Great

> 
>> One example of such a driver was F71805F sensor. You put that under
>> HAS_IOPORT, which would be available for all archs, I think. But I could
>> not see where config LEGACY_PCI is introduced. Could we further refine
>> that config to not build for such archs as arm64?
>>
>> BTW, I think that the PPC dependency was added there to stop building
>> for power for that same reason, so hopefully we get rid of that.
> Good point. It seems that I actually never added the LEGACY_PCI option
> to my patch,

ok, it would be nice to see that.

> so I'm just not building those drivers any more, and not
> defining the inb()/outb() helpers either, causing a build failure when I'm
> missing an option.
> 
> However it sounds like you are interested in a third option here, which
> brings us to:
> 
> LEGACY_PCI: any PCI driver that uses inb()/outb() or is only available
>      on old-style PCI but not PCIe hardware without a bridge.
>      To be disabled for most architectures and possibly distros but can
>      be enabled for kernels that want to use those devices, as long as
>      CONFIG_HAS_IOPORT is set by the architecture.
> 
> HAS_IOPORT: not a legacy PCI device, but can only be built on
>      architectures that define inb()/outb(). To be disabled for s390
>      and any other machine that has no useful definition of those
>      functions.

That seems reasonable. And asm-generic io.h should be ifdef'ed by 
HAS_IOPORT. In your patch you had it under CONFIG_IOPORT - was that 
intentional?

On another point, I noticed SCSI driver AHA152x depends on ISA, but is 
not an isa driver - however it does use port IO. Would such dependencies 
need to be changed to depend on HAS_IOPORT?

I did notice that arm32 support CONFIG_ISA - not sure why.

> 
> HARDCODED_IOPORT: (or another name you might think of,) Used by
>     drivers that unconditionally do inb()/outb() without checking the
>     validity of the address using firmware or other methods first.
>     depends on HAS_IOPORT and possibly architecture specific
>     settings.

Yeah, that sounds the same as what I was thinking. Maybe IOPORT_NATIVE 
could work as a name. I would think that only x86/ia64 would define it. 
A concern though is that someone could argue that is a functional 
dependency, rather than just a build dependency.

Thanks,
John


  reply	other threads:[~2021-08-03 11:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02 13:47 [GIT PULL 1/2] asm-generic: rework PCI I/O space access Arnd Bergmann
2021-07-02 13:49 ` [GIT PULL 2/2] asm-generic: Unify asm/unaligned.h around struct helper Arnd Bergmann
2021-07-02 20:13   ` pr-tracker-bot
2021-07-02 19:42 ` [GIT PULL 1/2] asm-generic: rework PCI I/O space access Linus Torvalds
2021-07-03 12:12   ` Arnd Bergmann
2021-07-05 10:06     ` Arnd Bergmann
2021-08-03  9:46       ` John Garry
2021-08-03 10:06         ` Arnd Bergmann
2021-08-03 11:23           ` John Garry [this message]
2021-08-03 12:15             ` Arnd Bergmann
2021-08-04  7:55               ` John Garry
2021-08-04  8:52                 ` Arnd Bergmann
2021-08-10  9:19                   ` John Garry
2021-08-10 11:33                     ` Arnd Bergmann
2021-09-03  8:31                       ` Niklas Schnelle
2021-12-17 13:19                       ` Niklas Schnelle
2021-12-17 13:32                         ` Arnd Bergmann
2021-12-17 13:52                           ` Niklas Schnelle
2021-12-17 14:05                             ` Arnd Bergmann
2021-12-17 14:27                             ` John Garry
2021-12-17 14:32                               ` Arnd Bergmann
2021-12-17 15:27                                 ` John Garry
2021-12-17 15:55                                   ` Arnd Bergmann
2021-12-17 16:30                                     ` John Garry
2021-12-20  9:27                                       ` Niklas Schnelle
2021-12-21 16:48                                         ` John Garry
2021-12-21 16:57                                           ` Niklas Schnelle
2021-12-19 14:23                               ` David Laight
2021-12-21 16:21                                 ` John Garry
2021-07-05 12:40     ` Niklas Schnelle

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=a74dfb1f-befd-92ce-4c30-233cb08e04d3@huawei.com \
    --to=john.garry@huawei.com \
    --cc=arnd@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=schnelle@linux.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /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.