linux-kernel - Re: [PATCH v9 2/2] PCI: eic7700: Add Eswin PCIe host controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cl2gdo6ledszdkubyimullksobtzuh4stplrctrsjxvi4u4xc4@s5daay4pwukx>
Date: Wed, 21 Jan 2026 18:21:45 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: zhangsenchuan <zhangsenchuan@...incomputing.com>, bhelgaas@...gle.com, 
	krzk+dt@...nel.org, conor+dt@...nel.org, lpieralisi@...nel.org, 
	kwilczynski@...nel.org, robh@...nel.org, p.zabel@...gutronix.de, jingoohan1@...il.com, 
	gustavo.pimentel@...opsys.com, linux-pci@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, christian.bruel@...s.st.com, mayank.rana@....qualcomm.com, 
	shradha.t@...sung.com, krishna.chundru@....qualcomm.com, thippeswamy.havalige@....com, 
	inochiama@...il.com, Frank.li@....com, ningyu@...incomputing.com, 
	linmin@...incomputing.com, pinkesh.vaghela@...fochips.com, ouyanghui@...incomputing.com, 
	Niklas Cassel <cassel@...nel.org>
Subject: Re: [PATCH v9 2/2] PCI: eic7700: Add Eswin PCIe host controller
 driver

On Tue, Jan 06, 2026 at 11:43:48AM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 06, 2026 at 06:49:58PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Jan 06, 2026 at 08:43:11PM +0800, zhangsenchuan wrote:
> > > > On Mon, Dec 29, 2025 at 07:32:07PM +0800, zhangsenchuan@...incomputing.com wrote:
> > > > > From: Senchuan Zhang <zhangsenchuan@...incomputing.com>
> > > > > 
> > > > > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > > > > the DesignWare PCIe core, IP revision 5.96a. The PCIe Gen.3 controller
> > > > > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > > > > interrupts.
> > > > 
> > > > > +config PCIE_EIC7700
> > > > > +	tristate "Eswin EIC7700 PCIe controller"
> > > > 
> > > > > +/* Vendor and device ID value */
> > > > > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > > > > +#define PCI_DEVICE_ID_ESWIN		0x2030
> > > > 
> > > > Usually the device name is a little more than just the vendor.  What
> > > > if Eswin ever makes a second device?
> > > 
> > > Okey, thanks.
> > > Perhaps it's a problem. Maybe PCI_DEVICE_ID_EIC7700 is better?
> 
> Check pci_ids.h and follow the style used there.  Device ID macros
> typically include both the vendor and the device.
> 
> > > > > +static struct platform_driver eic7700_pcie_driver = {
> > > > > +	.probe = eic7700_pcie_probe,
> > > > 
> > > > This driver is tristate but has no .remove() callback.  Seems like it
> > > > should have one?
> > > 
> > > In v2 patch, I referred to Mani's comments and removed the .remove()
> > > callback, as follows:
> > > "Since this controller implements irqchip using the DWC core driver,
> > > it is not safe to remove it during runtime."
> > > https://lore.kernel.org/linux-pci/jghozurjqyhmtunivotitgs67h6xo4sb46qcycnbbwyvjcm4ek@vgq75olazmoi/
> > > 
> > > In addition, remove .remove() callback, because this driver has been 
> > > modified to builtin_platform_driver and does not support HotPlug, 
> > > therefore, the .remove() callback is not needed. Do you have any
> > > better suggestions?
> > 
> > Yes, builtin_platform_driver() wouldn't allow the users to remove
> > the module. So remove() callback will become useless. The reason why
> > this driver is tristate is that it could be loaded from rootfs and
> > not always statically built to the kernel image.
> 
> This .remove() vs IRQ thing is a perennial issue and it's hard to know
> what style new drivers should copy.
> 
> There are lots of DWC-based drivers that are tristate, implement
> .remove(), and use module_platform_driver() (e.g., bt1, kirin,
> tegra194, rcar-gen4, exynos, k1, stm32).  Is there something different
> about the way they implement irqchip that makes .remove() safe?

Yes, there are differences currently. Mostly due to some drivers missed the
IRQ maintainers eyes when they got added (way before my involvement with PCI
controller drivers). I will fix them to maintain uniformity.

But the general undocumented rule is that if the controller driver implement
any irqchip (MSI/MSI-X/INTx), they should not get removed. So they can be
tristate, but builtin_platform_driver(). If they use any external irqchip
controller for receiving interrupts, they can safely be removed. This is the
case for dwc/pcie-stm32.

This prompts me to write the controller driver documentation that I was planning
for a while...

- Mani

-- 
மணிவண்ணன் சதாசிவம்