From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3AA8FC433F5 for ; Wed, 29 Dec 2021 11:46:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TkH+4zBmx9zm58NcreznHWCsCr+PXmGHpa1DBuqop7Y=; b=3VJsQ798ADHBZV RpVaBkSTQhJ5E5u42sBuYanJnVmmkha6alou37zHIwj5C0V1TBE8YgaZ9wSv+FB6Ke27sIYgHktuY FLRdBik0+jQ+DZqlUb0B+Ivx86/V0x0c2InvTajLjobIuaJZxPoCJksXPDEGe8NzlLowdbji4bTbb 5QQjwaaHUVkt3pm40t8CE4VuKsuPG0CG4E86I2YsIiCMYFpHUY3k5xq3J3F8gDazzxZJ7wsDQN7IL KQTzsvfuwmCteoJJejBnSKHPGP5uhFkeNXQBwx+JWXJn/WVzLmXgrENHY1pUqIKhZwhKsxP77j89I ux546XhlAxAazoTHOYhg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n2XPD-002hCe-Oj; Wed, 29 Dec 2021 11:46:07 +0000 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n2XPA-002hCL-D4 for linux-riscv@lists.infradead.org; Wed, 29 Dec 2021 11:46:06 +0000 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1BT8Pn7i011761; Wed, 29 Dec 2021 11:45:52 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : content-transfer-encoding : mime-version; s=pp1; bh=i5NP9XctF1wJk5OjQUo+Vc9uVmlIqZSnBT0u09LTmOk=; b=bFPRLC+jl+efdix4uwpI/mccWCcWO/G1P+1Qbs1rTtCAz+rovwnnR8pPTU6lF0Xc0joV YXPBuRb7BHXzyjRg7LITTk2SnNio9SeVldUJyNRqyoNhK4M9oyDLsc8IgyEZJiRqRm5A iHUmkTc7uG3NhK6BlA+KFy7MBz/iVluQaCQBkvCSyAyJcdcbD85ipXooRay1nJfQKYhr pBhuhwTvWtOhRjZ8iX5rVE8/8svHYNhhRkTQSfMP/PoiugU551FyKdeVPLCMOig/L1Pb f+O+aj84GXG4UtoEonKAgWOYfXmY61i73T8/AZi6WWLQ0MnyNiHoz+SQfgz7xDZNEMLG PQ== Received: from ppma01fra.de.ibm.com (46.49.7a9f.ip4.static.sl-reverse.com [159.122.73.70]) by mx0a-001b2d01.pphosted.com with ESMTP id 3d7rpyt5s9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:51 +0000 Received: from pps.filterd (ppma01fra.de.ibm.com [127.0.0.1]) by ppma01fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1BTBg94u031832; Wed, 29 Dec 2021 11:45:48 GMT Received: from b06avi18626390.portsmouth.uk.ibm.com (b06avi18626390.portsmouth.uk.ibm.com [9.149.26.192]) by ppma01fra.de.ibm.com with ESMTP id 3d5tx9cy34-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:48 +0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06avi18626390.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1BTBbEnS47841776 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 29 Dec 2021 11:37:14 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1CC32AE055; Wed, 29 Dec 2021 11:45:45 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DA088AE053; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Received: from sig-9-145-13-177.uk.ibm.com (unknown [9.145.13.177]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Message-ID: <8c59271de66dfa230142186d593f3501b8c789b1.camel@linux.ibm.com> Subject: Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI From: Niklas Schnelle To: Mauro Carvalho Chehab , Hans Verkuil , Ettore Chimenti Cc: Bjorn Helgaas , Greg Kroah-Hartman , Arnd Bergmann , Bjorn Helgaas , John Garry , Nick Hu , Greentime Hu , Vincent Chen , Paul Walmsley , Palmer Dabbelt , Albert Ou , Guo Ren , Damien Le Moal , Ian Abbott , H Hartley Sweeten , Linus Walleij , Bartosz Golaszewski , Jean Delvare , Guenter Roeck , Dmitry Torokhov , Karsten Keil , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Michael Grzeschik , "David S. Miller" , Jakub Kicinski , Jesse Brandeburg , Tony Nguyen , Kalle Valo , Jouni Malinen , "James E.J. Bottomley" , "Martin K. Petersen" , Hannes Reinecke , Kashyap Desai , Sumit Saxena , Shivasharan S , Nilesh Javali , GR-QLogic-Storage-Upstream@marvell.com, Mark Brown , Sudip Mukherjee , Teddy Wang , Forest Bond , Jiri Slaby , Wim Van Sebroeck , Jaroslav Kysela , Takashi Iwai , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-pci@vger.kernel.org, linux-riscv@lists.infradead.org, linux-csky@vger.kernel.org, linux-ide@vger.kernel.org, linux-gpio@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-input@vger.kernel.org, netdev@vger.kernel.org, linux-media@vger.kernel.org, MPT-FusionLinux.pdl@broadcom.com, linux-scsi@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-wireless@vger.kernel.org, megaraidlinux.pdl@broadcom.com, linux-spi@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-serial@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-watchdog@vger.kernel.org Date: Wed, 29 Dec 2021 12:45:38 +0100 In-Reply-To: <20211228181107.2d476028@coco.lan> References: <20211227164317.4146918-1-schnelle@linux.ibm.com> <20211227164317.4146918-2-schnelle@linux.ibm.com> <20211228101435.3a55b983@coco.lan> <20211228135425.0a69168c@coco.lan> <4b630b7b87bd983291f628c42a1394fc0d2d86bd.camel@linux.ibm.com> <20211228181107.2d476028@coco.lan> X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) X-TM-AS-GCONF: 00 X-Proofpoint-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn X-Proofpoint-ORIG-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2021-12-29_04,2021-12-29_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxlogscore=999 spamscore=0 phishscore=0 adultscore=0 mlxscore=0 bulkscore=0 clxscore=1015 malwarescore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2112290062 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211229_034604_485047_43665115 X-CRM114-Status: GOOD ( 70.52 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Tue, 2021-12-28 at 18:12 +0100, Mauro Carvalho Chehab wrote: > Em Tue, 28 Dec 2021 16:06:44 +0100 > Niklas Schnelle escreveu: > > (on a side note: the c/c list of this patch is too long. I would try to > avoid using a too long list, as otherwise this e-mail may end being rejected > by mail servers) > > > On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote: > > > > > ---8<--- > > > > > > > > > All you really care about is the "legacy" I/O spaces here, this isn't > > > > > > tied to PCI specifically at all, right? > > > > > > > > > > > > So why not just have a OLD_STYLE_IO config option or something like > > > > > > that, to show that it's the i/o functions we care about here, not PCI at > > > > > > all? > > > > > > > > > > > > And maybe not call it "old" or "legacy" as time constantly goes forward, > > > > > > just describe it as it is, "DIRECT_IO"? > > > > > > > > > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate > > > > > name for it. > > > > > > > > > > Thanks, > > > > > Mauro > > > > > > > > Hmm, I might be missing something here but that sounds a lot like the > > > > HAS_IOPORT option added in patch 02. > > > > > > > > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two > > > > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card > > > > while LEGACY_PCI is for PCI drivers that require port I/O. > > > > > > I didn't look at the other patches on this series, but why it is needed > > > to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be enough? > > > > > > I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y > > > where LEGACY_PCI shall be "n"? > > > > In the current patch set LEGACY_PCI is not currently selected by > > architectures, though of course it could be if we know that an > > architecture requires it. We should probably also set it in any > > defconfig that has devices depending on it so as not to break these. > > > > Other than that it would be set during kernel configuration if one > > wants/needs support for legacy PCI devices. For testing I ran with > > HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X > > based workstation and Raspberry Pi 4 (DT). I guess at the moment it > > would make most sense for special configs such as those tailored for > > vitualization guets but in the end that would be something for > > distributions to decide. > > IMO, it makes sense to have a "default y" there, as on systems that > support I/O space, disabling it will just randomly disable some drivers > that could be required by some hardware. I won't doubt that some of > those could be ported from using inb/outb to use, instead, readb/writeb. Makes sense, if these get more legacy over time we can always change the default. This would also mean we don't need to change defconfigs that include legacy PCI devices. > > > Arnd described the options here: > > https://lore.kernel.org/lkml/CAK8P3a3HHeP+Gw_k2P7Qtig0OmErf0HN30G22+qHic_uZTh11Q@mail.gmail.com/ > > Based on Arnd's description, LEGACY_PCI should depend on HAS_IOPORT. > This is missing on patch 1. You should probably reorder your patch > series to first create HAS_IOPORT and then add LEGACY_PCI with > depends on, as otherwise it may cause randconfig build issues > at robots and/or git bisect. > > I would also suggest to first introduce such change and then send > a per-subsystem LEGACY_PCI patch, as it would be a lot easier for > maintainers to review. Playing around with the reordering I think it might make sense to introduce HAS_IOPORT in patch 01, then LEGACY_PCI in patch 02 and then add dependencies for both on a per subsystem basis. I think it would be overkill to have two series of per subsystem patches. > > > > > > > > This > > > > includes pre-PCIe devices as well as PCIe devices which require > > > > features like I/O spaces. The "legacy" naming is comes from the PCIe > > > > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for > > > > compatibility with legacy devices which require their use. Future > > > > revisions of this specification may deprecate the use of I/O Space." > > > > > > I would still avoid calling it LEGACY_PCI, as this sounds too generic. > > > > > > I didn't read the PCI/PCIe specs, but I suspect that are a lot more > > > features that were/will be deprecated on PCI specs as time goes by. > > > > > > So, I would, instead, use something like PCI_LEGACY_IO_SPACE or > > > HAVE_PCI_LEGACY_IO_SPACE, in order to let it clear what "legacy" > > > means. > > > > Hmm, I'd like to hear Bjorn's opinion on this. Personally I feel like > > LEGACY_PCI is pretty clear since most devices are either pre-PCIe > > devices or a compatibility feature allowing drivers for a pre-PCIe > > device to work with a PCIe device. > > That's the main point: it is *not* disabling pre-PCIe devices or > even legacy PCI drivers. It just disables a random set of drivers just > because they use inb/outb instead of readb/writeb. It keeps several pure > PCI drivers selected, and disables some PCIe for no real reason. That is not intentional. The dependencies are certainly not perfect yet which is one of the reasons this is still an RFC. I hope getting these right will be a lot easier if we do both LEGACY_PCI and HAS_IOPORT dependency selection on a per subsystem basis. > > Just to give one example, this symbol: > > > diff --git a/drivers/media/cec/platform/Kconfig b/drivers/media/cec/platform/Kconfig > > index b672d3142eb7..5e92ece5b104 100644 > > --- a/drivers/media/cec/platform/Kconfig > > +++ b/drivers/media/cec/platform/Kconfig > > @@ -100,7 +100,7 @@ config CEC_TEGRA > > config CEC_SECO > > tristate "SECO Boards HDMI CEC driver" > > depends on (X86 || IA64) || COMPILE_TEST > > - depends on PCI && DMI > > + depends on LEGACY_PCI && DMI > > select CEC_CORE > > select CEC_NOTIFIER > > help > > Disables HDMI CEC support on some Intel motherboards. > Any distro meant to run on generic hardware should keep it selected. As far as I can see this one actually uses a hardcoded I/O port numbers and googling it looks like it's an on-board device on the UDOO x86 board. I guess that should indeed just be "depends on PCI && DMI && HAS_IOPORT". > > I can see some value of a "PCI_LEGACY" option to disable all > non-PCIe drivers, but this is not the case here. > > Thanks, > Mauro Ok, I think we definitely need to work on getting the dependencies right. I do think we agree that once done correctly there is value in such an option independent of HAS_IOPORT only gating inb() etc uses. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EDBEDC433EF for ; Wed, 29 Dec 2021 11:47:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240050AbhL2LrQ (ORCPT ); Wed, 29 Dec 2021 06:47:16 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:39060 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235832AbhL2LrL (ORCPT ); Wed, 29 Dec 2021 06:47:11 -0500 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1BT8Pn7i011761; Wed, 29 Dec 2021 11:45:52 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : content-transfer-encoding : mime-version; s=pp1; bh=i5NP9XctF1wJk5OjQUo+Vc9uVmlIqZSnBT0u09LTmOk=; b=bFPRLC+jl+efdix4uwpI/mccWCcWO/G1P+1Qbs1rTtCAz+rovwnnR8pPTU6lF0Xc0joV YXPBuRb7BHXzyjRg7LITTk2SnNio9SeVldUJyNRqyoNhK4M9oyDLsc8IgyEZJiRqRm5A iHUmkTc7uG3NhK6BlA+KFy7MBz/iVluQaCQBkvCSyAyJcdcbD85ipXooRay1nJfQKYhr pBhuhwTvWtOhRjZ8iX5rVE8/8svHYNhhRkTQSfMP/PoiugU551FyKdeVPLCMOig/L1Pb f+O+aj84GXG4UtoEonKAgWOYfXmY61i73T8/AZi6WWLQ0MnyNiHoz+SQfgz7xDZNEMLG PQ== Received: from ppma01fra.de.ibm.com (46.49.7a9f.ip4.static.sl-reverse.com [159.122.73.70]) by mx0a-001b2d01.pphosted.com with ESMTP id 3d7rpyt5s9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:51 +0000 Received: from pps.filterd (ppma01fra.de.ibm.com [127.0.0.1]) by ppma01fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1BTBg94u031832; Wed, 29 Dec 2021 11:45:48 GMT Received: from b06avi18626390.portsmouth.uk.ibm.com (b06avi18626390.portsmouth.uk.ibm.com [9.149.26.192]) by ppma01fra.de.ibm.com with ESMTP id 3d5tx9cy34-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:48 +0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06avi18626390.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1BTBbEnS47841776 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 29 Dec 2021 11:37:14 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1CC32AE055; Wed, 29 Dec 2021 11:45:45 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DA088AE053; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Received: from sig-9-145-13-177.uk.ibm.com (unknown [9.145.13.177]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Message-ID: <8c59271de66dfa230142186d593f3501b8c789b1.camel@linux.ibm.com> Subject: Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI From: Niklas Schnelle To: Mauro Carvalho Chehab , Hans Verkuil , Ettore Chimenti Cc: Bjorn Helgaas , Greg Kroah-Hartman , Arnd Bergmann , Bjorn Helgaas , John Garry , Nick Hu , Greentime Hu , Vincent Chen , Paul Walmsley , Palmer Dabbelt , Albert Ou , Guo Ren , Damien Le Moal , Ian Abbott , H Hartley Sweeten , Linus Walleij , Bartosz Golaszewski , Jean Delvare , Guenter Roeck , Dmitry Torokhov , Karsten Keil , Sathya Prakash , Sreekanth Reddy , Suganath Prabu Subramani , Michael Grzeschik , "David S. Miller" , Jakub Kicinski , Jesse Brandeburg , Tony Nguyen , Kalle Valo , Jouni Malinen , "James E.J. Bottomley" , "Martin K. Petersen" , Hannes Reinecke , Kashyap Desai , Sumit Saxena , Shivasharan S , Nilesh Javali , GR-QLogic-Storage-Upstream@marvell.com, Mark Brown , Sudip Mukherjee , Teddy Wang , Forest Bond , Jiri Slaby , Wim Van Sebroeck , Jaroslav Kysela , Takashi Iwai , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-pci@vger.kernel.org, linux-riscv@lists.infradead.org, linux-csky@vger.kernel.org, linux-ide@vger.kernel.org, linux-gpio@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org, linux-input@vger.kernel.org, netdev@vger.kernel.org, linux-media@vger.kernel.org, MPT-FusionLinux.pdl@broadcom.com, linux-scsi@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-wireless@vger.kernel.org, megaraidlinux.pdl@broadcom.com, linux-spi@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-serial@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-watchdog@vger.kernel.org Date: Wed, 29 Dec 2021 12:45:38 +0100 In-Reply-To: <20211228181107.2d476028@coco.lan> References: <20211227164317.4146918-1-schnelle@linux.ibm.com> <20211227164317.4146918-2-schnelle@linux.ibm.com> <20211228101435.3a55b983@coco.lan> <20211228135425.0a69168c@coco.lan> <4b630b7b87bd983291f628c42a1394fc0d2d86bd.camel@linux.ibm.com> <20211228181107.2d476028@coco.lan> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) X-TM-AS-GCONF: 00 X-Proofpoint-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn X-Proofpoint-ORIG-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2021-12-29_04,2021-12-29_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxlogscore=999 spamscore=0 phishscore=0 adultscore=0 mlxscore=0 bulkscore=0 clxscore=1015 malwarescore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2112290062 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Tue, 2021-12-28 at 18:12 +0100, Mauro Carvalho Chehab wrote: > Em Tue, 28 Dec 2021 16:06:44 +0100 > Niklas Schnelle escreveu: > > (on a side note: the c/c list of this patch is too long. I would try to > avoid using a too long list, as otherwise this e-mail may end being rejected > by mail servers) > > > On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote: > > > > > ---8<--- > > > > > > > > > All you really care about is the "legacy" I/O spaces here, this isn't > > > > > > tied to PCI specifically at all, right? > > > > > > > > > > > > So why not just have a OLD_STYLE_IO config option or something like > > > > > > that, to show that it's the i/o functions we care about here, not PCI at > > > > > > all? > > > > > > > > > > > > And maybe not call it "old" or "legacy" as time constantly goes forward, > > > > > > just describe it as it is, "DIRECT_IO"? > > > > > > > > > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate > > > > > name for it. > > > > > > > > > > Thanks, > > > > > Mauro > > > > > > > > Hmm, I might be missing something here but that sounds a lot like the > > > > HAS_IOPORT option added in patch 02. > > > > > > > > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two > > > > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card > > > > while LEGACY_PCI is for PCI drivers that require port I/O. > > > > > > I didn't look at the other patches on this series, but why it is needed > > > to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be enough? > > > > > > I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y > > > where LEGACY_PCI shall be "n"? > > > > In the current patch set LEGACY_PCI is not currently selected by > > architectures, though of course it could be if we know that an > > architecture requires it. We should probably also set it in any > > defconfig that has devices depending on it so as not to break these. > > > > Other than that it would be set during kernel configuration if one > > wants/needs support for legacy PCI devices. For testing I ran with > > HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X > > based workstation and Raspberry Pi 4 (DT). I guess at the moment it > > would make most sense for special configs such as those tailored for > > vitualization guets but in the end that would be something for > > distributions to decide. > > IMO, it makes sense to have a "default y" there, as on systems that > support I/O space, disabling it will just randomly disable some drivers > that could be required by some hardware. I won't doubt that some of > those could be ported from using inb/outb to use, instead, readb/writeb. Makes sense, if these get more legacy over time we can always change the default. This would also mean we don't need to change defconfigs that include legacy PCI devices. > > > Arnd described the options here: > > https://lore.kernel.org/lkml/CAK8P3a3HHeP+Gw_k2P7Qtig0OmErf0HN30G22+qHic_uZTh11Q@mail.gmail.com/ > > Based on Arnd's description, LEGACY_PCI should depend on HAS_IOPORT. > This is missing on patch 1. You should probably reorder your patch > series to first create HAS_IOPORT and then add LEGACY_PCI with > depends on, as otherwise it may cause randconfig build issues > at robots and/or git bisect. > > I would also suggest to first introduce such change and then send > a per-subsystem LEGACY_PCI patch, as it would be a lot easier for > maintainers to review. Playing around with the reordering I think it might make sense to introduce HAS_IOPORT in patch 01, then LEGACY_PCI in patch 02 and then add dependencies for both on a per subsystem basis. I think it would be overkill to have two series of per subsystem patches. > > > > > > > > This > > > > includes pre-PCIe devices as well as PCIe devices which require > > > > features like I/O spaces. The "legacy" naming is comes from the PCIe > > > > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for > > > > compatibility with legacy devices which require their use. Future > > > > revisions of this specification may deprecate the use of I/O Space." > > > > > > I would still avoid calling it LEGACY_PCI, as this sounds too generic. > > > > > > I didn't read the PCI/PCIe specs, but I suspect that are a lot more > > > features that were/will be deprecated on PCI specs as time goes by. > > > > > > So, I would, instead, use something like PCI_LEGACY_IO_SPACE or > > > HAVE_PCI_LEGACY_IO_SPACE, in order to let it clear what "legacy" > > > means. > > > > Hmm, I'd like to hear Bjorn's opinion on this. Personally I feel like > > LEGACY_PCI is pretty clear since most devices are either pre-PCIe > > devices or a compatibility feature allowing drivers for a pre-PCIe > > device to work with a PCIe device. > > That's the main point: it is *not* disabling pre-PCIe devices or > even legacy PCI drivers. It just disables a random set of drivers just > because they use inb/outb instead of readb/writeb. It keeps several pure > PCI drivers selected, and disables some PCIe for no real reason. That is not intentional. The dependencies are certainly not perfect yet which is one of the reasons this is still an RFC. I hope getting these right will be a lot easier if we do both LEGACY_PCI and HAS_IOPORT dependency selection on a per subsystem basis. > > Just to give one example, this symbol: > > > diff --git a/drivers/media/cec/platform/Kconfig b/drivers/media/cec/platform/Kconfig > > index b672d3142eb7..5e92ece5b104 100644 > > --- a/drivers/media/cec/platform/Kconfig > > +++ b/drivers/media/cec/platform/Kconfig > > @@ -100,7 +100,7 @@ config CEC_TEGRA > > config CEC_SECO > > tristate "SECO Boards HDMI CEC driver" > > depends on (X86 || IA64) || COMPILE_TEST > > - depends on PCI && DMI > > + depends on LEGACY_PCI && DMI > > select CEC_CORE > > select CEC_NOTIFIER > > help > > Disables HDMI CEC support on some Intel motherboards. > Any distro meant to run on generic hardware should keep it selected. As far as I can see this one actually uses a hardcoded I/O port numbers and googling it looks like it's an on-board device on the UDOO x86 board. I guess that should indeed just be "depends on PCI && DMI && HAS_IOPORT". > > I can see some value of a "PCI_LEGACY" option to disable all > non-PCIe drivers, but this is not the case here. > > Thanks, > Mauro Ok, I think we definitely need to work on getting the dependencies right. I do think we agree that once done correctly there is value in such an option independent of HAS_IOPORT only gating inb() etc uses. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 40E59C433F5 for ; Thu, 30 Dec 2021 17:17:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C924310E219; Thu, 30 Dec 2021 17:17:02 +0000 (UTC) Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by gabe.freedesktop.org (Postfix) with ESMTPS id C4CC810E231 for ; Wed, 29 Dec 2021 11:46:03 +0000 (UTC) Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1BT8Pn7i011761; Wed, 29 Dec 2021 11:45:52 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : content-transfer-encoding : mime-version; s=pp1; bh=i5NP9XctF1wJk5OjQUo+Vc9uVmlIqZSnBT0u09LTmOk=; b=bFPRLC+jl+efdix4uwpI/mccWCcWO/G1P+1Qbs1rTtCAz+rovwnnR8pPTU6lF0Xc0joV YXPBuRb7BHXzyjRg7LITTk2SnNio9SeVldUJyNRqyoNhK4M9oyDLsc8IgyEZJiRqRm5A iHUmkTc7uG3NhK6BlA+KFy7MBz/iVluQaCQBkvCSyAyJcdcbD85ipXooRay1nJfQKYhr pBhuhwTvWtOhRjZ8iX5rVE8/8svHYNhhRkTQSfMP/PoiugU551FyKdeVPLCMOig/L1Pb f+O+aj84GXG4UtoEonKAgWOYfXmY61i73T8/AZi6WWLQ0MnyNiHoz+SQfgz7xDZNEMLG PQ== Received: from ppma01fra.de.ibm.com (46.49.7a9f.ip4.static.sl-reverse.com [159.122.73.70]) by mx0a-001b2d01.pphosted.com with ESMTP id 3d7rpyt5s9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:51 +0000 Received: from pps.filterd (ppma01fra.de.ibm.com [127.0.0.1]) by ppma01fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1BTBg94u031832; Wed, 29 Dec 2021 11:45:48 GMT Received: from b06avi18626390.portsmouth.uk.ibm.com (b06avi18626390.portsmouth.uk.ibm.com [9.149.26.192]) by ppma01fra.de.ibm.com with ESMTP id 3d5tx9cy34-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 29 Dec 2021 11:45:48 +0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06avi18626390.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1BTBbEnS47841776 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 29 Dec 2021 11:37:14 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1CC32AE055; Wed, 29 Dec 2021 11:45:45 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DA088AE053; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Received: from sig-9-145-13-177.uk.ibm.com (unknown [9.145.13.177]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 29 Dec 2021 11:45:38 +0000 (GMT) Message-ID: <8c59271de66dfa230142186d593f3501b8c789b1.camel@linux.ibm.com> Subject: Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI From: Niklas Schnelle To: Mauro Carvalho Chehab , Hans Verkuil , Ettore Chimenti Date: Wed, 29 Dec 2021 12:45:38 +0100 In-Reply-To: <20211228181107.2d476028@coco.lan> References: <20211227164317.4146918-1-schnelle@linux.ibm.com> <20211227164317.4146918-2-schnelle@linux.ibm.com> <20211228101435.3a55b983@coco.lan> <20211228135425.0a69168c@coco.lan> <4b630b7b87bd983291f628c42a1394fc0d2d86bd.camel@linux.ibm.com> <20211228181107.2d476028@coco.lan> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) X-TM-AS-GCONF: 00 X-Proofpoint-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn X-Proofpoint-ORIG-GUID: fDpUVSRh2Ka7hPzyqZG9djJVno3Sdwgn Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2021-12-29_04,2021-12-29_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxlogscore=999 spamscore=0 phishscore=0 adultscore=0 mlxscore=0 bulkscore=0 clxscore=1015 malwarescore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2112290062 X-Mailman-Approved-At: Thu, 30 Dec 2021 17:17:01 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, linux-pci@vger.kernel.org, dri-devel@lists.freedesktop.org, Jaroslav Kysela , linux-ide@vger.kernel.org, Jean Delvare , Guo Ren , linux-i2c@vger.kernel.org, linux-riscv@lists.infradead.org, Vincent Chen , Jiri Slaby , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Hannes Reinecke , Michael Grzeschik , linux-scsi@vger.kernel.org, Sumit Saxena , Damien Le Moal , Sathya Prakash , Jesse Brandeburg , linux-csky@vger.kernel.org, Kashyap Desai , Nilesh Javali , Bjorn Helgaas , linux-serial@vger.kernel.org, GR-QLogic-Storage-Upstream@marvell.com, Jakub Kicinski , MPT-FusionLinux.pdl@broadcom.com, "James E.J. Bottomley" , Guenter Roeck , linux-media@vger.kernel.org, linux-input@vger.kernel.org, Albert Ou , linux-watchdog@vger.kernel.org, Jouni Malinen , Suganath Prabu Subramani , Kalle Valo , John Garry , linux-spi@vger.kernel.org, linux-gpio@vger.kernel.org, Ian Abbott , Mark Brown , Greentime Hu , Paul Walmsley , Bjorn Helgaas , Wim Van Sebroeck , megaraidlinux.pdl@broadcom.com, Teddy Wang , linux-hwmon@vger.kernel.org, Arnd Bergmann , Karsten Keil , Sreekanth Reddy , "Martin K. Petersen" , Nick Hu , Sudip Mukherjee , Shivasharan S , Greg Kroah-Hartman , Dmitry Torokhov , linux-wireless@vger.kernel.org, Takashi Iwai , "David S. Miller" , H Hartley Sweeten , Palmer Dabbelt , Forest Bond , netdev@vger.kernel.org, Tony Nguyen , intel-wired-lan@lists.osuosl.org, Bartosz Golaszewski Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, 2021-12-28 at 18:12 +0100, Mauro Carvalho Chehab wrote: > Em Tue, 28 Dec 2021 16:06:44 +0100 > Niklas Schnelle escreveu: > > (on a side note: the c/c list of this patch is too long. I would try to > avoid using a too long list, as otherwise this e-mail may end being rejected > by mail servers) > > > On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote: > > > > > ---8<--- > > > > > > > > > All you really care about is the "legacy" I/O spaces here, this isn't > > > > > > tied to PCI specifically at all, right? > > > > > > > > > > > > So why not just have a OLD_STYLE_IO config option or something like > > > > > > that, to show that it's the i/o functions we care about here, not PCI at > > > > > > all? > > > > > > > > > > > > And maybe not call it "old" or "legacy" as time constantly goes forward, > > > > > > just describe it as it is, "DIRECT_IO"? > > > > > > > > > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate > > > > > name for it. > > > > > > > > > > Thanks, > > > > > Mauro > > > > > > > > Hmm, I might be missing something here but that sounds a lot like the > > > > HAS_IOPORT option added in patch 02. > > > > > > > > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two > > > > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card > > > > while LEGACY_PCI is for PCI drivers that require port I/O. > > > > > > I didn't look at the other patches on this series, but why it is needed > > > to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be enough? > > > > > > I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y > > > where LEGACY_PCI shall be "n"? > > > > In the current patch set LEGACY_PCI is not currently selected by > > architectures, though of course it could be if we know that an > > architecture requires it. We should probably also set it in any > > defconfig that has devices depending on it so as not to break these. > > > > Other than that it would be set during kernel configuration if one > > wants/needs support for legacy PCI devices. For testing I ran with > > HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X > > based workstation and Raspberry Pi 4 (DT). I guess at the moment it > > would make most sense for special configs such as those tailored for > > vitualization guets but in the end that would be something for > > distributions to decide. > > IMO, it makes sense to have a "default y" there, as on systems that > support I/O space, disabling it will just randomly disable some drivers > that could be required by some hardware. I won't doubt that some of > those could be ported from using inb/outb to use, instead, readb/writeb. Makes sense, if these get more legacy over time we can always change the default. This would also mean we don't need to change defconfigs that include legacy PCI devices. > > > Arnd described the options here: > > https://lore.kernel.org/lkml/CAK8P3a3HHeP+Gw_k2P7Qtig0OmErf0HN30G22+qHic_uZTh11Q@mail.gmail.com/ > > Based on Arnd's description, LEGACY_PCI should depend on HAS_IOPORT. > This is missing on patch 1. You should probably reorder your patch > series to first create HAS_IOPORT and then add LEGACY_PCI with > depends on, as otherwise it may cause randconfig build issues > at robots and/or git bisect. > > I would also suggest to first introduce such change and then send > a per-subsystem LEGACY_PCI patch, as it would be a lot easier for > maintainers to review. Playing around with the reordering I think it might make sense to introduce HAS_IOPORT in patch 01, then LEGACY_PCI in patch 02 and then add dependencies for both on a per subsystem basis. I think it would be overkill to have two series of per subsystem patches. > > > > > > > > This > > > > includes pre-PCIe devices as well as PCIe devices which require > > > > features like I/O spaces. The "legacy" naming is comes from the PCIe > > > > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for > > > > compatibility with legacy devices which require their use. Future > > > > revisions of this specification may deprecate the use of I/O Space." > > > > > > I would still avoid calling it LEGACY_PCI, as this sounds too generic. > > > > > > I didn't read the PCI/PCIe specs, but I suspect that are a lot more > > > features that were/will be deprecated on PCI specs as time goes by. > > > > > > So, I would, instead, use something like PCI_LEGACY_IO_SPACE or > > > HAVE_PCI_LEGACY_IO_SPACE, in order to let it clear what "legacy" > > > means. > > > > Hmm, I'd like to hear Bjorn's opinion on this. Personally I feel like > > LEGACY_PCI is pretty clear since most devices are either pre-PCIe > > devices or a compatibility feature allowing drivers for a pre-PCIe > > device to work with a PCIe device. > > That's the main point: it is *not* disabling pre-PCIe devices or > even legacy PCI drivers. It just disables a random set of drivers just > because they use inb/outb instead of readb/writeb. It keeps several pure > PCI drivers selected, and disables some PCIe for no real reason. That is not intentional. The dependencies are certainly not perfect yet which is one of the reasons this is still an RFC. I hope getting these right will be a lot easier if we do both LEGACY_PCI and HAS_IOPORT dependency selection on a per subsystem basis. > > Just to give one example, this symbol: > > > diff --git a/drivers/media/cec/platform/Kconfig b/drivers/media/cec/platform/Kconfig > > index b672d3142eb7..5e92ece5b104 100644 > > --- a/drivers/media/cec/platform/Kconfig > > +++ b/drivers/media/cec/platform/Kconfig > > @@ -100,7 +100,7 @@ config CEC_TEGRA > > config CEC_SECO > > tristate "SECO Boards HDMI CEC driver" > > depends on (X86 || IA64) || COMPILE_TEST > > - depends on PCI && DMI > > + depends on LEGACY_PCI && DMI > > select CEC_CORE > > select CEC_NOTIFIER > > help > > Disables HDMI CEC support on some Intel motherboards. > Any distro meant to run on generic hardware should keep it selected. As far as I can see this one actually uses a hardcoded I/O port numbers and googling it looks like it's an on-board device on the UDOO x86 board. I guess that should indeed just be "depends on PCI && DMI && HAS_IOPORT". > > I can see some value of a "PCI_LEGACY" option to disable all > non-PCIe drivers, but this is not the case here. > > Thanks, > Mauro Ok, I think we definitely need to work on getting the dependencies right. I do think we agree that once done correctly there is value in such an option independent of HAS_IOPORT only gating inb() etc uses. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Niklas Schnelle Date: Wed, 29 Dec 2021 12:45:38 +0100 Subject: [Intel-wired-lan] [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI In-Reply-To: <20211228181107.2d476028@coco.lan> References: <20211227164317.4146918-1-schnelle@linux.ibm.com> <20211227164317.4146918-2-schnelle@linux.ibm.com> <20211228101435.3a55b983@coco.lan> <20211228135425.0a69168c@coco.lan> <4b630b7b87bd983291f628c42a1394fc0d2d86bd.camel@linux.ibm.com> <20211228181107.2d476028@coco.lan> Message-ID: <8c59271de66dfa230142186d593f3501b8c789b1.camel@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On Tue, 2021-12-28 at 18:12 +0100, Mauro Carvalho Chehab wrote: > Em Tue, 28 Dec 2021 16:06:44 +0100 > Niklas Schnelle escreveu: > > (on a side note: the c/c list of this patch is too long. I would try to > avoid using a too long list, as otherwise this e-mail may end being rejected > by mail servers) > > > On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote: > > > > > ---8<--- > > > > > > > > > All you really care about is the "legacy" I/O spaces here, this isn't > > > > > > tied to PCI specifically at all, right? > > > > > > > > > > > > So why not just have a OLD_STYLE_IO config option or something like > > > > > > that, to show that it's the i/o functions we care about here, not PCI at > > > > > > all? > > > > > > > > > > > > And maybe not call it "old" or "legacy" as time constantly goes forward, > > > > > > just describe it as it is, "DIRECT_IO"? > > > > > > > > > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate > > > > > name for it. > > > > > > > > > > Thanks, > > > > > Mauro > > > > > > > > Hmm, I might be missing something here but that sounds a lot like the > > > > HAS_IOPORT option added in patch 02. > > > > > > > > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two > > > > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card > > > > while LEGACY_PCI is for PCI drivers that require port I/O. > > > > > > I didn't look at the other patches on this series, but why it is needed > > > to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be enough? > > > > > > I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y > > > where LEGACY_PCI shall be "n"? > > > > In the current patch set LEGACY_PCI is not currently selected by > > architectures, though of course it could be if we know that an > > architecture requires it. We should probably also set it in any > > defconfig that has devices depending on it so as not to break these. > > > > Other than that it would be set during kernel configuration if one > > wants/needs support for legacy PCI devices. For testing I ran with > > HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X > > based workstation and Raspberry Pi 4 (DT). I guess at the moment it > > would make most sense for special configs such as those tailored for > > vitualization guets but in the end that would be something for > > distributions to decide. > > IMO, it makes sense to have a "default y" there, as on systems that > support I/O space, disabling it will just randomly disable some drivers > that could be required by some hardware. I won't doubt that some of > those could be ported from using inb/outb to use, instead, readb/writeb. Makes sense, if these get more legacy over time we can always change the default. This would also mean we don't need to change defconfigs that include legacy PCI devices. > > > Arnd described the options here: > > https://lore.kernel.org/lkml/CAK8P3a3HHeP+Gw_k2P7Qtig0OmErf0HN30G22+qHic_uZTh11Q at mail.gmail.com/ > > Based on Arnd's description, LEGACY_PCI should depend on HAS_IOPORT. > This is missing on patch 1. You should probably reorder your patch > series to first create HAS_IOPORT and then add LEGACY_PCI with > depends on, as otherwise it may cause randconfig build issues > at robots and/or git bisect. > > I would also suggest to first introduce such change and then send > a per-subsystem LEGACY_PCI patch, as it would be a lot easier for > maintainers to review. Playing around with the reordering I think it might make sense to introduce HAS_IOPORT in patch 01, then LEGACY_PCI in patch 02 and then add dependencies for both on a per subsystem basis. I think it would be overkill to have two series of per subsystem patches. > > > > > > > > This > > > > includes pre-PCIe devices as well as PCIe devices which require > > > > features like I/O spaces. The "legacy" naming is comes from the PCIe > > > > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for > > > > compatibility with legacy devices which require their use. Future > > > > revisions of this specification may deprecate the use of I/O Space." > > > > > > I would still avoid calling it LEGACY_PCI, as this sounds too generic. > > > > > > I didn't read the PCI/PCIe specs, but I suspect that are a lot more > > > features that were/will be deprecated on PCI specs as time goes by. > > > > > > So, I would, instead, use something like PCI_LEGACY_IO_SPACE or > > > HAVE_PCI_LEGACY_IO_SPACE, in order to let it clear what "legacy" > > > means. > > > > Hmm, I'd like to hear Bjorn's opinion on this. Personally I feel like > > LEGACY_PCI is pretty clear since most devices are either pre-PCIe > > devices or a compatibility feature allowing drivers for a pre-PCIe > > device to work with a PCIe device. > > That's the main point: it is *not* disabling pre-PCIe devices or > even legacy PCI drivers. It just disables a random set of drivers just > because they use inb/outb instead of readb/writeb. It keeps several pure > PCI drivers selected, and disables some PCIe for no real reason. That is not intentional. The dependencies are certainly not perfect yet which is one of the reasons this is still an RFC. I hope getting these right will be a lot easier if we do both LEGACY_PCI and HAS_IOPORT dependency selection on a per subsystem basis. > > Just to give one example, this symbol: > > > diff --git a/drivers/media/cec/platform/Kconfig b/drivers/media/cec/platform/Kconfig > > index b672d3142eb7..5e92ece5b104 100644 > > --- a/drivers/media/cec/platform/Kconfig > > +++ b/drivers/media/cec/platform/Kconfig > > @@ -100,7 +100,7 @@ config CEC_TEGRA > > config CEC_SECO > > tristate "SECO Boards HDMI CEC driver" > > depends on (X86 || IA64) || COMPILE_TEST > > - depends on PCI && DMI > > + depends on LEGACY_PCI && DMI > > select CEC_CORE > > select CEC_NOTIFIER > > help > > Disables HDMI CEC support on some Intel motherboards. > Any distro meant to run on generic hardware should keep it selected. As far as I can see this one actually uses a hardcoded I/O port numbers and googling it looks like it's an on-board device on the UDOO x86 board. I guess that should indeed just be "depends on PCI && DMI && HAS_IOPORT". > > I can see some value of a "PCI_LEGACY" option to disable all > non-PCIe drivers, but this is not the case here. > > Thanks, > Mauro Ok, I think we definitely need to work on getting the dependencies right. I do think we agree that once done correctly there is value in such an option independent of HAS_IOPORT only gating inb() etc uses.