Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller
@ 2026-09-08 15:28 Lorenzo Bianconi
  2026-09-08 15:28 ` [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document " Lorenzo Bianconi
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Lorenzo Bianconi,
	Bartosz Golaszewski, Manivannan Sadhasivam, Konrad Dybcio,
	Abel Vesa

The TC9563 embeds a GPIO controller currently used for per-port reset
signals.
Add a driver for the GPIO controller embedded in the Toshiba TC9563 PCIe
switch providing 37 GPIO lines. The controller is registered as an auxiliary
device by the TC9563 power controller driver and accessed through the same
i2c device.
Please note this series is based on pwrctrl upstream branch.

---
Changes in v4:
- Do not always use 0 as aux device id.
- Link to v3: https://lore.kernel.org/r/20260904-pci-tc9563-aux-v3-0-5b1449d62ba2@oss.qualcomm.com

Changes in v3:
- Describe gpio-controller support in TC9563 binding instead of defining
  a gpio child node.
- Link to v2: https://lore.kernel.org/r/20260903-pci-tc9563-aux-v2-0-34c625b542c1@oss.qualcomm.com

Changes in v2:
- Fix PCI_PWRCTRL_TC9563 Kconfig dependency
- Move of_node refcounting management in tc9563_pwrctrl_adev_add().
- Link to v1: https://lore.kernel.org/r/20260901-pci-tc9563-aux-v1-0-dd9b80738e70@oss.qualcomm.com

---
Alex Elder (1):
      gpio: tc9563: Add support for the embedded GPIO controller

Lorenzo Bianconi (4):
      dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller
      PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
      PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API
      arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller

 .../devicetree/bindings/pci/toshiba,tc9563.yaml    |  19 ++-
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts       |   7 +-
 drivers/gpio/Kconfig                               |  11 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-tc9563.c                         |  99 +++++++++++++
 drivers/pci/pwrctrl/Kconfig                        |   1 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c           | 165 +++++++++++++++++++--
 include/linux/soc/qcom/tc9563.h                    |  19 +++
 8 files changed, 305 insertions(+), 17 deletions(-)
---
base-commit: 028f86457e822ab9de5b0d6d23efddc1b8b54ae1
change-id: 20260731-pci-tc9563-aux-66a29d438f60

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>


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

* [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller
  2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
@ 2026-09-08 15:28 ` Lorenzo Bianconi
  2026-09-08 15:34   ` sashiko-bot
  2026-09-08 15:28 ` [PATCH v4 2/5] gpio: tc9563: Add support for the " Lorenzo Bianconi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Lorenzo Bianconi

The TC9563 PCIe switch embeds a GPIO controller providing 37 GPIO
lines. Describe the switch node itself as the embedded GPIO controller
and document the per-port reset-gpios property.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 .../devicetree/bindings/pci/toshiba,tc9563.yaml       | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
index f084830c6d0c..2f03e4e45114 100644
--- a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
+++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
@@ -31,6 +31,11 @@ properties:
     description:
       GPIO controlling the RESX# pin.
 
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
   vdd18-supply: true
 
   vdd09-supply: true
@@ -59,6 +64,11 @@ patternProperties:
       child nodes describing the internal downstream ports of
       the tc9563 switch.
     type: object
+    properties:
+      reset-gpios:
+        maxItems: 1
+        description:
+          GPIO controlling the reset line connected to the downstream port.
     allOf:
       - $ref: "#/$defs/tc9563-node"
       - $ref: /schemas/pci/pci-pci-bridge.yaml#
@@ -128,7 +138,7 @@ examples:
             ranges;
             bus-range = <0x01 0xff>;
 
-            pcie@0,0 {
+            tc9563: pcie@0,0 {
                 compatible = "pci1179,0623";
 
                 reg = <0x10000 0x0 0x0 0x0 0x0>;
@@ -149,6 +159,9 @@ examples:
 
                 resx-gpios = <&gpio 1 GPIO_ACTIVE_LOW>;
 
+                gpio-controller;
+                #gpio-cells = <2>;
+
                 pcie@1,0 {
                     compatible = "pciclass,0604";
                     reg = <0x20800 0x0 0x0 0x0 0x0>;
@@ -158,6 +171,8 @@ examples:
                     ranges;
                     bus-range = <0x03 0xff>;
 
+                    reset-gpios = <&tc9563 2 GPIO_ACTIVE_LOW>;
+
                     toshiba,no-dfe-support;
                 };
 
@@ -170,6 +185,8 @@ examples:
                     ranges;
                     bus-range = <0x04 0xff>;
 
+                    reset-gpios = <&tc9563 3 GPIO_ACTIVE_LOW>;
+
                     toshiba,tx-amplitude-microvolt = <10>;
                 };
 

-- 
2.55.0


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

* [PATCH v4 2/5] gpio: tc9563: Add support for the embedded GPIO controller
  2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
  2026-09-08 15:28 ` [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document " Lorenzo Bianconi
@ 2026-09-08 15:28 ` Lorenzo Bianconi
  2026-09-08 15:37   ` sashiko-bot
  2026-09-08 15:28 ` [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support Lorenzo Bianconi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Bartosz Golaszewski,
	Manivannan Sadhasivam, Lorenzo Bianconi

From: Alex Elder <elder@riscstar.com>

Add a driver for the GPIO controller embedded in the Toshiba TC9563
PCIe switch (and the Qualcomm QPS615). The device implements 35
GPIOs using two register banks: three registers control the first 32
GPIOs (20 and 21 are reserved) and three more control GPIOs 32-36.
GPIOs 22-24, 27-28, 31, and 34 are input-only.

The driver is registered as an auxiliary device driver. The TC9563
power controller creates the auxiliary device and provides a regmap
that gives access to the GPIO registers, so the gpio-regmap helpers
are used to implement the GPIO chip.

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/gpio/Kconfig            | 11 +++++
 drivers/gpio/Makefile           |  1 +
 drivers/gpio/gpio-tc9563.c      | 99 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/tc9563.h | 16 +++++++
 4 files changed, 127 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a48586bb8edb..2179eaffcc48 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1830,6 +1830,17 @@ config GPIO_LTC4283
 
 endmenu
 
+config GPIO_TC9563
+	tristate "Toshiba TC9563 GPIO support"
+	default m if ARCH_QCOM
+	select AUXILIARY_BUS
+	select GPIO_REGMAP
+	help
+	  This enables support for the GPIO controller embedded in the Toshiba
+	  TC9563 (and Qualcomm QPS615).  This device connects to the host
+	  via PCIe port, which is the upstream port on an internal PCIe
+	  switch.
+
 menu "PCI GPIO expanders"
 	depends on PCI
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index dc9e6d643b5b..792faa2668c9 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -182,6 +182,7 @@ obj-$(CONFIG_GPIO_SYSCON)		+= gpio-syscon.o
 obj-$(CONFIG_GPIO_TANGIER)		+= gpio-tangier.o
 obj-$(CONFIG_GPIO_TB10X)		+= gpio-tb10x.o
 obj-$(CONFIG_GPIO_TC3589X)		+= gpio-tc3589x.o
+obj-$(CONFIG_GPIO_TC9563)		+= gpio-tc9563.o
 obj-$(CONFIG_GPIO_TEGRA186)		+= gpio-tegra186.o
 obj-$(CONFIG_GPIO_TEGRA)		+= gpio-tegra.o
 obj-$(CONFIG_GPIO_THUNDERX)		+= gpio-thunderx.o
diff --git a/drivers/gpio/gpio-tc9563.c b/drivers/gpio/gpio-tc9563.c
new file mode 100644
index 000000000000..68c20c8cba79
--- /dev/null
+++ b/drivers/gpio/gpio-tc9563.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
+ */
+
+/*
+ * The Toshiba TC9563 implements a PCIe Gen 3 switch that connects an
+ * upstream x4 port to two downstream PCIe x2 ports. It incorporates
+ * an internal endpoint on a internal PCIe port that implements two
+ * Synopsys XGMAC Ethernet interfaces.
+ *
+ * 37 GPIOs are also implemented by an embedded GPIO controller.  Three
+ * registers control the first 32 GPIOs (other than 20 and 21, which are
+ * reserved). Three other registers control GPIOs 32 through 36. GPIOs
+ * 22-24, 27-28, 31, and 34 are treated as "input only".
+ *
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/soc/qcom/tc9563.h>
+
+/*
+ * There are two sets of registers, each representing (up to) 32 GPIOs with a
+ * stride of 4 bytes (IN1 is 4 bytes past IN0, EN1 is 4 bytes past EN0, etc.).
+ */
+#define TC9563_GPIO_COUNT		37
+#define TC9563_GPIO_PER_REG		32
+#define TC9563_GPIO_REG_STRIDE		4
+
+static int tc9563_gpio_init_valid_mask(struct gpio_chip *gc,
+				       unsigned long *valid_mask,
+				       unsigned int ngpios)
+{
+	/* GPIOs 20 and 21 are reserved */
+	bitmap_fill(valid_mask, ngpios);
+	bitmap_clear(valid_mask, 20, 2);
+
+	return 0;
+}
+
+static int tc9563_gpio_probe(struct auxiliary_device *adev,
+			     const struct auxiliary_device_id *id)
+{
+	struct gpio_regmap_config config = {
+		.parent = &adev->dev,
+		.ngpio = TC9563_GPIO_COUNT,
+		.reg_stride = TC9563_GPIO_REG_STRIDE,
+		.ngpio_per_reg = TC9563_GPIO_PER_REG,
+		.reg_dat_base = GPIO_REGMAP_ADDR(TC9563_GPIO_IN0_OFFSET),
+		.reg_set_base = GPIO_REGMAP_ADDR(TC9563_GPIO_OUT0_OFFSET),
+		.reg_dir_in_base = GPIO_REGMAP_ADDR(TC9563_GPIO_EN0_OFFSET),
+		.init_valid_mask = tc9563_gpio_init_valid_mask,
+	};
+	DECLARE_BITMAP(fixed_dir_mask, TC9563_GPIO_COUNT);
+	DECLARE_BITMAP(fixed_dir_out, TC9563_GPIO_COUNT);
+
+	config.regmap = dev_get_platdata(&adev->dev);
+	if (!config.regmap)
+		return -EINVAL;
+
+	/*
+	 * Only some of our GPIOs are fixed direction:
+	 * 22, 23, 24, 27, 28, 31, and 34 are input-only.
+	 */
+	bitmap_zero(fixed_dir_mask, TC9563_GPIO_COUNT);
+	bitmap_set(fixed_dir_mask, 22, 3);
+	bitmap_set(fixed_dir_mask, 27, 2);
+	set_bit(31, fixed_dir_mask);
+	set_bit(34, fixed_dir_mask);
+	config.fixed_direction_mask = fixed_dir_mask;
+
+	bitmap_zero(fixed_dir_out, TC9563_GPIO_COUNT);
+	config.fixed_direction_output = fixed_dir_out;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&adev->dev, &config));
+};
+
+static const struct auxiliary_device_id tc9563_gpio_ids[] = {
+	{ "pci_pwrctrl_tc9563." TC9563_GPIO_DEV_NAME },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(auxiliary, tc9563_gpio_ids);
+
+static struct auxiliary_driver tc9563_gpio_driver = {
+	.name		= TC9563_GPIO_DEV_NAME,
+	.probe		= tc9563_gpio_probe,
+	.id_table	= tc9563_gpio_ids,
+};
+module_auxiliary_driver(tc9563_gpio_driver);
+
+MODULE_AUTHOR("Alex Elder <elder@riscstar.com>");
+MODULE_AUTHOR("Daniel Thompson <daniel@riscstar.com>");
+MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>");
+MODULE_DESCRIPTION("Toshiba TC9563 GPIO Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/soc/qcom/tc9563.h b/include/linux/soc/qcom/tc9563.h
new file mode 100644
index 000000000000..0dfd25747b9a
--- /dev/null
+++ b/include/linux/soc/qcom/tc9563.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2026 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
+ */
+
+#ifndef __QCOM_TC9563_H
+#define __QCOM_TC9563_H
+
+#define TC9563_GPIO_DEV_NAME	"tc9563-gpio"
+
+#define TC9563_GPIO_IN0_OFFSET		0x801200
+#define TC9563_GPIO_EN0_OFFSET		0x801208
+#define TC9563_GPIO_OUT0_OFFSET		0x801210
+
+#endif /* __QCOM_TC9563_H */

-- 
2.55.0


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

* [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
  2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
  2026-09-08 15:28 ` [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document " Lorenzo Bianconi
  2026-09-08 15:28 ` [PATCH v4 2/5] gpio: tc9563: Add support for the " Lorenzo Bianconi
@ 2026-09-08 15:28 ` Lorenzo Bianconi
  2026-09-08 15:45   ` sashiko-bot
  2026-09-08 15:28 ` [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API Lorenzo Bianconi
  2026-09-08 15:28 ` [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller Lorenzo Bianconi
  4 siblings, 1 reply; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Bartosz Golaszewski,
	Manivannan Sadhasivam, Lorenzo Bianconi

The TC9563 embeds a GPIO controller used for per-port reset signals.
Create an auxiliary device for it so the gpio-tc9563 driver can
register the GPIO chip and enable DT-based GPIO lookups. Pass the
tc9563 regmap to the auxiliary device as its platform data.

The pwrctrl driver does not wait for the GPIO chip to be probed. The
per-port reset GPIO lookup, returning -EPROBE_DEFER until the chip is
registered, is added in the next patch.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Reviewed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/Kconfig              |  1 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 80 ++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
index 1952ab4f29b6..38aab596aa04 100644
--- a/drivers/pci/pwrctrl/Kconfig
+++ b/drivers/pci/pwrctrl/Kconfig
@@ -29,6 +29,7 @@ config PCI_PWRCTRL_TC9563
 	select PCI_PWRCTRL
 	default m if ARCH_QCOM
 	depends on I2C
+	depends on GPIO_TC9563
 	select REGMAP_I2C
 	help
 	  Say Y here to enable the PCI Power Control driver of TC9563 PCIe
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index 59ad219c26c0..3fb862105fa5 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -4,12 +4,14 @@
  */
 
 #include <linux/array_size.h>
+#include <linux/auxiliary_bus.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/idr.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -18,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/soc/qcom/tc9563.h>
 #include <linux/string.h>
 #include <linux/types.h>
 
@@ -151,6 +154,8 @@ static const struct reg_sequence dsp2_pwroff_seq[] = {
 	{TC9563_PORT_ACCESS_ENABLE, 0x8},
 };
 
+static DEFINE_IDA(tc9563_pwrctrl_ida);
+
 static int tc9563_pwrctrl_disable_port(struct tc9563_pwrctrl *tc9563,
 				       enum tc9563_pwrctrl_ports port)
 {
@@ -393,6 +398,77 @@ static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
 	return 0;
 }
 
+static void tc9563_pwrctrl_adev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	ida_free(&tc9563_pwrctrl_ida, adev->id);
+	of_node_put(adev->dev.of_node);
+	kfree(adev);
+}
+
+static void tc9563_pwrctrl_adev_remove(void *data)
+{
+	struct auxiliary_device *adev = data;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static int tc9563_pwrctrl_adev_add(struct device *dev, const char *name,
+				   struct device_node *of_node,
+				   void *priv_data)
+{
+	struct auxiliary_device *adev;
+	int id, ret;
+
+	adev = kzalloc_obj(*adev);
+	if (!adev)
+		return -ENOMEM;
+
+	id = ida_alloc(&tc9563_pwrctrl_ida, GFP_KERNEL);
+	if (id < 0) {
+		kfree(adev);
+		return id;
+	}
+
+	adev->id = id;
+	adev->name = name;
+	adev->dev.parent = dev;
+	adev->dev.platform_data = priv_data;
+	adev->dev.release = tc9563_pwrctrl_adev_release;
+	adev->dev.of_node = of_node_get(of_node);
+	dev_set_of_node_reused(&adev->dev);
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		ida_free(&tc9563_pwrctrl_ida, id);
+		of_node_put(adev->dev.of_node);
+		kfree(adev);
+		return ret;
+	}
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(dev, tc9563_pwrctrl_adev_remove, adev);
+}
+
+static int tc9563_pwrctrl_add_gpio_adev(struct tc9563_pwrctrl *tc9563)
+{
+	struct device *dev = tc9563->pwrctrl.dev;
+
+	if (!of_property_read_bool(dev->of_node, "gpio-controller") ||
+	    !of_property_present(dev->of_node, "#gpio-cells"))
+		return 0;
+
+	return tc9563_pwrctrl_adev_add(dev, TC9563_GPIO_DEV_NAME, dev->of_node,
+				       tc9563->regmap);
+}
+
 static int tc9563_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
 {
 	struct tc9563_pwrctrl *tc9563 = container_of(pwrctrl,
@@ -596,6 +672,10 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 	tc9563->pwrctrl.power_on = tc9563_pwrctrl_power_on;
 	tc9563->pwrctrl.power_off = tc9563_pwrctrl_power_off;
 
+	ret = tc9563_pwrctrl_add_gpio_adev(tc9563);
+	if (ret)
+		goto remove_i2c;
+
 	ret = devm_pci_pwrctrl_device_set_ready(dev, &tc9563->pwrctrl);
 	if (ret)
 		goto power_off;

-- 
2.55.0


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

* [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API
  2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2026-09-08 15:28 ` [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support Lorenzo Bianconi
@ 2026-09-08 15:28 ` Lorenzo Bianconi
  2026-09-08 15:45   ` sashiko-bot
  2026-09-08 15:28 ` [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller Lorenzo Bianconi
  4 siblings, 1 reply; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Bartosz Golaszewski,
	Manivannan Sadhasivam, Lorenzo Bianconi

Remove the local TC9563_GPIO_MASK and TC9563_GPIO_DEASSERT_BITS
definitions, which are no longer used after switching to the GPIO
descriptor API. Move TC9563_GPIO_CONFIG and TC9563_RESET_GPIO
definitions in tc9563.h header file.

Replace the direct regmap-based per-port reset logic in
assert_deassert_reset() with gpiod_direction_output() calls, falling
back to the legacy regmap approach only when no reset-gpios DT
property is present for a given port.

Add the reset GPIO pointer to struct tc9563_pwrctrl_cfg and introduce
tc9563_pwrctrl_parse_reset_line() to look up reset-gpios from each
PCI downstream port child node. The lookup is done lazily at the
beginning of power_on(), returning -EPROBE_DEFER until the GPIO chip
is registered.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Reviewed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 85 ++++++++++++++++++++++++++------
 include/linux/soc/qcom/tc9563.h          |  3 ++
 2 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index 3fb862105fa5..59bc0d77d3c4 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -26,9 +26,6 @@
 
 #include "../pci.h"
 
-#define TC9563_GPIO_CONFIG		0x801208
-#define TC9563_RESET_GPIO		0x801210
-
 #define TC9563_PORT_L0S_DELAY		0x82496c
 #define TC9563_PORT_L1_DELAY		0x824970
 
@@ -60,9 +57,6 @@
 #define TC9563_POWER_CONTROL		0x82b09c
 #define TC9563_POWER_CONTROL_OVREN	0x82b2c8
 
-#define TC9563_GPIO_MASK		0xfffffff3
-#define TC9563_GPIO_DEASSERT_BITS	0xc  /* Clear to deassert GPIO */
-
 #define TC9563_TX_MARGIN_MIN_UA		400000
 
 /*
@@ -88,6 +82,7 @@ struct tc9563_pwrctrl_cfg {
 	u8 nfts[2]; /* GEN1 & GEN2 */
 	bool disable_dfe;
 	bool disable_port;
+	struct gpio_desc *reset;
 };
 
 #define TC9563_PWRCTL_MAX_SUPPLY	6
@@ -354,16 +349,40 @@ static int tc9563_pwrctrl_set_nfts(struct tc9563_pwrctrl *tc9563,
 static int tc9563_pwrctrl_assert_deassert_reset(struct tc9563_pwrctrl *tc9563,
 						bool deassert)
 {
-	int ret, val;
-
-	ret = regmap_write(tc9563->regmap, TC9563_GPIO_CONFIG,
-			   TC9563_GPIO_MASK);
-	if (ret)
-		return ret;
-
-	val = deassert ? TC9563_GPIO_DEASSERT_BITS : 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tc9563->cfg); i++) {
+		int err;
+
+		if (tc9563->cfg[i].reset) {
+			err = gpiod_direction_output(tc9563->cfg[i].reset,
+						     !deassert);
+			if (err)
+				return err;
+		} else {
+			/* Fallback: legacy DTS without reset-gpios */
+			switch (i) {
+			case TC9563_DSP1:
+			case TC9563_DSP2:
+				err = regmap_clear_bits(tc9563->regmap,
+							TC9563_GPIO_CONFIG,
+							BIT(i + 1));
+				if (err)
+					return err;
+
+				err = regmap_assign_bits(tc9563->regmap,
+							 TC9563_RESET_GPIO,
+							 BIT(i + 1), deassert);
+				if (err)
+					return err;
+				break;
+			default:
+				break;
+			}
+		}
+	}
 
-	return regmap_write(tc9563->regmap, TC9563_RESET_GPIO, val);
+	return 0;
 }
 
 static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
@@ -398,6 +417,38 @@ static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
 	return 0;
 }
 
+static int tc9563_pwrctrl_parse_reset_line(struct tc9563_pwrctrl *tc9563)
+{
+	enum tc9563_pwrctrl_ports port = TC9563_USP;
+	struct device *dev = tc9563->pwrctrl.dev;
+	struct device_node *node = dev->of_node;
+
+	for_each_child_of_node_scoped(node, child) {
+		struct tc9563_pwrctrl_cfg *cfg;
+
+		if (++port >= TC9563_MAX)
+			break;
+
+		cfg = &tc9563->cfg[port];
+		if (cfg->reset) /* Already discovered */
+			continue;
+
+		cfg->reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(child),
+						   "reset", GPIOD_ASIS,
+						   NULL);
+		if (IS_ERR(cfg->reset)) {
+			int err = PTR_ERR(cfg->reset);
+
+			cfg->reset = NULL;
+			if (err != -ENOENT)
+				return dev_err_probe(dev, err,
+						     "failed to get reset\n");
+		}
+	}
+
+	return 0;
+}
+
 static void tc9563_pwrctrl_adev_release(struct device *dev)
 {
 	struct auxiliary_device *adev = to_auxiliary_dev(dev);
@@ -489,6 +540,10 @@ static int tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
 	struct tc9563_pwrctrl_cfg *cfg;
 	int ret, i;
 
+	ret = tc9563_pwrctrl_parse_reset_line(tc9563);
+	if (ret)
+		return ret;
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(tc9563->supplies),
 				    tc9563->supplies);
 	if (ret < 0)
diff --git a/include/linux/soc/qcom/tc9563.h b/include/linux/soc/qcom/tc9563.h
index 0dfd25747b9a..086f37a40d80 100644
--- a/include/linux/soc/qcom/tc9563.h
+++ b/include/linux/soc/qcom/tc9563.h
@@ -13,4 +13,7 @@
 #define TC9563_GPIO_EN0_OFFSET		0x801208
 #define TC9563_GPIO_OUT0_OFFSET		0x801210
 
+#define TC9563_GPIO_CONFIG		TC9563_GPIO_EN0_OFFSET
+#define TC9563_RESET_GPIO		TC9563_GPIO_OUT0_OFFSET
+
 #endif /* __QCOM_TC9563_H */

-- 
2.55.0


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

* [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller
  2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
                   ` (3 preceding siblings ...)
  2026-09-08 15:28 ` [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API Lorenzo Bianconi
@ 2026-09-08 15:28 ` Lorenzo Bianconi
  2026-09-08 15:41   ` sashiko-bot
  4 siblings, 1 reply; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chaitanya Chundru, Linus Walleij,
	Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
	Michael Walle
  Cc: Alex Elder, Daniel Thompson, Sushrut Shree Trivedi, linux-pci,
	devicetree, linux-gpio, linux-arm-msm, Konrad Dybcio,
	Bartosz Golaszewski, Abel Vesa, Lorenzo Bianconi

The Toshiba TC9563 PCIe switch embeds a GPIO controller providing 37 GPIO
lines. The controller is registered as an auxiliary device by the TC9563
power controller driver and accessed through the same i2c device.

Describe the switch node itself as the embedded GPIO controller and us
it to drive the PERST# reset lines of the two external downstream port
(pcie@1,0 and pcie@2,0), as expected by the TC9563 power controller after
switching to the GPIO descriptor API for per-port resets.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index a13315bf0fb0..4da7342ca90d 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -867,7 +867,7 @@ &pcie1_phy {
 &pcie1_port0 {
 	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
 
-	pcie@0,0 {
+	tc9563: pcie@0,0 {
 		compatible = "pci1179,0623";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
 		#address-cells = <3>;
@@ -891,6 +891,9 @@ pcie@0,0 {
 		pinctrl-0 = <&tc9563_resx_n>;
 		pinctrl-names = "default";
 
+		gpio-controller;
+		#gpio-cells = <2>;
+
 		pcie1_switch0_dsp1: pcie@1,0 {
 			reg = <0x20800 0x0 0x0 0x0 0x0>;
 			#address-cells = <3>;
@@ -899,6 +902,7 @@ pcie1_switch0_dsp1: pcie@1,0 {
 			device_type = "pci";
 			ranges;
 			bus-range = <0x3 0xff>;
+			reset-gpios = <&tc9563 2 GPIO_ACTIVE_LOW>;
 		};
 
 		pcie@2,0 {
@@ -909,6 +913,7 @@ pcie@2,0 {
 			device_type = "pci";
 			ranges;
 			bus-range = <0x4 0xff>;
+			reset-gpios = <&tc9563 3 GPIO_ACTIVE_LOW>;
 
 			/* Renesas μPD720201 PCIe USB3.0 Host Controller */
 			usb-controller@0,0 {

-- 
2.55.0


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

* Re: [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller
  2026-09-08 15:28 ` [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document " Lorenzo Bianconi
@ 2026-09-08 15:34   ` sashiko-bot
  2026-09-08 15:47     ` Lorenzo Bianconi
  0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-09-08 15:34 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: devicetree, conor+dt, linux-pci, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added GPIO provider capabilities 'gpio-controller' and '#gpio-cells' are missing from the schema's 'required' list.
--

commit 6d6cb30a1eb86b847e98eea66d5ba6cc77a091a7
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller

The TC9563 PCIe switch embeds a GPIO controller providing 37 GPIO
lines. Describe the switch node itself as the embedded GPIO controller
and document the per-port reset-gpios property.

> diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> index f084830c6d0cb..2f03e4e45114d 100644
> --- a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> @@ -31,6 +31,11 @@ properties:
>      description:
>        GPIO controlling the RESX# pin.
>  
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +

[Severity: Medium]
Should these newly added GPIO provider capabilities be included in the
schema's required properties list?

Looking at the required list in
Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml:

required:
  - resx-gpios
  - vdd18-supply
  - vdd09-supply
  - vddc-supply
  - vddio1-supply
  - vddio2-supply
  - vddio18-supply
  - i2c-parent

If the new capabilities are omitted from this list, device tree nodes missing
gpio-controller and #gpio-cells will silently pass schema validation,
allowing incomplete configurations to enter the device tree.

At runtime, dependent nodes (such as the downstream ports attempting to use
reset-gpios) could fail when the provider is misconfigured.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=1

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

* Re: [PATCH v4 2/5] gpio: tc9563: Add support for the embedded GPIO controller
  2026-09-08 15:28 ` [PATCH v4 2/5] gpio: tc9563: Add support for the " Lorenzo Bianconi
@ 2026-09-08 15:37   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-09-08 15:37 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: conor+dt, robh, devicetree, linux-pci

> From: Alex Elder <elder@riscstar.com>
> 
> Add a driver for the GPIO controller embedded in the Toshiba TC9563
> PCIe switch (and the Qualcomm QPS615). The device implements 35
> GPIOs using two register banks: three registers control the first 32
> GPIOs (20 and 21 are reserved) and three more control GPIOs 32-36.
> GPIOs 22-24, 27-28, 31, and 34 are input-only.
> 
> The driver is registered as an auxiliary device driver. The TC9563
> power controller creates the auxiliary device and provides a regmap
> that gives access to the GPIO registers, so the gpio-regmap helpers
> are used to implement the GPIO chip.
> 
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Co-developed-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=2


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

* Re: [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller
  2026-09-08 15:28 ` [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller Lorenzo Bianconi
@ 2026-09-08 15:41   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-09-08 15:41 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: linux-pci, conor+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but in `pci_pwrctrl_power_on_device()`, if powering on a child node fails (e.g., returning `-EPROBE_DEFER`), previously successfully powered-on sibling children within the same loop iteration are not rolled back, leading to left-on power states.
--

commit f88f26b5508f76a873435c0070f5f6adab4564f7
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller

This commit adds the TC9563 embedded GPIO controller to the QCS6490-RB3Gen2
device tree. It describes the switch node as a GPIO controller and uses it
to drive the PERST# reset lines for two external downstream PCIe ports
(pcie@1,0 and pcie@2,0).

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index a13315bf0fb07..4da7342ca90d6 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts

[ ... ]

> @@ -891,6 +891,9 @@ pcie@0,0 {
>  		pinctrl-0 = <&tc9563_resx_n>;
>  		pinctrl-names = "default";
>  
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +
>  		pcie1_switch0_dsp1: pcie@1,0 {
>  			reg = <0x20800 0x0 0x0 0x0 0x0>;
>  			#address-cells = <3>;
> @@ -899,6 +902,7 @@ pcie1_switch0_dsp1: pcie@1,0 {
>  			device_type = "pci";
>  			ranges;
>  			bus-range = <0x3 0xff>;
> +			reset-gpios = <&tc9563 2 GPIO_ACTIVE_LOW>;
>  		};
>  
>  		pcie@2,0 {
> @@ -909,6 +913,7 @@ pcie@2,0 {
>  			device_type = "pci";
>  			ranges;
>  			bus-range = <0x4 0xff>;
> +			reset-gpios = <&tc9563 3 GPIO_ACTIVE_LOW>;

[Severity: High]
This isn't a bug introduced by this patch, but adding multiple downstream PCIe
ports under a single power-controlled switch node here exposes a pre-existing
resource leak in the PCI power control core.

In pci_pwrctrl_power_on_device(), if powering on a child node fails (for
example, returning -EPROBE_DEFER for pcie@2,0), previously successfully
powered-on sibling children within the same loop iteration (like pcie@1,0)
are not rolled back:

drivers/pci/pwrctrl/core.c:pci_pwrctrl_power_on_device() {
    ...
    for_each_available_child_of_node_scoped(np, child) {
        ret = pci_pwrctrl_power_on_device(child);
        if (ret)
            return ret;
    }
    ...
}

Because the function returns immediately on failure, the regulator and clock
enable counts for the earlier siblings are permanently leaked. Since probe
deferral is a standard boot path, this leads to unbounded resource enable
increments.

Should pci_pwrctrl_power_on_device() be updated to power off successfully
processed children during the error path before returning?

>  
>  			/* Renesas μPD720201 PCIe USB3.0 Host Controller */
>  			usb-controller@0,0 {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=5

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

* Re: [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
  2026-09-08 15:28 ` [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support Lorenzo Bianconi
@ 2026-09-08 15:45   ` sashiko-bot
  2026-09-08 15:55     ` Lorenzo Bianconi
  0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-09-08 15:45 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: robh, conor+dt, linux-pci, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Devm vs manual cleanup ordering bug: Auxiliary device is unregistered via devres after the hardware is manually powered off and the I2C adapter reference is dropped.
--

commit ee51cc5ce17159f5e9569310449014a414a8580e
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
    
The commit adds GPIO auxiliary device support to the TC9563 PCI power control
driver, allowing a companion GPIO driver to handle per-port reset signals.
It passes the tc9563 regmap to the auxiliary device and uses devres to
manage unregistration.

> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 59ad219c26c02..3fb862105fa58 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
[ ... ]
> @@ -393,6 +398,77 @@ static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
>  	return 0;
>  }
>  
> +static void tc9563_pwrctrl_adev_release(struct device *dev)
[ ... ]
> +static int tc9563_pwrctrl_adev_add(struct device *dev, const char *name,
> +				   struct device_node *of_node,
> +				   void *priv_data)
> +{
> +	struct auxiliary_device *adev;
[ ... ]
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(dev, tc9563_pwrctrl_adev_remove, adev);

[Severity: High]
Does registering the auxiliary device unregistration to the devm cleanup phase
cause a teardown ordering issue?

Because devm actions execute after .remove() returns, tc9563_pwrctrl_remove()
will execute first:

static void tc9563_pwrctrl_remove(struct platform_device *pdev)
{
	...
	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
	i2c_put_adapter(tc9563->adapter);
}

Will this cause the hardware to be manually powered off and the I2C adapter
reference dropped before the child device is unregistered?

Could this lead to I2C transactions on a powered-off device if the GPIO
controller is accessed during the unbind window, and a potential use-after-free
if the adapter module is unloaded while the dummy client and regmap are still
active?

> +}
[ ... ]
> @@ -596,6 +672,10 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
>  	tc9563->pwrctrl.power_on = tc9563_pwrctrl_power_on;
>  	tc9563->pwrctrl.power_off = tc9563_pwrctrl_power_off;
>  
> +	ret = tc9563_pwrctrl_add_gpio_adev(tc9563);
> +	if (ret)
> +		goto remove_i2c;
> +
>  	ret = devm_pci_pwrctrl_device_set_ready(dev, &tc9563->pwrctrl);
>  	if (ret)
>  		goto power_off;
>  
>  	return 0;
>  
>  power_off:
>  	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
>  remove_i2c:
>  	i2c_put_adapter(tc9563->adapter);
>  	return ret;

[Severity: High]
Is the same teardown ordering regression reproduced in this error path?

If devm_pci_pwrctrl_device_set_ready() fails, the code jumps to power_off,
which manually powers off the hardware and drops the I2C adapter reference.
Then probe returns the error, and devres will clean up the auxiliary device
afterward.

Could this create a race window where the child device is fully bound
and accessible to userspace or other subsystems while the underlying hardware is
already powered off?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=3

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

* Re: [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API
  2026-09-08 15:28 ` [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API Lorenzo Bianconi
@ 2026-09-08 15:45   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-09-08 15:45 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: robh, conor+dt, devicetree, linux-pci

> Remove the local TC9563_GPIO_MASK and TC9563_GPIO_DEASSERT_BITS
> definitions, which are no longer used after switching to the GPIO
> descriptor API. Move TC9563_GPIO_CONFIG and TC9563_RESET_GPIO
> definitions in tc9563.h header file.
> 
> Replace the direct regmap-based per-port reset logic in
> assert_deassert_reset() with gpiod_direction_output() calls, falling
> back to the legacy regmap approach only when no reset-gpios DT
> property is present for a given port.
> 
> Add the reset GPIO pointer to struct tc9563_pwrctrl_cfg and introduce
> tc9563_pwrctrl_parse_reset_line() to look up reset-gpios from each
> PCI downstream port child node. The lookup is done lazily at the
> beginning of power_on(), returning -EPROBE_DEFER until the GPIO chip
> is registered.
> 
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Reviewed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=4


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

* Re: [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller
  2026-09-08 15:34   ` sashiko-bot
@ 2026-09-08 15:47     ` Lorenzo Bianconi
  0 siblings, 0 replies; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:47 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: devicetree, conor+dt, linux-pci, robh

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added GPIO provider capabilities 'gpio-controller' and '#gpio-cells' are missing from the schema's 'required' list.
> --
> 
> commit 6d6cb30a1eb86b847e98eea66d5ba6cc77a091a7
> Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> 
> dt-bindings: PCI: toshiba,tc9563: Document embedded GPIO controller
> 
> The TC9563 PCIe switch embeds a GPIO controller providing 37 GPIO
> lines. Describe the switch node itself as the embedded GPIO controller
> and document the per-port reset-gpios property.
> 
> > diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> > index f084830c6d0cb..2f03e4e45114d 100644
> > --- a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> > +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> > @@ -31,6 +31,11 @@ properties:
> >      description:
> >        GPIO controlling the RESX# pin.
> >  
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    const: 2
> > +
> 
> [Severity: Medium]
> Should these newly added GPIO provider capabilities be included in the
> schema's required properties list?
> 
> Looking at the required list in
> Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml:
> 
> required:
>   - resx-gpios
>   - vdd18-supply
>   - vdd09-supply
>   - vddc-supply
>   - vddio1-supply
>   - vddio2-supply
>   - vddio18-supply
>   - i2c-parent
> 
> If the new capabilities are omitted from this list, device tree nodes missing
> gpio-controller and #gpio-cells will silently pass schema validation,
> allowing incomplete configurations to enter the device tree.
> 
> At runtime, dependent nodes (such as the downstream ports attempting to use
> reset-gpios) could fail when the provider is misconfigured.

In order to introduce any ABI change, we decided to not define 'gpio-controller'
and '#gpio-cells' properties as required.

Regards,
Lorenzo

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
  2026-09-08 15:45   ` sashiko-bot
@ 2026-09-08 15:55     ` Lorenzo Bianconi
  0 siblings, 0 replies; 13+ messages in thread
From: Lorenzo Bianconi @ 2026-09-08 15:55 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: robh, conor+dt, linux-pci, devicetree

[-- Attachment #1: Type: text/plain, Size: 4157 bytes --]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Devm vs manual cleanup ordering bug: Auxiliary device is unregistered via devres after the hardware is manually powered off and the I2C adapter reference is dropped.
> --
> 
> commit ee51cc5ce17159f5e9569310449014a414a8580e
> Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> 
> PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
>     
> The commit adds GPIO auxiliary device support to the TC9563 PCI power control
> driver, allowing a companion GPIO driver to handle per-port reset signals.
> It passes the tc9563 regmap to the auxiliary device and uses devres to
> manage unregistration.
> 
> > diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> > index 59ad219c26c02..3fb862105fa58 100644
> > --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> > +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> [ ... ]
> > @@ -393,6 +398,77 @@ static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
> >  	return 0;
> >  }
> >  
> > +static void tc9563_pwrctrl_adev_release(struct device *dev)
> [ ... ]
> > +static int tc9563_pwrctrl_adev_add(struct device *dev, const char *name,
> > +				   struct device_node *of_node,
> > +				   void *priv_data)
> > +{
> > +	struct auxiliary_device *adev;
> [ ... ]
> > +	ret = auxiliary_device_add(adev);
> > +	if (ret) {
> > +		auxiliary_device_uninit(adev);
> > +		return ret;
> > +	}
> > +
> > +	return devm_add_action_or_reset(dev, tc9563_pwrctrl_adev_remove, adev);
> 
> [Severity: High]
> Does registering the auxiliary device unregistration to the devm cleanup phase
> cause a teardown ordering issue?
> 
> Because devm actions execute after .remove() returns, tc9563_pwrctrl_remove()
> will execute first:
> 
> static void tc9563_pwrctrl_remove(struct platform_device *pdev)
> {
> 	...
> 	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
> 	i2c_put_adapter(tc9563->adapter);
> }
> 
> Will this cause the hardware to be manually powered off and the I2C adapter
> reference dropped before the child device is unregistered?
> 
> Could this lead to I2C transactions on a powered-off device if the GPIO
> controller is accessed during the unbind window, and a potential use-after-free
> if the adapter module is unloaded while the dummy client and regmap are still
> active?

I think this is the issue already reported in v3.
I guess this can't really occur since i2c_adapter refcount is 2 after probe
(of_get_i2c_adapter_by_node()) and devm_i2c_new_dummy_device()). During
teardown phase, the first reference is released (i2c_put_adapter()) but the
second one is released just after aux device is destroyed.

> 
> > +}
> [ ... ]
> > @@ -596,6 +672,10 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> >  	tc9563->pwrctrl.power_on = tc9563_pwrctrl_power_on;
> >  	tc9563->pwrctrl.power_off = tc9563_pwrctrl_power_off;
> >  
> > +	ret = tc9563_pwrctrl_add_gpio_adev(tc9563);
> > +	if (ret)
> > +		goto remove_i2c;
> > +
> >  	ret = devm_pci_pwrctrl_device_set_ready(dev, &tc9563->pwrctrl);
> >  	if (ret)
> >  		goto power_off;
> >  
> >  	return 0;
> >  
> >  power_off:
> >  	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
> >  remove_i2c:
> >  	i2c_put_adapter(tc9563->adapter);
> >  	return ret;
> 
> [Severity: High]
> Is the same teardown ordering regression reproduced in this error path?
> 
> If devm_pci_pwrctrl_device_set_ready() fails, the code jumps to power_off,
> which manually powers off the hardware and drops the I2C adapter reference.
> Then probe returns the error, and devres will clean up the auxiliary device
> afterward.
> 
> Could this create a race window where the child device is fully bound
> and accessible to userspace or other subsystems while the underlying hardware is
> already powered off?

For the above reason I guess this issue can't occur.

Regards,
Lorenzo

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=3

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2026-09-08 15:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 15:28 [PATCH v4 0/5] PCI/pwrctrl: tc9563: Introduce support for embedded GPIO controller Lorenzo Bianconi
2026-09-08 15:28 ` [PATCH v4 1/5] dt-bindings: PCI: toshiba,tc9563: Document " Lorenzo Bianconi
2026-09-08 15:34   ` sashiko-bot
2026-09-08 15:47     ` Lorenzo Bianconi
2026-09-08 15:28 ` [PATCH v4 2/5] gpio: tc9563: Add support for the " Lorenzo Bianconi
2026-09-08 15:37   ` sashiko-bot
2026-09-08 15:28 ` [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support Lorenzo Bianconi
2026-09-08 15:45   ` sashiko-bot
2026-09-08 15:55     ` Lorenzo Bianconi
2026-09-08 15:28 ` [PATCH v4 4/5] PCI/pwrctrl: tc9563: Switch per-port reset to GPIO descriptor API Lorenzo Bianconi
2026-09-08 15:45   ` sashiko-bot
2026-09-08 15:28 ` [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller Lorenzo Bianconi
2026-09-08 15:41   ` sashiko-bot

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