[PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Vidya Sagar]

To support the Hot-plug feature, PCIe spec has a well-defined model for 
hardware implementation and software programming interface. There are also
some architectures/platforms where the Hot-plug feature is implemented in a
non-standard way and software support for the respective implementations is
available with the kernel. This patch series attempts to add support for one
such non-standard way of supporting the Hot-plug feature where a single GPIO
is used to detect and report the Hot-Plug and Unplug events to the SW.
The platforms that can use this piece of software need to have GPIO routed
from the slot to the controller which can indicate the presence/absence of
the downstream device through its state. This GPIO should also have the
capability to interrupt the system when the connection/disconnection event
takes place.
A GPIO Hot-plug framework is written which looks for a "hotplug-gpios" named
GPIO entry in the corresponding device-tree entry of the controller and
registers a hot-pluggable slot with the Hot-plug framework.
The platform drivers of the PCIe host bridges/root ports can register with the
aforementioned GPIO Hot-Plug framework along with ops to perform any platform
specific tasks during Hot-Plug/Unplug events.

Oza Pawandeep made an attempt to upstream support for a similar Hot-plug
feature implementation at a platform level, but the implementation as such
was very specific to that platform (at least the way I understood it).
https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-2-git-send-email-oza.oza@broadcom.com/
https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-3-git-send-email-oza.oza@broadcom.com/
https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-4-git-send-email-oza.oza@broadcom.com/
This current series also attempts to address that by extracting out all the
common code to do with GPIO and Hot-plug core framework and expecting the
platform drivers to only register/unregister with the GPIO framework. So,
@Oza, could you try using the GPIO framework from this series and enable
Hot-plug support for your platform if it still makes sense?

@Rob,
Regarding the DT documentation change to add about 'hotplug-gpios, I'm not
sure if pci.txt is the right place or the dt-schema repository
i.e https://github.com/devicetree-org/dt-schema
But, in the interest of keeping all the changes related to this feature in the
the same repository, I made the changes to the pci.txt file in this repo itself.
Please let me know if the documentation change needs to be moved to the other
repo.

The Changes have been tested on the Tegra234 platform.

Vidya Sagar (4):
  dt-bindings: Add "hotplug-gpios" PCIe property
  PCI/hotplug: Add GPIO PCIe hotplug driver
  PCI: tegra194: Add support to configure a pluggable slot
  PCI: tegra194: Enable GPIO based Hot-Plug support

 Documentation/devicetree/bindings/pci/pci.txt |   4 +
 drivers/pci/controller/dwc/pcie-tegra194.c    |  85 +++++++-
 drivers/pci/hotplug/Kconfig                   |  11 +
 drivers/pci/hotplug/Makefile                  |   1 +
 drivers/pci/hotplug/gpio_php.c                | 200 ++++++++++++++++++
 drivers/pci/hotplug/gpiophp.h                 |  40 ++++
 6 files changed, 334 insertions(+), 7 deletions(-)
 create mode 100644 drivers/pci/hotplug/gpio_php.c
 create mode 100644 drivers/pci/hotplug/gpiophp.h

-- 
2.17.1

[PATCH V1 1/4] dt-bindings: Add "hotplug-gpios" PCIe property

[Vidya Sagar]

Provide a way for the firmware to tell the OS about the GPIO that can be
used to get the Hot-Plug and Unplug events.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 Documentation/devicetree/bindings/pci/pci.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
index 6a8f2874a24d..7b89bddc8dde 100644
--- a/Documentation/devicetree/bindings/pci/pci.txt
+++ b/Documentation/devicetree/bindings/pci/pci.txt
@@ -32,6 +32,10 @@ driver implementation may support the following properties:
    root port to downstream device and host bridge drivers can do programming
    which depends on CLKREQ signal existence. For example, programming root port
    not to advertise ASPM L1 Sub-States support if there is no CLKREQ signal.
+- hotplug-gpios:
+   If present this property specifies the GPIO to be used for Hot-Plug/Unplug
+   functionality. It is used by the PCIe GPIO Hot-Plug core driver for
+   PCIe device Hot-Plug/Unplug events.
 
 PCI-PCI Bridge properties
 -------------------------
-- 
2.17.1

Re: [PATCH V1 1/4] dt-bindings: Add "hotplug-gpios" PCIe property

[Lukas Wunner]

On Sat, Oct 01, 2022 at 12:57:44AM +0530, Vidya Sagar wrote:
> Provide a way for the firmware to tell the OS about the GPIO that can be
> used to get the Hot-Plug and Unplug events.
[...]
> --- a/Documentation/devicetree/bindings/pci/pci.txt
> +++ b/Documentation/devicetree/bindings/pci/pci.txt
> @@ -32,6 +32,10 @@ driver implementation may support the following properties:
>     root port to downstream device and host bridge drivers can do programming
>     which depends on CLKREQ signal existence. For example, programming root port
>     not to advertise ASPM L1 Sub-States support if there is no CLKREQ signal.
> +- hotplug-gpios:
> +   If present this property specifies the GPIO to be used for Hot-Plug/Unplug
> +   functionality. It is used by the PCIe GPIO Hot-Plug core driver for
> +   PCIe device Hot-Plug/Unplug events.

Please specify the GPIO's semantics in more detail:
Is the pin high as long as presence of a card is detected?
Or does it pulse when a hotplug/unplug event occurs?

Thanks,

Lukas

Re: [PATCH V1 1/4] dt-bindings: Add "hotplug-gpios" PCIe property

[Pali Rohár]

On Saturday 01 October 2022 17:56:26 Lukas Wunner wrote:
> On Sat, Oct 01, 2022 at 12:57:44AM +0530, Vidya Sagar wrote:
> > Provide a way for the firmware to tell the OS about the GPIO that can be
> > used to get the Hot-Plug and Unplug events.
> [...]
> > --- a/Documentation/devicetree/bindings/pci/pci.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci.txt
> > @@ -32,6 +32,10 @@ driver implementation may support the following properties:
> >     root port to downstream device and host bridge drivers can do programming
> >     which depends on CLKREQ signal existence. For example, programming root port
> >     not to advertise ASPM L1 Sub-States support if there is no CLKREQ signal.
> > +- hotplug-gpios:
> > +   If present this property specifies the GPIO to be used for Hot-Plug/Unplug
> > +   functionality. It is used by the PCIe GPIO Hot-Plug core driver for
> > +   PCIe device Hot-Plug/Unplug events.
> 
> Please specify the GPIO's semantics in more detail:
> Is the pin high as long as presence of a card is detected?

Hello! In PCIe is this semantics called "Presence Detect" (see PCIe Slot
Capabilities). So should it be rather named "presence-detect-gpios"
instead of hotplug?

> Or does it pulse when a hotplug/unplug event occurs?
> 
> Thanks,
> 
> Lukas

[PATCH V1 2/4] PCI/hotplug: Add GPIO PCIe hotplug driver

[Vidya Sagar]

This adds a standalone driver to support PCIe hotplug functionality
merely based on a GPIO indicating the status of a downstream device
connectivity. It looks for "hotplug-gpios" property in the corresponding
device node to get the GPIO information.

It also provides a mechanism for platform drivers of the controllers
to register ops to perform any platform specific operations while
enabling/disabling the slots.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/hotplug/Kconfig    |  11 ++
 drivers/pci/hotplug/Makefile   |   1 +
 drivers/pci/hotplug/gpio_php.c | 200 +++++++++++++++++++++++++++++++++
 drivers/pci/hotplug/gpiophp.h  |  40 +++++++
 4 files changed, 252 insertions(+)
 create mode 100644 drivers/pci/hotplug/gpio_php.c
 create mode 100644 drivers/pci/hotplug/gpiophp.h

diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index 840a84bb5ee2..dbac64ff6915 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -158,4 +158,15 @@ config HOTPLUG_PCI_S390
 
 	  When in doubt, say Y.
 
+config HOTPLUG_PCI_GPIO
+	bool "GPIO based PCI Hotplug Support"
+	depends on OF_GPIO
+	help
+	  Say Y here if you want to have GPIO based PCIe hot-plug framework.
+	  This framework helps to register a GPIO based Hot-Plug controller
+	  with the system where a GPIO can be used to represent device
+	  connect and disconnect states.
+
+	  When in doubt, say N.
+
 endif # HOTPLUG_PCI
diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index 5196983220df..70e11d698807 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_HOTPLUG_PCI_RPA)		+= rpaphp.o
 obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR)	+= rpadlpar_io.o
 obj-$(CONFIG_HOTPLUG_PCI_ACPI)		+= acpiphp.o
 obj-$(CONFIG_HOTPLUG_PCI_S390)		+= s390_pci_hpc.o
+obj-$(CONFIG_HOTPLUG_PCI_GPIO)		+= gpio_php.o
 
 # acpiphp_ibm extends acpiphp, so should be linked afterwards.
 
diff --git a/drivers/pci/hotplug/gpio_php.c b/drivers/pci/hotplug/gpio_php.c
new file mode 100644
index 000000000000..33c8105aade5
--- /dev/null
+++ b/drivers/pci/hotplug/gpio_php.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * GPIO based PCI Hotplug Driver.
+ *
+ */
+
+#include <linux/libfdt.h>
+#include <linux/module.h>
+#include <linux/of_fdt.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include "../pci.h"
+#include "../pcie/portdrv.h"
+
+#include "gpiophp.h"
+
+static DEFINE_MUTEX(slot_mutex);
+
+static inline struct gpio_hotplug_slot *to_gpio_hotplug_slot(struct hotplug_slot *slot)
+{
+	return container_of(slot, struct gpio_hotplug_slot, hotplug_slot);
+}
+
+static int gpio_hp_get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
+{
+	struct gpio_hotplug_slot *slot = to_gpio_hotplug_slot(hotplug_slot);
+	struct pci_dev *dev;
+
+	pci_config_pm_runtime_get(slot->pdev);
+	dev = pci_get_slot(slot->pdev->subordinate, PCI_DEVFN(0, 0));
+	if (dev) {
+		pci_dev_put(dev);
+		*value = 1;
+	} else {
+		*value = 0;
+	}
+	pci_dbg(slot->pdev, "Power Status = %u\n", *value);
+	pci_config_pm_runtime_put(slot->pdev);
+
+	return 0;
+}
+
+static int gpio_hp_enable_slot(struct hotplug_slot *hotplug_slot)
+{
+	struct gpio_hotplug_slot *slot = to_gpio_hotplug_slot(hotplug_slot);
+	int ret = 0;
+	u8 value;
+
+	mutex_lock(&slot_mutex);
+
+	gpio_hp_get_power_status(hotplug_slot, &value);
+	if (value) {
+		pci_info(slot->pdev, "Device is already plugged-in\n");
+		goto exit;
+	}
+
+	if (slot->plat_ops && slot->plat_ops->enable)
+		slot->plat_ops->enable(slot);
+
+	pm_runtime_get_sync(&slot->pdev->dev);
+
+	pci_lock_rescan_remove();
+	pci_rescan_bus(slot->pdev->bus);
+	pci_unlock_rescan_remove();
+
+	pm_runtime_put(&slot->pdev->dev);
+
+exit:
+	mutex_unlock(&slot_mutex);
+	return ret;
+}
+
+static int gpio_hp_disable_slot(struct hotplug_slot *hotplug_slot)
+{
+	struct gpio_hotplug_slot *slot = to_gpio_hotplug_slot(hotplug_slot);
+	struct pci_dev *dev, *temp;
+	u8 value;
+
+	mutex_lock(&slot_mutex);
+
+	gpio_hp_get_power_status(hotplug_slot, &value);
+	if (!value) {
+		pci_info(slot->pdev, "Device is already removed\n");
+		goto exit;
+	}
+
+	pci_lock_rescan_remove();
+
+	list_for_each_entry_safe_reverse(dev, temp, &slot->pdev->subordinate->devices, bus_list) {
+		pci_dev_get(dev);
+		pci_stop_and_remove_bus_device(dev);
+		pci_dev_put(dev);
+	}
+
+	pci_unlock_rescan_remove();
+
+exit:
+	if (slot->plat_ops && slot->plat_ops->disable)
+		slot->plat_ops->disable(slot);
+	mutex_unlock(&slot_mutex);
+	return 0;
+}
+
+static const struct hotplug_slot_ops gpio_hotplug_slot_ops = {
+	.enable_slot = gpio_hp_enable_slot,
+	.disable_slot = gpio_hp_disable_slot,
+	.get_power_status = gpio_hp_get_power_status,
+};
+
+static irqreturn_t pcie_gpio_hp_irq(int irq, void *arg)
+{
+	struct gpio_hotplug_slot *slot = arg;
+
+	if (gpiod_get_value(slot->gpiod)) {
+		pci_dbg(slot->pdev, "Hot-Plug Event\n");
+		gpio_hp_enable_slot(&slot->hotplug_slot);
+	} else {
+		pci_dbg(slot->pdev, "Hot-UnPlug Event\n");
+		gpio_hp_disable_slot(&slot->hotplug_slot);
+	}
+
+	return IRQ_HANDLED;
+}
+
+int register_gpio_hotplug_slot(struct gpio_hotplug_slot *slot)
+{
+	struct device *dev = &slot->pdev->dev;
+	struct pci_dev *pdev = slot->pdev;
+	struct gpio_desc *gpiod;
+	unsigned int irq;
+	char *name;
+	int ret;
+
+	gpiod = devm_gpiod_get(&pdev->bus->dev, "hotplug", GPIOD_IN);
+	if (IS_ERR(gpiod)) {
+		ret = PTR_ERR(gpiod);
+		pci_err(pdev, "Failed to find GPIO for Hot-Plug functionality: %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_to_irq(gpiod);
+	if (ret < 0) {
+		pci_err(pdev, "Failed to get IRQ for Hot_Plug GPIO: %d\n", ret);
+		return ret;
+	}
+	irq = (unsigned int)ret;
+
+	slot->gpiod = gpiod;
+	slot->hotplug_slot.ops = &gpio_hotplug_slot_ops;
+	slot->irq = irq;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "slot_%u", slot->slot_nr);
+	if (!name) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	ret = pci_hp_register(&slot->hotplug_slot, pdev->subordinate, 0, name);
+	if (ret) {
+		pci_err(pdev, "Failed to register hotplug slot: %d\n", ret);
+		goto exit;
+	}
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "pcie_gpio_hp_irq_%u", slot->slot_nr);
+	if (!name) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	ret = devm_request_threaded_irq(dev, slot->irq,
+					NULL, pcie_gpio_hp_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					name, slot);
+	if (ret < 0) {
+		pci_err(pdev, "Failed to request IRQ for Hot-Plug: %d\n", ret);
+		goto exit;
+	}
+
+	pci_dbg(pdev, "Hot-Plug Slot registered for %s\n", pci_name(pdev));
+
+	gpio_hp_enable_slot(&slot->hotplug_slot);
+
+	return 0;
+
+exit:
+	return ret;
+}
+
+void unregister_gpio_hotplug_slot(struct gpio_hotplug_slot *slot)
+{
+	struct device *dev = &slot->pdev->dev;
+	struct pci_dev *pdev = slot->pdev;
+
+	gpio_hp_disable_slot(&slot->hotplug_slot);
+	devm_free_irq(dev, slot->irq, slot);
+	pci_hp_deregister(&slot->hotplug_slot);
+	devm_gpiod_put(&pdev->bus->dev, slot->gpiod);
+}
+
diff --git a/drivers/pci/hotplug/gpiophp.h b/drivers/pci/hotplug/gpiophp.h
new file mode 100644
index 000000000000..77cc76976e0d
--- /dev/null
+++ b/drivers/pci/hotplug/gpiophp.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * GPIO based PCI Hotplug Driver.
+ *
+ */
+
+#ifndef _GPIOPHP_H
+#define _GPIOPHP_H
+
+#include <linux/pci_hotplug.h>
+
+struct gpio_hotplug_slot;
+
+struct gpio_hotplug_slot_plat_ops {
+	int (*enable)(struct gpio_hotplug_slot *hp_slot);
+	int (*disable)(struct gpio_hotplug_slot *hp_slot);
+};
+
+struct gpio_hotplug_slot {
+	struct device_node *np;
+	int slot_nr;
+	const struct gpio_hotplug_slot_plat_ops *plat_ops;
+	struct pci_dev *pdev;
+
+	struct gpio_desc *gpiod;
+	unsigned int irq;
+
+	struct hotplug_slot hotplug_slot;
+};
+
+#ifdef CONFIG_HOTPLUG_PCI_GPIO
+int register_gpio_hotplug_slot(struct gpio_hotplug_slot *hp_slot);
+void unregister_gpio_hotplug_slot(struct gpio_hotplug_slot *hp_slot);
+#else
+static inline int register_gpio_hotplug_slot(struct gpio_hotplug_slot *hp_slot)
+{ return 0; }
+static inline void unregister_gpio_hotplug_slot(struct gpio_hotplug_slot *hp_slot) {}
+#endif
+
+#endif //_GPIOPHP_H
-- 
2.17.1

[PATCH V1 3/4] PCI: tegra194: Add support to configure a pluggable slot

[Vidya Sagar]

Add support to configure a slot as a pluggable slot by not power-gating
the respective controller based on the DT property "hotplug-gpios".

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 2600304522eb..0370e881422d 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -280,6 +280,8 @@ struct tegra_pcie_dw {
 	unsigned int phy_count;
 	struct phy **phys;
 
+	bool slot_pluggable;
+
 	struct dentry *debugfs;
 
 	/* Endpoint mode specific */
@@ -1089,6 +1091,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 	struct device_node *np = pcie->dev->of_node;
+	struct property *prop;
 	int ret;
 
 	pcie->dbi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
@@ -1158,6 +1161,10 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
 		pcie->enable_srns =
 			of_property_read_bool(np, "nvidia,enable-srns");
 
+	prop = of_find_property(np, "hotplug-gpios", NULL);
+	if (prop)
+		pcie->slot_pluggable = true;
+
 	if (pcie->of_data->mode == DW_PCIE_RC_TYPE)
 		return 0;
 
@@ -1655,7 +1662,7 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
 	}
 
 	pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
-	if (!pcie->link_state) {
+	if (!pcie->link_state && !pcie->slot_pluggable) {
 		ret = -ENOMEDIUM;
 		goto fail_host_init;
 	}
@@ -2267,7 +2274,7 @@ static int tegra_pcie_dw_remove(struct platform_device *pdev)
 	struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
 
 	if (pcie->of_data->mode == DW_PCIE_RC_TYPE) {
-		if (!pcie->link_state)
+		if (!pcie->link_state && !pcie->slot_pluggable)
 			return 0;
 
 		debugfs_remove_recursive(pcie->debugfs);
@@ -2296,7 +2303,7 @@ static int tegra_pcie_dw_suspend_late(struct device *dev)
 		return -EPERM;
 	}
 
-	if (!pcie->link_state)
+	if (!pcie->link_state && !pcie->slot_pluggable)
 		return 0;
 
 	/* Enable HW_HOT_RST mode */
@@ -2315,7 +2322,7 @@ static int tegra_pcie_dw_suspend_noirq(struct device *dev)
 {
 	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
 
-	if (!pcie->link_state)
+	if (!pcie->link_state && !pcie->slot_pluggable)
 		return 0;
 
 	tegra_pcie_downstream_dev_to_D0(pcie);
@@ -2330,7 +2337,7 @@ static int tegra_pcie_dw_resume_noirq(struct device *dev)
 	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
 	int ret;
 
-	if (!pcie->link_state)
+	if (!pcie->link_state && !pcie->slot_pluggable)
 		return 0;
 
 	ret = tegra_pcie_config_controller(pcie, true);
@@ -2366,7 +2373,7 @@ static int tegra_pcie_dw_resume_early(struct device *dev)
 		return -ENOTSUPP;
 	}
 
-	if (!pcie->link_state)
+	if (!pcie->link_state && !pcie->slot_pluggable)
 		return 0;
 
 	/* Disable HW_HOT_RST mode */
@@ -2388,7 +2395,7 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
 	struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
 
 	if (pcie->of_data->mode == DW_PCIE_RC_TYPE) {
-		if (!pcie->link_state)
+		if (!pcie->link_state && !pcie->slot_pluggable)
 			return;
 
 		debugfs_remove_recursive(pcie->debugfs);
-- 
2.17.1

[PATCH V1 4/4] PCI: tegra194: Enable GPIO based Hot-Plug support

[Vidya Sagar]

Enable the Hot-Plug functionality by registering it with the GPIO Hot-Plug
controller framework.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 0370e881422d..1b70aba08473 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -36,6 +36,7 @@
 #include <soc/tegra/bpmp.h>
 #include <soc/tegra/bpmp-abi.h>
 #include "../../pci.h"
+#include "../../hotplug/gpiophp.h"
 
 #define TEGRA194_DWC_IP_VER			0x490A
 #define TEGRA234_DWC_IP_VER			0x562A
@@ -281,6 +282,7 @@ struct tegra_pcie_dw {
 	struct phy **phys;
 
 	bool slot_pluggable;
+	struct gpio_hotplug_slot hp_slot;
 
 	struct dentry *debugfs;
 
@@ -296,6 +298,11 @@ static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci)
 	return container_of(pci, struct tegra_pcie_dw, pci);
 }
 
+static inline struct tegra_pcie_dw *to_tegra_pcie_from_slot(struct gpio_hotplug_slot *slot)
+{
+	return container_of(slot, struct tegra_pcie_dw, hp_slot);
+}
+
 static inline void appl_writel(struct tegra_pcie_dw *pcie, const u32 value,
 			       const u32 reg)
 {
@@ -1046,6 +1053,45 @@ static const struct dw_pcie_ops tegra_dw_pcie_ops = {
 	.stop_link = tegra_pcie_dw_stop_link,
 };
 
+static int tegra_pcie_slot_enable(struct gpio_hotplug_slot *slot)
+{
+	struct tegra_pcie_dw *pcie = to_tegra_pcie_from_slot(slot);
+	int ret;
+
+	ret = tegra_pcie_dw_start_link(&pcie->pci);
+	if (!ret)
+		pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
+
+	return ret;
+}
+
+static int tegra_pcie_slot_disable(struct gpio_hotplug_slot *slot)
+{
+	struct tegra_pcie_dw *pcie = to_tegra_pcie_from_slot(slot);
+	u32 val;
+
+	val = appl_readl(pcie, APPL_PINMUX);
+	val &= ~APPL_PINMUX_PEX_RST;
+	appl_writel(pcie, val, APPL_PINMUX);
+
+	/*
+	 * Deassert LTSSM state to stop the state toggling between
+	 * polling and detect.
+	 */
+	val = readl(pcie->appl_base + APPL_CTRL);
+	val &= ~APPL_CTRL_LTSSM_EN;
+	writel(val, pcie->appl_base + APPL_CTRL);
+
+	pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
+
+	return 0;
+}
+
+static const struct gpio_hotplug_slot_plat_ops tegra_pcie_gpio_hp_plat_ops = {
+	.enable = tegra_pcie_slot_enable,
+	.disable = tegra_pcie_slot_disable,
+};
+
 static const struct dw_pcie_host_ops tegra_pcie_dw_host_ops = {
 	.host_init = tegra_pcie_dw_host_init,
 };
@@ -1676,6 +1722,20 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
 	pcie->debugfs = debugfs_create_dir(name, NULL);
 	init_debugfs(pcie);
 
+	if (pcie->slot_pluggable) {
+		pcie->hp_slot.plat_ops = &tegra_pcie_gpio_hp_plat_ops;
+		pcie->hp_slot.np = pcie->dev->of_node;
+		pcie->hp_slot.slot_nr = pcie->cid;
+		pcie->hp_slot.pdev = pci_get_slot(pcie->pci.pp.bridge->bus, PCI_DEVFN(0, 0));
+
+		ret = register_gpio_hotplug_slot(&pcie->hp_slot);
+		if (ret < 0)
+			dev_warn(dev,
+				 "Failed to register platform ops for GPIO Hot-Plug controller: %d\n",
+				 ret);
+		ret = 0;
+	}
+
 	return ret;
 
 fail_host_init:
@@ -2277,6 +2337,8 @@ static int tegra_pcie_dw_remove(struct platform_device *pdev)
 		if (!pcie->link_state && !pcie->slot_pluggable)
 			return 0;
 
+		if (pcie->slot_pluggable)
+			unregister_gpio_hotplug_slot(&pcie->hp_slot);
 		debugfs_remove_recursive(pcie->debugfs);
 		tegra_pcie_deinit_controller(pcie);
 		pm_runtime_put_sync(pcie->dev);
@@ -2398,6 +2460,8 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
 		if (!pcie->link_state && !pcie->slot_pluggable)
 			return;
 
+		if (pcie->slot_pluggable)
+			unregister_gpio_hotplug_slot(&pcie->hp_slot);
 		debugfs_remove_recursive(pcie->debugfs);
 		tegra_pcie_downstream_dev_to_D0(pcie);
 
-- 
2.17.1

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Lukas Wunner]

Adding Marek, Pali & Jon to cc as they've worked on somewhat similar
functionality:

https://lore.kernel.org/linux-pci/20220927141926.8895-1-kabel@kernel.org/
https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/

On Sat, Oct 01, 2022 at 12:57:43AM +0530, Vidya Sagar wrote:
> To support the Hot-plug feature, PCIe spec has a well-defined model for 
> hardware implementation and software programming interface. There are also
> some architectures/platforms where the Hot-plug feature is implemented in a
> non-standard way and software support for the respective implementations is
> available with the kernel. This patch series attempts to add support for one
> such non-standard way of supporting the Hot-plug feature where a single GPIO
> is used to detect and report the Hot-Plug and Unplug events to the SW.
> The platforms that can use this piece of software need to have GPIO routed
> from the slot to the controller which can indicate the presence/absence of
> the downstream device through its state. This GPIO should also have the
> capability to interrupt the system when the connection/disconnection event
> takes place.
> A GPIO Hot-plug framework is written which looks for a "hotplug-gpios" named
> GPIO entry in the corresponding device-tree entry of the controller and
> registers a hot-pluggable slot with the Hot-plug framework.
> The platform drivers of the PCIe host bridges/root ports can register with the
> aforementioned GPIO Hot-Plug framework along with ops to perform any platform
> specific tasks during Hot-Plug/Unplug events.
> 
> Oza Pawandeep made an attempt to upstream support for a similar Hot-plug
> feature implementation at a platform level, but the implementation as such
> was very specific to that platform (at least the way I understood it).
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-2-git-send-email-oza.oza@broadcom.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-3-git-send-email-oza.oza@broadcom.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-4-git-send-email-oza.oza@broadcom.com/
> This current series also attempts to address that by extracting out all the
> common code to do with GPIO and Hot-plug core framework and expecting the
> platform drivers to only register/unregister with the GPIO framework. So,
> @Oza, could you try using the GPIO framework from this series and enable
> Hot-plug support for your platform if it still makes sense?
> 
> @Rob,
> Regarding the DT documentation change to add about 'hotplug-gpios, I'm not
> sure if pci.txt is the right place or the dt-schema repository
> i.e https://github.com/devicetree-org/dt-schema
> But, in the interest of keeping all the changes related to this feature in the
> the same repository, I made the changes to the pci.txt file in this repo itself.
> Please let me know if the documentation change needs to be moved to the other
> repo.
> 
> The Changes have been tested on the Tegra234 platform.
> 
> Vidya Sagar (4):
>   dt-bindings: Add "hotplug-gpios" PCIe property
>   PCI/hotplug: Add GPIO PCIe hotplug driver
>   PCI: tegra194: Add support to configure a pluggable slot
>   PCI: tegra194: Enable GPIO based Hot-Plug support
> 
>  Documentation/devicetree/bindings/pci/pci.txt |   4 +
>  drivers/pci/controller/dwc/pcie-tegra194.c    |  85 +++++++-
>  drivers/pci/hotplug/Kconfig                   |  11 +
>  drivers/pci/hotplug/Makefile                  |   1 +
>  drivers/pci/hotplug/gpio_php.c                | 200 ++++++++++++++++++
>  drivers/pci/hotplug/gpiophp.h                 |  40 ++++
>  6 files changed, 334 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/pci/hotplug/gpio_php.c
>  create mode 100644 drivers/pci/hotplug/gpiophp.h
> 
> -- 
> 2.17.1
> 

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Pali Rohár]

On Saturday 01 October 2022 18:00:25 Lukas Wunner wrote:
> Adding Marek, Pali & Jon to cc as they've worked on somewhat similar
> functionality:
> 
> https://lore.kernel.org/linux-pci/20220927141926.8895-1-kabel@kernel.org/
> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
> 
> On Sat, Oct 01, 2022 at 12:57:43AM +0530, Vidya Sagar wrote:
> > To support the Hot-plug feature, PCIe spec has a well-defined model for 
> > hardware implementation and software programming interface. There are also
> > some architectures/platforms where the Hot-plug feature is implemented in a
> > non-standard way and software support for the respective implementations is
> > available with the kernel. This patch series attempts to add support for one
> > such non-standard way of supporting the Hot-plug feature where a single GPIO
> > is used to detect and report the Hot-Plug and Unplug events to the SW.
> > The platforms that can use this piece of software need to have GPIO routed
> > from the slot to the controller which can indicate the presence/absence of
> > the downstream device through its state. This GPIO should also have the
> > capability to interrupt the system when the connection/disconnection event
> > takes place.
> > A GPIO Hot-plug framework is written which looks for a "hotplug-gpios" named
> > GPIO entry in the corresponding device-tree entry of the controller and
> > registers a hot-pluggable slot with the Hot-plug framework.
> > The platform drivers of the PCIe host bridges/root ports can register with the
> > aforementioned GPIO Hot-Plug framework along with ops to perform any platform
> > specific tasks during Hot-Plug/Unplug events.
> > 
> > Oza Pawandeep made an attempt to upstream support for a similar Hot-plug
> > feature implementation at a platform level, but the implementation as such
> > was very specific to that platform (at least the way I understood it).
> > https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-2-git-send-email-oza.oza@broadcom.com/
> > https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-3-git-send-email-oza.oza@broadcom.com/
> > https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-4-git-send-email-oza.oza@broadcom.com/
> > This current series also attempts to address that by extracting out all the
> > common code to do with GPIO and Hot-plug core framework and expecting the
> > platform drivers to only register/unregister with the GPIO framework. So,
> > @Oza, could you try using the GPIO framework from this series and enable
> > Hot-plug support for your platform if it still makes sense?

Hello!

Would not it better to rather synthesise PCIe Slot Capabilities support
in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
existing PCI hotplug code to take care for hotplugging? Because it
already implements all required stuff for re-scanning, registering and
unregistering PCIe devices for Root Ports with Slot Capabilities. And I
think that there is no need to have just another (GPIO based)
implementation of PCI hotplug.

Similar thing Marek and me have implemented for PCIe link state events
in patch series with Lukas pointed.

> > @Rob,
> > Regarding the DT documentation change to add about 'hotplug-gpios, I'm not
> > sure if pci.txt is the right place or the dt-schema repository
> > i.e https://github.com/devicetree-org/dt-schema
> > But, in the interest of keeping all the changes related to this feature in the
> > the same repository, I made the changes to the pci.txt file in this repo itself.
> > Please let me know if the documentation change needs to be moved to the other
> > repo.
> > 
> > The Changes have been tested on the Tegra234 platform.
> > 
> > Vidya Sagar (4):
> >   dt-bindings: Add "hotplug-gpios" PCIe property
> >   PCI/hotplug: Add GPIO PCIe hotplug driver
> >   PCI: tegra194: Add support to configure a pluggable slot
> >   PCI: tegra194: Enable GPIO based Hot-Plug support
> > 
> >  Documentation/devicetree/bindings/pci/pci.txt |   4 +
> >  drivers/pci/controller/dwc/pcie-tegra194.c    |  85 +++++++-
> >  drivers/pci/hotplug/Kconfig                   |  11 +
> >  drivers/pci/hotplug/Makefile                  |   1 +
> >  drivers/pci/hotplug/gpio_php.c                | 200 ++++++++++++++++++
> >  drivers/pci/hotplug/gpiophp.h                 |  40 ++++
> >  6 files changed, 334 insertions(+), 7 deletions(-)
> >  create mode 100644 drivers/pci/hotplug/gpio_php.c
> >  create mode 100644 drivers/pci/hotplug/gpiophp.h
> > 
> > -- 
> > 2.17.1
> > 

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Jonathan Derrick]

On 10/1/2022 10:20 AM, Pali Rohár wrote:
> On Saturday 01 October 2022 18:00:25 Lukas Wunner wrote:
>> Adding Marek, Pali & Jon to cc as they've worked on somewhat similar
>> functionality:
>>
>> https://lore.kernel.org/linux-pci/20220927141926.8895-1-kabel@kernel.org/
>> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
>>
>> On Sat, Oct 01, 2022 at 12:57:43AM +0530, Vidya Sagar wrote:
>>> To support the Hot-plug feature, PCIe spec has a well-defined model for
>>> hardware implementation and software programming interface. There are also
>>> some architectures/platforms where the Hot-plug feature is implemented in a
>>> non-standard way and software support for the respective implementations is
>>> available with the kernel. This patch series attempts to add support for one
>>> such non-standard way of supporting the Hot-plug feature where a single GPIO
>>> is used to detect and report the Hot-Plug and Unplug events to the SW.
>>> The platforms that can use this piece of software need to have GPIO routed
>>> from the slot to the controller which can indicate the presence/absence of
>>> the downstream device through its state. This GPIO should also have the
>>> capability to interrupt the system when the connection/disconnection event
>>> takes place.
>>> A GPIO Hot-plug framework is written which looks for a "hotplug-gpios" named
>>> GPIO entry in the corresponding device-tree entry of the controller and
>>> registers a hot-pluggable slot with the Hot-plug framework.
>>> The platform drivers of the PCIe host bridges/root ports can register with the
>>> aforementioned GPIO Hot-Plug framework along with ops to perform any platform
>>> specific tasks during Hot-Plug/Unplug events.
>>>
>>> Oza Pawandeep made an attempt to upstream support for a similar Hot-plug
>>> feature implementation at a platform level, but the implementation as such
>>> was very specific to that platform (at least the way I understood it).
>>> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-2-git-send-email-oza.oza@broadcom.com/
>>> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-3-git-send-email-oza.oza@broadcom.com/
>>> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-4-git-send-email-oza.oza@broadcom.com/
>>> This current series also attempts to address that by extracting out all the
>>> common code to do with GPIO and Hot-plug core framework and expecting the
>>> platform drivers to only register/unregister with the GPIO framework. So,
>>> @Oza, could you try using the GPIO framework from this series and enable
>>> Hot-plug support for your platform if it still makes sense?
> 
> Hello!
> 
> Would not it better to rather synthesise PCIe Slot Capabilities support
> in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
> existing PCI hotplug code to take care for hotplugging? Because it
> already implements all required stuff for re-scanning, registering and
> unregistering PCIe devices for Root Ports with Slot Capabilities. And I
> think that there is no need to have just another (GPIO based)
> implementation of PCI hotplug.
I did that a few years ago (rejected), but can attest to the robustness 
of the pcie hotplug code on non-hotplug slots.
https://lwn.net/Articles/811988/


> 
> Similar thing Marek and me have implemented for PCIe link state events
> in patch series with Lukas pointed.
> 
>>> @Rob,
>>> Regarding the DT documentation change to add about 'hotplug-gpios, I'm not
>>> sure if pci.txt is the right place or the dt-schema repository
>>> i.e https://github.com/devicetree-org/dt-schema
>>> But, in the interest of keeping all the changes related to this feature in the
>>> the same repository, I made the changes to the pci.txt file in this repo itself.
>>> Please let me know if the documentation change needs to be moved to the other
>>> repo.
>>>
>>> The Changes have been tested on the Tegra234 platform.
>>>
>>> Vidya Sagar (4):
>>>    dt-bindings: Add "hotplug-gpios" PCIe property
>>>    PCI/hotplug: Add GPIO PCIe hotplug driver
>>>    PCI: tegra194: Add support to configure a pluggable slot
>>>    PCI: tegra194: Enable GPIO based Hot-Plug support
>>>
>>>   Documentation/devicetree/bindings/pci/pci.txt |   4 +
>>>   drivers/pci/controller/dwc/pcie-tegra194.c    |  85 +++++++-
>>>   drivers/pci/hotplug/Kconfig                   |  11 +
>>>   drivers/pci/hotplug/Makefile                  |   1 +
>>>   drivers/pci/hotplug/gpio_php.c                | 200 ++++++++++++++++++
>>>   drivers/pci/hotplug/gpiophp.h                 |  40 ++++
>>>   6 files changed, 334 insertions(+), 7 deletions(-)
>>>   create mode 100644 drivers/pci/hotplug/gpio_php.c
>>>   create mode 100644 drivers/pci/hotplug/gpiophp.h
>>>
>>> -- 
>>> 2.17.1
>>>

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Bjorn Helgaas]

On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
> On 10/1/2022 10:20 AM, Pali Rohár wrote:
> ...

> > Would not it better to rather synthesise PCIe Slot Capabilities support
> > in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
> > existing PCI hotplug code to take care for hotplugging? Because it
> > already implements all required stuff for re-scanning, registering and
> > unregistering PCIe devices for Root Ports with Slot Capabilities. And I
> > think that there is no need to have just another (GPIO based)
> > implementation of PCI hotplug.
>
> I did that a few years ago (rejected), but can attest to the robustness of
> the pcie hotplug code on non-hotplug slots.
> https://lwn.net/Articles/811988/

I think the thread is here:
https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
and I'm sorry that my response came across as "rejected".  I intended
it as "this is good ideas and good work and we should keep going".

Bjorn

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Pali Rohár]

On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
> On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
> > On 10/1/2022 10:20 AM, Pali Rohár wrote:
> > ...
> 
> > > Would not it better to rather synthesise PCIe Slot Capabilities support
> > > in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
> > > existing PCI hotplug code to take care for hotplugging? Because it
> > > already implements all required stuff for re-scanning, registering and
> > > unregistering PCIe devices for Root Ports with Slot Capabilities. And I
> > > think that there is no need to have just another (GPIO based)
> > > implementation of PCI hotplug.
> >
> > I did that a few years ago (rejected), but can attest to the robustness of
> > the pcie hotplug code on non-hotplug slots.
> > https://lwn.net/Articles/811988/
> 
> I think the thread is here:
> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
> and I'm sorry that my response came across as "rejected".  I intended
> it as "this is good ideas and good work and we should keep going".
> 
> Bjorn

Nice! So we have consensus that this is a good idea. Anyway, if you need
help with designing something here, please let me know as I have good
understanding of all (just two) consumers of pci-bridge-emul.c driver.

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Jonathan Derrick]

On 10/3/2022 12:21 PM, Pali Rohár wrote:
> On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
>> On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
>>> On 10/1/2022 10:20 AM, Pali Rohár wrote:
>>> ...
>>
>>>> Would not it better to rather synthesise PCIe Slot Capabilities support
>>>> in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
>>>> existing PCI hotplug code to take care for hotplugging? Because it
>>>> already implements all required stuff for re-scanning, registering and
>>>> unregistering PCIe devices for Root Ports with Slot Capabilities. And I
>>>> think that there is no need to have just another (GPIO based)
>>>> implementation of PCI hotplug.
>>>
>>> I did that a few years ago (rejected), but can attest to the robustness of
>>> the pcie hotplug code on non-hotplug slots.
>>> https://lwn.net/Articles/811988/
>>
>> I think the thread is here:
>> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
>> and I'm sorry that my response came across as "rejected".  I intended
>> it as "this is good ideas and good work and we should keep going".
>>
>> Bjorn
> 
> Nice! So we have consensus that this is a good idea. Anyway, if you need
> help with designing something here, please let me know as I have good
> understanding of all (just two) consumers of pci-bridge-emul.c driver.

I haven't looked at the changes required to conform to 6.0. My 
implementation was more or less a filter driver on top of the pciehp 
slot (if it existed or had to be emulated). It's not really a hack for 
anything and was intended for use with an interposer to a bunch of M.2 
that a OEM wanted to hotplug without adding slot hardware. (Yes I know 
M.2 is not technically hot-pluggable. OEM ended up with sysfs managed 
hotplug with this patchset).

But there are systems out there with U.2 without slot logic that could 
likely survive the event gracefully, given that modern CPUs expect this 
thing in the same way that modern kernels don't rely on Surprise+.


My implementation is a bit of overkill if we'd want to strip it from the 
gpio implementation. If we want to include and extend it, we can make 
the gpio_isr another caller of pciehp_ist() (via [1])

[1] 
https://lore.kernel.org/linux-pci/1581120007-5280-7-git-send-email-jonathan.derrick@intel.com/

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Vidya Sagar]

On 10/3/2022 11:51 PM, Pali Rohár wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
>> On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
>>> On 10/1/2022 10:20 AM, Pali Rohár wrote:
>>> ...
>>
>>>> Would not it better to rather synthesise PCIe Slot Capabilities support
>>>> in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then let
>>>> existing PCI hotplug code to take care for hotplugging? Because it
>>>> already implements all required stuff for re-scanning, registering and
>>>> unregistering PCIe devices for Root Ports with Slot Capabilities. And I
>>>> think that there is no need to have just another (GPIO based)
>>>> implementation of PCI hotplug.
>>>
>>> I did that a few years ago (rejected), but can attest to the robustness of
>>> the pcie hotplug code on non-hotplug slots.
>>> https://lwn.net/Articles/811988/
>>
>> I think the thread is here:
>> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
>> and I'm sorry that my response came across as "rejected".  I intended
>> it as "this is good ideas and good work and we should keep going".
>>
>> Bjorn
> 
> Nice! So we have consensus that this is a good idea. Anyway, if you need
> help with designing something here, please let me know as I have good
> understanding of all (just two) consumers of pci-bridge-emul.c driver.
> 

Thanks all for your comments.

I would like to hear from Bjorn / Lorenzo if the design of the current 
patch series is fine at a high level or I should explore emulating the 
root port's configuration space to fake slot config/control registers 
(which in turn depend on the hotplug GPIO interrupt & state to update 
Presence Detect related bits in Slot status register) and use the PCIe 
native Hot-plug framework itself to carry out with enabling the Hot-plug 
functionality?

Thanks,
Vidya Sagar

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Vidya Sagar]

On 10/4/2022 9:34 AM, Vidya Sagar wrote:
> 
> 
> On 10/3/2022 11:51 PM, Pali Rohár wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
>>> On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
>>>> On 10/1/2022 10:20 AM, Pali Rohár wrote:
>>>> ...
>>>
>>>>> Would not it better to rather synthesise PCIe Slot Capabilities 
>>>>> support
>>>>> in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and then 
>>>>> let
>>>>> existing PCI hotplug code to take care for hotplugging? Because it
>>>>> already implements all required stuff for re-scanning, registering and
>>>>> unregistering PCIe devices for Root Ports with Slot Capabilities. 
>>>>> And I
>>>>> think that there is no need to have just another (GPIO based)
>>>>> implementation of PCI hotplug.
>>>>
>>>> I did that a few years ago (rejected), but can attest to the 
>>>> robustness of
>>>> the pcie hotplug code on non-hotplug slots.
>>>> https://lwn.net/Articles/811988/
>>>
>>> I think the thread is here:
>>> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/ 
>>>
>>> and I'm sorry that my response came across as "rejected".  I intended
>>> it as "this is good ideas and good work and we should keep going".
>>>
>>> Bjorn
>>
>> Nice! So we have consensus that this is a good idea. Anyway, if you need
>> help with designing something here, please let me know as I have good
>> understanding of all (just two) consumers of pci-bridge-emul.c driver.
>>
> 
> Thanks all for your comments.
> 
> I would like to hear from Bjorn / Lorenzo if the design of the current 
> patch series is fine at a high level or I should explore emulating the 
> root port's configuration space to fake slot config/control registers 
> (which in turn depend on the hotplug GPIO interrupt & state to update 
> Presence Detect related bits in Slot status register) and use the PCIe 
> native Hot-plug framework itself to carry out with enabling the Hot-plug 
> functionality?

Bjorn / Lorenzo,
Could you please take time to comment on the discussion happened here 
and the right approach to be followed?

Thanks,
Vidya Sagar

> 
> Thanks,
> Vidya Sagar
> 
> 

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Vidya Sagar]

On 10/10/2022 11:44 AM, Vidya Sagar wrote:
> 
> 
> On 10/4/2022 9:34 AM, Vidya Sagar wrote:
>>
>>
>> On 10/3/2022 11:51 PM, Pali Rohár wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
>>>> On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
>>>>> On 10/1/2022 10:20 AM, Pali Rohár wrote:
>>>>> ...
>>>>
>>>>>> Would not it better to rather synthesise PCIe Slot Capabilities 
>>>>>> support
>>>>>> in your PCIe Root Port device (e.g. via pci-bridge-emul.c) and 
>>>>>> then let
>>>>>> existing PCI hotplug code to take care for hotplugging? Because it
>>>>>> already implements all required stuff for re-scanning, registering 
>>>>>> and
>>>>>> unregistering PCIe devices for Root Ports with Slot Capabilities. 
>>>>>> And I
>>>>>> think that there is no need to have just another (GPIO based)
>>>>>> implementation of PCI hotplug.
>>>>>
>>>>> I did that a few years ago (rejected), but can attest to the 
>>>>> robustness of
>>>>> the pcie hotplug code on non-hotplug slots.
>>>>> https://lwn.net/Articles/811988/
>>>>
>>>> I think the thread is here:
>>>> https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/ 
>>>>
>>>> and I'm sorry that my response came across as "rejected".  I intended
>>>> it as "this is good ideas and good work and we should keep going".
>>>>
>>>> Bjorn
>>>
>>> Nice! So we have consensus that this is a good idea. Anyway, if you need
>>> help with designing something here, please let me know as I have good
>>> understanding of all (just two) consumers of pci-bridge-emul.c driver.
>>>
>>
>> Thanks all for your comments.
>>
>> I would like to hear from Bjorn / Lorenzo if the design of the current 
>> patch series is fine at a high level or I should explore emulating the 
>> root port's configuration space to fake slot config/control registers 
>> (which in turn depend on the hotplug GPIO interrupt & state to update 
>> Presence Detect related bits in Slot status register) and use the PCIe 
>> native Hot-plug framework itself to carry out with enabling the 
>> Hot-plug functionality?
> 
> Bjorn / Lorenzo,
> Could you please take time to comment on the discussion happened here 
> and the right approach to be followed?

I'm really sorry to bug you on this, but would like to hear your 
comments on the approach to be taken. So, I would really like to hear 
your take on this.

Thanks,
Vidya Sagar

> 
> Thanks,
> Vidya Sagar
> 
>>
>> Thanks,
>> Vidya Sagar
>>
>>

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Manivannan Sadhasivam]

On Mon, Oct 17, 2022 at 08:16:06AM +0530, Vidya Sagar wrote:
> 
> 
> On 10/10/2022 11:44 AM, Vidya Sagar wrote:
> > 
> > 
> > On 10/4/2022 9:34 AM, Vidya Sagar wrote:
> > > 
> > > 
> > > On 10/3/2022 11:51 PM, Pali Rohár wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > On Monday 03 October 2022 13:09:49 Bjorn Helgaas wrote:
> > > > > On Sat, Oct 01, 2022 at 05:50:07PM -0600, Jonathan Derrick wrote:
> > > > > > On 10/1/2022 10:20 AM, Pali Rohár wrote:
> > > > > > ...
> > > > > 
> > > > > > > Would not it better to rather synthesise PCIe Slot
> > > > > > > Capabilities support
> > > > > > > in your PCIe Root Port device (e.g. via
> > > > > > > pci-bridge-emul.c) and then let
> > > > > > > existing PCI hotplug code to take care for hotplugging? Because it
> > > > > > > already implements all required stuff for
> > > > > > > re-scanning, registering and
> > > > > > > unregistering PCIe devices for Root Ports with Slot
> > > > > > > Capabilities. And I
> > > > > > > think that there is no need to have just another (GPIO based)
> > > > > > > implementation of PCI hotplug.
> > > > > > 
> > > > > > I did that a few years ago (rejected), but can attest to
> > > > > > the robustness of
> > > > > > the pcie hotplug code on non-hotplug slots.
> > > > > > https://lwn.net/Articles/811988/
> > > > > 
> > > > > I think the thread is here:
> > > > > https://lore.kernel.org/linux-pci/1581120007-5280-1-git-send-email-jonathan.derrick@intel.com/
> > > > > 
> > > > > and I'm sorry that my response came across as "rejected".  I intended
> > > > > it as "this is good ideas and good work and we should keep going".
> > > > > 
> > > > > Bjorn
> > > > 
> > > > Nice! So we have consensus that this is a good idea. Anyway, if you need
> > > > help with designing something here, please let me know as I have good
> > > > understanding of all (just two) consumers of pci-bridge-emul.c driver.
> > > > 
> > > 
> > > Thanks all for your comments.
> > > 
> > > I would like to hear from Bjorn / Lorenzo if the design of the
> > > current patch series is fine at a high level or I should explore
> > > emulating the root port's configuration space to fake slot
> > > config/control registers (which in turn depend on the hotplug GPIO
> > > interrupt & state to update Presence Detect related bits in Slot
> > > status register) and use the PCIe native Hot-plug framework itself
> > > to carry out with enabling the Hot-plug functionality?
> > 
> > Bjorn / Lorenzo,
> > Could you please take time to comment on the discussion happened here
> > and the right approach to be followed?
> 
> I'm really sorry to bug you on this, but would like to hear your comments on
> the approach to be taken. So, I would really like to hear your take on this.
> 

Since Bjorn already expressed his good will about the approach, I think you
can just proceed with the emulation layer. I don't think there will be any
controversy.

Thanks,
Mani

> Thanks,
> Vidya Sagar
> 
> > 
> > Thanks,
> > Vidya Sagar
> > 
> > > 
> > > Thanks,
> > > Vidya Sagar
> > > 
> > > 

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

Re: [PATCH V1 0/4] GPIO based PCIe Hot-Plug support

[Rob Herring]

On Sat, Oct 01, 2022 at 12:57:43AM +0530, Vidya Sagar wrote:
> To support the Hot-plug feature, PCIe spec has a well-defined model for 
> hardware implementation and software programming interface. There are also
> some architectures/platforms where the Hot-plug feature is implemented in a
> non-standard way and software support for the respective implementations is
> available with the kernel. This patch series attempts to add support for one
> such non-standard way of supporting the Hot-plug feature where a single GPIO
> is used to detect and report the Hot-Plug and Unplug events to the SW.
> The platforms that can use this piece of software need to have GPIO routed
> from the slot to the controller which can indicate the presence/absence of
> the downstream device through its state. This GPIO should also have the
> capability to interrupt the system when the connection/disconnection event
> takes place.
> A GPIO Hot-plug framework is written which looks for a "hotplug-gpios" named
> GPIO entry in the corresponding device-tree entry of the controller and
> registers a hot-pluggable slot with the Hot-plug framework.
> The platform drivers of the PCIe host bridges/root ports can register with the
> aforementioned GPIO Hot-Plug framework along with ops to perform any platform
> specific tasks during Hot-Plug/Unplug events.
> 
> Oza Pawandeep made an attempt to upstream support for a similar Hot-plug
> feature implementation at a platform level, but the implementation as such
> was very specific to that platform (at least the way I understood it).
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-2-git-send-email-oza.oza@broadcom.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-3-git-send-email-oza.oza@broadcom.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1504155029-24729-4-git-send-email-oza.oza@broadcom.com/
> This current series also attempts to address that by extracting out all the
> common code to do with GPIO and Hot-plug core framework and expecting the
> platform drivers to only register/unregister with the GPIO framework. So,
> @Oza, could you try using the GPIO framework from this series and enable
> Hot-plug support for your platform if it still makes sense?
> 
> @Rob,
> Regarding the DT documentation change to add about 'hotplug-gpios, I'm not
> sure if pci.txt is the right place or the dt-schema repository
> i.e https://github.com/devicetree-org/dt-schema
> But, in the interest of keeping all the changes related to this feature in the
> the same repository, I made the changes to the pci.txt file in this repo itself.
> Please let me know if the documentation change needs to be moved to the other
> repo.

Nothing should be added to pci.txt. So yes, dt-schema is where this 
needs to end up.

Rob