[PATCH v3 0/4] PCI: Keystone: Enable loadable module support

[Manivannan Sadhasivam]

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

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