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: <20260106174348.GA365798@bhelgaas>
Date: Tue, 6 Jan 2026 11:43:48 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Manivannan Sadhasivam <mani@...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 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?