linux-kernel - Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support

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: <qdz2d57q3hyosmvh7xzxy2qdhpjyxkl2mh6dr4or4nj4qakpoh@gg6ihzpaynst>
Date: Mon, 22 Sep 2025 16:47:39 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Siddharth Vadapalli <s-vadapalli@...com>
Cc: lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, 
	bhelgaas@...gle.com, jingoohan1@...il.com, christian.bruel@...s.st.com, 
	quic_wenbyao@...cinc.com, inochiama@...il.com, mayank.rana@....qualcomm.com, 
	thippeswamy.havalige@....com, shradha.t@...sung.com, cassel@...nel.org, kishon@...nel.org, 
	sergio.paracuellos@...il.com, 18255117159@....com, rongqianfeng@...o.com, jirislaby@...nel.org, 
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, srk@...com
Subject: Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support

On Mon, Sep 22, 2025 at 02:55:05PM +0530, Siddharth Vadapalli wrote:
> On Mon, Sep 22, 2025 at 02:02:43PM +0530, Manivannan Sadhasivam wrote:
> 
> Hello Mani,
> 
> > On Mon, Sep 22, 2025 at 01:56:08PM +0530, Manivannan Sadhasivam wrote:
> > > 
> > > On Mon, 22 Sep 2025 12:42:12 +0530, Siddharth Vadapalli wrote:
> > > > This series enables support for the 'pci-keystone.c' driver to be built
> > > > as a loadable module. The motivation for the series is that PCIe is not
> > > > a necessity for booting Linux due to which the 'pci-keystone.c' driver
> > > > does not need to be built-in.
> > > > 
> > > > Series is based on commit
> > > > dc72930fe22e Merge branch 'pci/misc'
> > > > of pci/next.
> > > > 
> > > > [...]
> > > 
> > > Applied, thanks!
> > > 
> > > [1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone
> > >       commit: c514ba0fa8938ae09370beecb77257868c1568a7
> > > [2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq()
> > >       commit: db9ff606a5535aee94bf41682f03aba500ff3ad6
> > > [3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode
> > >       commit: 76d23c87a3e06af003ae3a08053279d06141c716
> > > [4/4] PCI: keystone: Add support to build as a loadable module
> > >       commit: e82d56b5f3844189f2b2240b1c3eaeeafc8f1fd2
> > > 
> > 
> > I just noticed the build dependency mentioned in the cover letter after applying
> > the series. This is problematic since there is no guarantee that the dependent
> > commit will reach mainline first. So if this series gets applied by Linus first,
> > then building this driver as module will break the build. We should not have the
> > build error at any cost.
> 
> As feedback for the future, is there a better way that I could have
> highlighted the build dependency? I agree that a build failure is
> unacceptable which is why I tried to highlight the dependency, but, it
> probably wasn't the best approach to point it out by mentioning it in
> the cover letter. Please let me know if I could make it easier for you
> and other Maintainers to notice such stated dependencies.
> 

Mentioning the build dependency in the cover letter is the right thing to do.
But somehow I failed to spot it as it was not highlighted enough (just for my
eyes).

Maybe you could mention the dependencies under a sub-section. Like,

Dependency
==========

Some people also mark the patches as DNM (Do Not Merge), but that's for patches
not intended to be merged as is. Not for this series though.

Anyhow, I take the blame of not going through the cover letter properly, but you
did the right thing.

- Mani

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