From: Marc Zyngier <maz@kernel.org> To: "Pali Rohár" <pali@kernel.org> Cc: "Marek Behún" <kabel@kernel.org>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Bjorn Helgaas" <helgaas@kernel.org>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 23/23] PCI: aardvark: Make main irq_chip structure a static driver structure Date: Mon, 10 Jan 2022 14:44:17 +0000 [thread overview] Message-ID: <87mtk3tzum.wl-maz@kernel.org> (raw) In-Reply-To: <20220110105324.jud6bzdtmoiuvyas@pali> On Mon, 10 Jan 2022 10:53:24 +0000, Pali Rohár <pali@kernel.org> wrote: > > On Monday 10 January 2022 09:28:39 Marc Zyngier wrote: > > On 2022-01-10 01:50, Marek Behún wrote: > > > Marc Zyngier says [1] that we should use struct irq_chip as a global > > > static struct in the driver. Even though the structure currently > > > contains a dynamic member (parent_device), Marc says [2] that he plans > > > to kill it and make the structure completely static. > > > > > > We have already converted others irq_chip structures in this driver in > > > this way, but we omitted this one because the .name member is > > > dynamically created from device's name, and the name is displayed in > > > sysfs, so changing it would break sysfs ABI. > > > > > > The rationale for changing the name (to "advk-INT") in spite of sysfs > > > ABI, and thus allowing to convert to a static structure, is that after > > > the other changes we made in this series, the IRQ chip is basically > > > something different: it no logner generates ERR and PME interrupts (they > > > are generated by emulated bridge's rp_irq_chip). > > > > There is no 'is spite of the ABI'. If you don't understand why > > we don't break the ABI, you have an even bigger problem. > > > > So NAK to this patch, now and forever. Any change to the structure to > > make it read-only must allow the preservation of the existing names > > when they are generated by the driver. > > Marc, you already presented that you do not like Armada 3720 platform > and that you do not care about it. What I like or not is irrelevant here. What I ask for is that userspace ABIs are not broken. > But please do not slowdown development for this platform. That's quite an accusation. > Arguments about ABIs, breaking it and similar are not relevant here as > this current kernel implementation is broken. And has to be replaced by > a working one. We are doing on it for more than year. > > It really does not make sense to try doing some backward compatibility > with something which is broken by design and does not work. It just take > lot of time without any value. > > We really need to more forward and fix driver as in current state is > PCIe on Armada 3720 unusable. This patch doesn't fix anything. It has the potential to break userspace, and I'm not having any of it. You may not care about backward compatibility, but this is thankfully *not* your pet playground. You can claim that I am doing a bad job. In which case, feel free to submit a patch removing me from the MAINTAINER file, and we can have that discussion. In the meantime, I will continue to oppose these kind of patches that pretend to 'fix' things without adding any value. M. -- Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org> To: "Pali Rohár" <pali@kernel.org> Cc: "Marek Behún" <kabel@kernel.org>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Bjorn Helgaas" <helgaas@kernel.org>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 23/23] PCI: aardvark: Make main irq_chip structure a static driver structure Date: Mon, 10 Jan 2022 14:44:17 +0000 [thread overview] Message-ID: <87mtk3tzum.wl-maz@kernel.org> (raw) In-Reply-To: <20220110105324.jud6bzdtmoiuvyas@pali> On Mon, 10 Jan 2022 10:53:24 +0000, Pali Rohár <pali@kernel.org> wrote: > > On Monday 10 January 2022 09:28:39 Marc Zyngier wrote: > > On 2022-01-10 01:50, Marek Behún wrote: > > > Marc Zyngier says [1] that we should use struct irq_chip as a global > > > static struct in the driver. Even though the structure currently > > > contains a dynamic member (parent_device), Marc says [2] that he plans > > > to kill it and make the structure completely static. > > > > > > We have already converted others irq_chip structures in this driver in > > > this way, but we omitted this one because the .name member is > > > dynamically created from device's name, and the name is displayed in > > > sysfs, so changing it would break sysfs ABI. > > > > > > The rationale for changing the name (to "advk-INT") in spite of sysfs > > > ABI, and thus allowing to convert to a static structure, is that after > > > the other changes we made in this series, the IRQ chip is basically > > > something different: it no logner generates ERR and PME interrupts (they > > > are generated by emulated bridge's rp_irq_chip). > > > > There is no 'is spite of the ABI'. If you don't understand why > > we don't break the ABI, you have an even bigger problem. > > > > So NAK to this patch, now and forever. Any change to the structure to > > make it read-only must allow the preservation of the existing names > > when they are generated by the driver. > > Marc, you already presented that you do not like Armada 3720 platform > and that you do not care about it. What I like or not is irrelevant here. What I ask for is that userspace ABIs are not broken. > But please do not slowdown development for this platform. That's quite an accusation. > Arguments about ABIs, breaking it and similar are not relevant here as > this current kernel implementation is broken. And has to be replaced by > a working one. We are doing on it for more than year. > > It really does not make sense to try doing some backward compatibility > with something which is broken by design and does not work. It just take > lot of time without any value. > > We really need to more forward and fix driver as in current state is > PCIe on Armada 3720 unusable. This patch doesn't fix anything. It has the potential to break userspace, and I'm not having any of it. You may not care about backward compatibility, but this is thankfully *not* your pet playground. You can claim that I am doing a bad job. In which case, feel free to submit a patch removing me from the MAINTAINER file, and we can have that discussion. In the meantime, I will continue to oppose these kind of patches that pretend to 'fix' things without adding any value. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-01-10 14:44 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-10 1:49 [PATCH v2 00/23] PCI: aardvark controller fixes BATCH 4 Marek Behún 2022-01-10 1:49 ` Marek Behún 2022-01-10 1:49 ` [PATCH v2 01/23] PCI: aardvark: Replace custom PCIE_CORE_INT_* macros with PCI_INTERRUPT_* Marek Behún 2022-01-10 1:49 ` Marek Behún 2022-01-10 17:07 ` Bjorn Helgaas 2022-01-10 17:07 ` Bjorn Helgaas 2022-01-10 1:49 ` [PATCH v2 02/23] PCI: aardvark: Fix reading MSI interrupt number Marek Behún 2022-01-10 1:49 ` Marek Behún 2022-02-04 17:24 ` Lorenzo Pieralisi 2022-02-04 17:24 ` Lorenzo Pieralisi 2022-02-05 10:53 ` Marc Zyngier 2022-02-05 10:53 ` Marc Zyngier 2022-01-10 1:49 ` [PATCH v2 03/23] PCI: aardvark: Fix support for MSI interrupts Marek Behún 2022-01-10 1:49 ` Marek Behún 2022-01-10 1:49 ` [PATCH v2 04/23] PCI: aardvark: Rewrite IRQ code to chained IRQ handler Marek Behún 2022-01-10 1:49 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 05/23] PCI: aardvark: Check return value of generic_handle_domain_irq() when processing INTx IRQ Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 06/23] PCI: aardvark: Make MSI irq_chip structures static driver structures Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 07/23] PCI: aardvark: Make msi_domain_info structure a static driver structure Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 08/23] PCI: aardvark: Use dev_fwnode() instead of of_node_to_fwnode(dev->of_node) Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 09/23] PCI: aardvark: Refactor unmasking summary MSI interrupt Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 10/23] PCI: aardvark: Add support for masking MSI interrupts Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 11/23] PCI: aardvark: Fix setting MSI address Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-02-17 17:14 ` Bjorn Helgaas 2022-02-17 17:14 ` Bjorn Helgaas 2022-02-18 14:43 ` Marek Behún 2022-02-18 14:43 ` Marek Behún 2022-02-23 18:13 ` Bjorn Helgaas 2022-02-23 18:13 ` Bjorn Helgaas 2022-02-24 12:59 ` Pali Rohár 2022-02-24 12:59 ` Pali Rohár 2022-02-24 19:43 ` Bjorn Helgaas 2022-02-24 19:43 ` Bjorn Helgaas 2022-01-10 1:50 ` [PATCH v2 12/23] PCI: aardvark: Enable MSI-X support Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 13/23] PCI: aardvark: Add support for ERR interrupt on emulated bridge Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 14/23] PCI: aardvark: Fix reading PCI_EXP_RTSTA_PME bit " Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 15/23] PCI: aardvark: Optimize writing PCI_EXP_RTCTL_PMEIE and PCI_EXP_RTSTA_PME " Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 16/23] PCI: aardvark: Add support for PME interrupts Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 17/23] PCI: aardvark: Fix support for PME requester on emulated bridge Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 18/23] PCI: aardvark: Use separate INTA interrupt for emulated root bridge Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 19/23] PCI: aardvark: Remove irq_mask_ack callback for INTx interrupts Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 20/23] PCI: aardvark: Don't mask irq when mapping Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 21/23] PCI: aardvark: Drop __maybe_unused from advk_pcie_disable_phy() Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 22/23] PCI: aardvark: Update comment about link going down after link-up Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 1:50 ` [PATCH v2 23/23] PCI: aardvark: Make main irq_chip structure a static driver structure Marek Behún 2022-01-10 1:50 ` Marek Behún 2022-01-10 9:28 ` Marc Zyngier 2022-01-10 9:28 ` Marc Zyngier 2022-01-10 10:23 ` Marek Behún 2022-01-10 10:23 ` Marek Behún 2022-01-10 10:53 ` Pali Rohár 2022-01-10 10:53 ` Pali Rohár 2022-01-10 14:44 ` Marc Zyngier [this message] 2022-01-10 14:44 ` Marc Zyngier 2022-01-10 15:19 ` Marek Behún 2022-01-10 15:19 ` Marek Behún 2022-02-08 10:50 ` (subset) [PATCH v2 00/23] PCI: aardvark controller fixes BATCH 4 Lorenzo Pieralisi 2022-02-08 10:50 ` Lorenzo Pieralisi
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=87mtk3tzum.wl-maz@kernel.org \ --to=maz@kernel.org \ --cc=helgaas@kernel.org \ --cc=kabel@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=pali@kernel.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: linkBe 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.