mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
@ 2025-09-22  7:12 Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  7:12 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

Hello,

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.

NOTE for MAINTAINERS: This series has the following dependencies:
1. The following commit in Linux-Next is a build-dependency for the
series:
https://github.com/ColinIanKing/linux-next/commit/8de1de5a3a8d42975953382068fb5195e9d6e6c6
Since the v1 series was based on linux-next, there were no build errors.
However, since this series is based on pci/next based on the feedback from
Manivannan Sadhasivam <mani@kernel.org> at:
https://lore.kernel.org/r/2gzqupa7i7qhiscwm4uin2jmdb6qowp55mzk7w4o3f73ob64e7@taf5vjd7lhc5/
without the aforementioned commit, build will fail.
2. The following patch series include fixes for the driver which are
required to verify the driver functionality:
https://lore.kernel.org/r/20250912100802.3136121-1-s-vadapalli@ti.com/

v2 of this series is at:
https://lore.kernel.org/r/20250912122356.3326888-1-s-vadapalli@ti.com/
Changes since v2:
- Patch 04/10 of the v2 series has been squashed into patch 02/10 of the
  v2 series. In the v3 series, the squashed patch is 02/04.
- Patches 03/10, 05/10, 06/10, 07/10 and 08/10 of the v2 series have been
  dropped. The reason for dropping them is that all of the aforementioned
  patches introduce helpers for cleanup on driver removal. Since Mani
  pointed out that the driver cannot be removed until the IRQ issues are
  fixed, the driver's remove function hasn't been updated and therefore
  the helpers have been discarded.
- The commit message of patch 09/10 of the v2 series has been updated,
  keeping it concise and focusing on the issue and the fix. Moreover, a
  'Fixes' tag has been included although a backport isn't necessary, in
  order to address Mani's feedback.
- All changes associated with driver removal in patch 10/10 of the v2
  series have been discarded.
- Patch relation between v2 and v3 series is as follows:
  v3 01/04 => v2 01/10
  v3 02/04 => v2 02/10 + 04/10
  v3 03/04 => v2 09/10
  v3 04/04 => v2 10/10

For testing the series, Linux has been built in the following manner:
1. Check out at commit dc72930fe22e Merge branch 'pci/misc' of pci/next
2. Apply commit and patch series mentioned as dependencies above.
3. Apply current series.
4. Build Linux with CONFIG_PCI_KEYSTONE, CONFIG_PCI_KEYSTONE_HOST and
   CONFIG_PCI_KEYSTONE_EP set to 'm'.

Series has been tested on AM654-EVM with an NVMe SSD connected to the
PCIe connector on the board and verifying that the NVMe SSD enumerates
successfully. Additionally, the 'hdparm' utility has been used to read
the NVMe SSD for verifying functionality. Test Logs:
https://gist.github.com/Siddharth-Vadapalli-at-TI/182a80bb43e9c407982f7674034a7c9d

Regards,
Siddharth.

Siddharth Vadapalli (4):
  PCI: Export pci_get_host_bridge_device() for use by pci-keystone
  PCI: dwc: Export dw_pcie_allocate_domains() and
    dw_pcie_ep_raise_msix_irq()
  PCI: keystone: Exit ks_pcie_probe() for invalid mode
  PCI: keystone: Add support to build as a loadable module

 drivers/pci/controller/dwc/Kconfig                | 6 +++---
 drivers/pci/controller/dwc/pci-keystone.c         | 8 ++++++++
 drivers/pci/controller/dwc/pcie-designware-ep.c   | 1 +
 drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
 drivers/pci/host-bridge.c                         | 1 +
 include/linux/pci.h                               | 1 +
 6 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone
  2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
@ 2025-09-22  7:12 ` Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  7:12 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

The pci-keystone.c driver uses the 'pci_get_host_bridge_device()' helper.
In preparation for enabling the pci-keystone.c driver to be built as a
loadable module, export 'pci_get_host_bridge_device()'.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

v2 of this patch is at:
https://lore.kernel.org/r/20250912122356.3326888-2-s-vadapalli@ti.com/
No changes since v2.

 drivers/pci/host-bridge.c | 1 +
 include/linux/pci.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index afa50b446567..be5ef6516cff 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -33,6 +33,7 @@ struct device *pci_get_host_bridge_device(struct pci_dev *dev)
 	kobject_get(&bridge->kobj);
 	return bridge;
 }
+EXPORT_SYMBOL_GPL(pci_get_host_bridge_device);
 
 void  pci_put_host_bridge_device(struct device *dev)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d1fdf81fbe1e..b253cbc27d36 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -646,6 +646,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv);
 struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
 						   size_t priv);
 void pci_free_host_bridge(struct pci_host_bridge *bridge);
+struct device *pci_get_host_bridge_device(struct pci_dev *dev);
 struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq()
  2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
@ 2025-09-22  7:12 ` Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  7:12 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

The pci-keystone.c driver uses the functions 'dw_pcie_allocate_domains()'
and 'dw_pcie_ep_raise_msix_irq()'. In preparation for enabling the
pci-keystone.c driver to be built as a loadable module, export them.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

This patch is a combination of patches 02/10 and 04/10 of the v2 series:
02/10: https://lore.kernel.org/r/20250912122356.3326888-3-s-vadapalli@ti.com/
04/10: https://lore.kernel.org/r/20250912122356.3326888-5-s-vadapalli@ti.com/
Except for merging the patches of the v2 series together, and updating the
commit message, no other changes have been made to get to this v3 patch.

 drivers/pci/controller/dwc/pcie-designware-ep.c   | 1 +
 drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 7f2112c2fb21..19571ac2b961 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -797,6 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msix_irq);
 
 /**
  * dw_pcie_ep_cleanup - Cleanup DWC EP resources after fundamental reset
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b501..3cc83d921376 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -229,6 +229,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_allocate_domains);
 
 void dw_pcie_free_msi(struct dw_pcie_rp *pp)
 {
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode
  2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
@ 2025-09-22  7:12 ` Siddharth Vadapalli
  2025-09-22  7:12 ` [PATCH v3 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
  2025-09-22  8:26 ` [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
  4 siblings, 0 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  7:12 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

Commit under Fixes introduced support for PCIe EP mode on AM654x platforms.
When the mode happens to be either "DW_PCIE_RC_TYPE" or "DW_PCIE_EP_TYPE",
the PCIe Controller is configured accordingly. However, when the mode is
neither of them, an error message is displayed but the driver probe
succeeds. Since this "invalid" mode is not associated with a functional
PCIe Controller, the probe should fail.

Fix the behavior by exiting "ks_pcie_probe()" with the return value of
"-EINVAL" in addition to displaying the existing error message when the
mode is invalid.

Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

v2 of this patch is at:
https://lore.kernel.org/r/20250912122356.3326888-10-s-vadapalli@ti.com/
Changes since v2:
- The commit subject and description has been updated to keep it concise
  and highlight the issue and the fix.
- A "Fixes" tag has been added but 'stable' hasn't been CCed on purpose
  since backporting the patch isn't required - doesn't enable
  functionality.

 drivers/pci/controller/dwc/pci-keystone.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index d03e95bf7d54..f9f8235ea3cd 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1337,6 +1337,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
 		break;
 	default:
 		dev_err(dev, "INVALID device type %d\n", mode);
+		ret = -EINVAL;
+		goto err_get_sync;
 	}
 
 	ks_pcie_enable_error_irq(ks_pcie);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 4/4] PCI: keystone: Add support to build as a loadable module
  2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
                   ` (2 preceding siblings ...)
  2025-09-22  7:12 ` [PATCH v3 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
@ 2025-09-22  7:12 ` Siddharth Vadapalli
  2025-09-22  8:26 ` [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
  4 siblings, 0 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  7:12 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

The 'pci-keystone.c' driver is the application/glue/wrapper driver for the
Designware PCIe Controllers on TI SoCs. Now that all of the helper APIs
that the 'pci-keystone.c' driver depends upon have been exported for use,
enable support to build the driver as a loadable module.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

v2 of this patch is at:
https://lore.kernel.org/r/20250912122356.3326888-11-s-vadapalli@ti.com/
Changes since v2:
- Based on Mani's feedback, all code changes associated with driver
  removal have been discarded.

 drivers/pci/controller/dwc/Kconfig        | 6 +++---
 drivers/pci/controller/dwc/pci-keystone.c | 6 ++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 34abc859c107..46012d6a607e 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -482,10 +482,10 @@ config PCI_DRA7XX_EP
 	  This uses the DesignWare core.
 
 config PCI_KEYSTONE
-	bool
+	tristate
 
 config PCI_KEYSTONE_HOST
-	bool "TI Keystone PCIe controller (host mode)"
+	tristate "TI Keystone PCIe controller (host mode)"
 	depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
 	depends on PCI_MSI
 	select PCIE_DW_HOST
@@ -497,7 +497,7 @@ config PCI_KEYSTONE_HOST
 	  DesignWare core functions to implement the driver.
 
 config PCI_KEYSTONE_EP
-	bool "TI Keystone PCIe controller (endpoint mode)"
+	tristate "TI Keystone PCIe controller (endpoint mode)"
 	depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
 	depends on PCI_ENDPOINT
 	select PCIE_DW_EP
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index f9f8235ea3cd..2fbc714bb6e5 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -17,6 +17,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/mfd/syscon.h>
+#include <linux/module.h>
 #include <linux/msi.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -1134,6 +1135,7 @@ static const struct of_device_id ks_pcie_of_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, ks_pcie_of_match);
 
 static int ks_pcie_probe(struct platform_device *pdev)
 {
@@ -1382,3 +1384,7 @@ static struct platform_driver ks_pcie_driver = {
 	},
 };
 builtin_platform_driver(ks_pcie_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PCIe host controller driver for Texas Instruments Keystone SoCs");
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
  2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
                   ` (3 preceding siblings ...)
  2025-09-22  7:12 ` [PATCH v3 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
@ 2025-09-22  8:26 ` Manivannan Sadhasivam
  2025-09-22  8:32   ` Manivannan Sadhasivam
  4 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-22  8:26 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby,
	Siddharth Vadapalli
  Cc: Manivannan Sadhasivam, linux-pci, linux-kernel, linux-arm-kernel, srk


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

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
  2025-09-22  8:26 ` [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
@ 2025-09-22  8:32   ` Manivannan Sadhasivam
  2025-09-22  9:25     ` Siddharth Vadapalli
  0 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-22  8:32 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby,
	Siddharth Vadapalli
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk

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.

So I'm dropping this series now. Please repost once the fix is in mainline
(which will be next cycle).

- Mani

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
  2025-09-22  8:32   ` Manivannan Sadhasivam
@ 2025-09-22  9:25     ` Siddharth Vadapalli
  2025-09-22 11:17       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22  9:25 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby,
	Siddharth Vadapalli, linux-pci, linux-kernel, linux-arm-kernel,
	srk

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.

> 
> So I'm dropping this series now. Please repost once the fix is in mainline
> (which will be next cycle).

Sure, I will repost the series. I had posted the v3 series right away
to make it easier for you and other reviewers to provide feedback before
people lose context. Thank you for actively reviewing this series and
sharing your valuable feedback :)

Regards,
Siddharth.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
  2025-09-22  9:25     ` Siddharth Vadapalli
@ 2025-09-22 11:17       ` Manivannan Sadhasivam
  2025-09-22 12:49         ` Siddharth Vadapalli
  0 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-22 11:17 UTC (permalink / raw)
  To: Siddharth Vadapalli
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, quic_wenbyao, inochiama, mayank.rana,
	thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby,
	linux-pci, linux-kernel, linux-arm-kernel, srk

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] PCI: Keystone: Enable loadable module support
  2025-09-22 11:17       ` Manivannan Sadhasivam
@ 2025-09-22 12:49         ` Siddharth Vadapalli
  0 siblings, 0 replies; 10+ messages in thread
From: Siddharth Vadapalli @ 2025-09-22 12:49 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Siddharth Vadapalli, lpieralisi, kwilczynski, robh, bhelgaas,
	jingoohan1, christian.bruel, quic_wenbyao, inochiama,
	mayank.rana, thippeswamy.havalige, shradha.t, cassel, kishon,
	sergio.paracuellos, 18255117159, rongqianfeng, jirislaby,
	linux-pci, linux-kernel, linux-arm-kernel, srk

On Mon, Sep 22, 2025 at 04:47:39PM +0530, Manivannan Sadhasivam wrote:
> 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
> ==========

I will follow this format in the future. Thank you for the suggestion.

Regards,
Siddharth.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-22 12:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-22  7:12 [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
2025-09-22  7:12 ` [PATCH v3 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
2025-09-22  7:12 ` [PATCH v3 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
2025-09-22  7:12 ` [PATCH v3 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
2025-09-22  7:12 ` [PATCH v3 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
2025-09-22  8:26 ` [PATCH v3 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
2025-09-22  8:32   ` Manivannan Sadhasivam
2025-09-22  9:25     ` Siddharth Vadapalli
2025-09-22 11:17       ` Manivannan Sadhasivam
2025-09-22 12:49         ` Siddharth Vadapalli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®