linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
@ 2023-12-13  9:28 Sherry Sun
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Sherry Sun @ 2023-12-13  9:28 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: linux-imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel

Add pci host wakeup feature for imx platforms. The host wake pin is a
standard feature in the PCIe bus specification, so we can add this
property under PCI dts node to support the host gpio wakeup feature.

Example of configuring the corresponding dts property under the PCI node:
    wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;

---
changes in V2:
1. Rename host-wake-gpio property to wake-gpios.
2. Improve the wake-gpios property description in the dt-binding doc to avoid
confusion.
3. Remove unnecessary debugging info in host_wake_irq_handler().
4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error paths.
5. Use dev_err_probe() to simplify error path code.
---

Sherry Sun (4):
  PCI: imx6: Add pci host wakeup support on imx platforms.
  dt-bindings: imx6q-pcie: Add wake-gpios property
  arm64: dts: imx8mp-evk: add wake-gpios property for pci bus
  arm64: dts: imx8mq-evk: add wake-gpios property for pci bus

 .../bindings/pci/fsl,imx6q-pcie.yaml          |  6 ++
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts  |  2 +
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  2 +
 drivers/pci/controller/dwc/pci-imx6.c         | 60 +++++++++++++++++++
 4 files changed, 70 insertions(+)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
@ 2023-12-13  9:28 ` Sherry Sun
  2023-12-13 19:57   ` Bjorn Helgaas
                     ` (3 more replies)
  2023-12-13  9:28 ` [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property Sherry Sun
                   ` (3 subsequent siblings)
  4 siblings, 4 replies; 17+ messages in thread
From: Sherry Sun @ 2023-12-13  9:28 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: linux-imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel

Add pci host wakeup feature for imx platforms.
Example of configuring the corresponding dts property under the PCI
node:
wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec..0cf7f21adff8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -72,6 +72,7 @@ struct imx6_pcie_drvdata {
 struct imx6_pcie {
 	struct dw_pcie		*pci;
 	int			reset_gpio;
+	int			host_wake_irq;
 	bool			gpio_active_high;
 	bool			link_is_up;
 	struct clk		*pcie_bus;
@@ -1237,11 +1238,44 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 	return 0;
 }
 
+static int imx6_pcie_suspend(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+	if (imx6_pcie->host_wake_irq >= 0)
+		enable_irq_wake(imx6_pcie->host_wake_irq);
+
+	return 0;
+}
+
+static int imx6_pcie_resume(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+	if (imx6_pcie->host_wake_irq >= 0)
+		disable_irq_wake(imx6_pcie->host_wake_irq);
+
+	return 0;
+}
+
 static const struct dev_pm_ops imx6_pcie_pm_ops = {
 	NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
 				  imx6_pcie_resume_noirq)
+	SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend, imx6_pcie_resume)
 };
 
+irqreturn_t host_wake_irq_handler(int irq, void *priv)
+{
+	struct imx6_pcie *imx6_pcie = priv;
+	struct device *dev = imx6_pcie->pci->dev;
+
+	/* Notify PM core we are wakeup source */
+	pm_wakeup_event(dev, 0);
+	pm_system_wakeup();
+
+	return IRQ_HANDLED;
+}
+
 static int imx6_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1250,6 +1284,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	struct device_node *np;
 	struct resource *dbi_base;
 	struct device_node *node = dev->of_node;
+	struct gpio_desc *host_wake_gpio;
 	int ret;
 	u16 val;
 
@@ -1457,6 +1492,26 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 			val |= PCI_MSI_FLAGS_ENABLE;
 			dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
 		}
+
+		/* host wakeup support */
+		imx6_pcie->host_wake_irq = -1;
+		host_wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
+		if (IS_ERR(host_wake_gpio))
+			return PTR_ERR(host_wake_gpio);
+
+		if (host_wake_gpio != NULL) {
+			imx6_pcie->host_wake_irq = gpiod_to_irq(host_wake_gpio);
+			ret = devm_request_threaded_irq(dev, imx6_pcie->host_wake_irq, NULL,
+							host_wake_irq_handler,
+							IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+							"host_wake", imx6_pcie);
+			if (ret)
+				return dev_err_probe(dev, ret, "Failed to request host_wake_irq\n");
+
+			ret = device_init_wakeup(dev, true);
+			if (ret)
+				return dev_err_probe(dev, ret, "Failed to init host_wake_irq\n");
+		}
 	}
 
 	return 0;
@@ -1466,6 +1521,11 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
 {
 	struct imx6_pcie *imx6_pcie = platform_get_drvdata(pdev);
 
+	if (imx6_pcie->host_wake_irq >= 0) {
+		device_init_wakeup(&pdev->dev, false);
+		disable_irq(imx6_pcie->host_wake_irq);
+	}
+
 	/* bring down link, so bootloader gets clean state in case of reboot */
 	imx6_pcie_assert_core_reset(imx6_pcie);
 }
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
  2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
@ 2023-12-13  9:28 ` Sherry Sun
  2025-04-04  9:41   ` Francesco Dolcini
  2023-12-13  9:28 ` [PATCH V2 3/4] arm64: dts: imx8mp-evk: add wake-gpios property for pci bus Sherry Sun
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Sherry Sun @ 2023-12-13  9:28 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: linux-imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel

Add wake-gpios property that can be used to wakeup the host
processor.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 81bbb8728f0f..fba757d937e1 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -72,6 +72,12 @@ properties:
       L=operation state) (optional required).
     type: boolean
 
+  wake-gpios:
+    description: If present this property specifies WAKE# sideband signaling
+      to implement wakeup functionality. This is an input GPIO pin for the Root
+      Port mode here. Host drivers will wakeup the host using the IRQ
+      corresponding to the passed GPIO.
+
 required:
   - compatible
   - reg
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH V2 3/4] arm64: dts: imx8mp-evk: add wake-gpios property for pci bus
  2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
  2023-12-13  9:28 ` [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property Sherry Sun
@ 2023-12-13  9:28 ` Sherry Sun
  2023-12-13  9:28 ` [PATCH V2 4/4] arm64: dts: imx8mq-evk: " Sherry Sun
  2023-12-14  5:13 ` [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Manivannan Sadhasivam
  4 siblings, 0 replies; 17+ messages in thread
From: Sherry Sun @ 2023-12-13  9:28 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: linux-imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel

The host wake pin is a standard feature in the PCIe bus specification,
so we add this property under PCI dts node to enable the host wake
function.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f87fa5a948cc..4378b9c1308c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -542,6 +542,7 @@ &pcie {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pcie0>;
 	reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
 	vpcie-supply = <&reg_pcie0>;
 	status = "okay";
 };
@@ -772,6 +773,7 @@ pinctrl_pcie0: pcie0grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B	0x60 /* open drain, pull up */
 			MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07	0x40
+			MX8MP_IOMUXC_I2C4_SDA__GPIO5_IO21	0x1c4
 		>;
 	};
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH V2 4/4] arm64: dts: imx8mq-evk: add wake-gpios property for pci bus
  2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
                   ` (2 preceding siblings ...)
  2023-12-13  9:28 ` [PATCH V2 3/4] arm64: dts: imx8mp-evk: add wake-gpios property for pci bus Sherry Sun
@ 2023-12-13  9:28 ` Sherry Sun
  2023-12-14  5:13 ` [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Manivannan Sadhasivam
  4 siblings, 0 replies; 17+ messages in thread
From: Sherry Sun @ 2023-12-13  9:28 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: linux-imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel

The host wake pin is a standard feature in the PCIe bus specification,
so we add this property under PCI dts node to enable the host wake
function.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 7507548cdb16..b8463ef230c5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -367,6 +367,7 @@ &pcie1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pcie1>;
 	reset-gpio = <&gpio5 12 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 		 <&pcie0_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
@@ -545,6 +546,7 @@ pinctrl_pcie1: pcie1grp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_I2C4_SDA_PCIE2_CLKREQ_B		0x76
 			MX8MQ_IOMUXC_ECSPI2_MISO_GPIO5_IO12		0x16
+			MX8MQ_IOMUXC_ECSPI2_MOSI_GPIO5_IO11		0x41
 		>;
 	};
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
@ 2023-12-13 19:57   ` Bjorn Helgaas
  2023-12-14  4:08     ` Sherry Sun
  2023-12-13 21:03   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2023-12-13 19:57 UTC (permalink / raw)
  To: Sherry Sun
  Cc: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, linux-imx, linux-pci, linux-arm-kernel, devicetree,
	linux-kernel

Drop period at the end of subject line.  It only adds the possibility
of unnecessary line wrapping in git log.

On Wed, Dec 13, 2023 at 05:28:47PM +0800, Sherry Sun wrote:
> Add pci host wakeup feature for imx platforms.

s/pci/PCI/
s/imx/i.MX/ (based on how nxp.com web pages refer to it)

> Example of configuring the corresponding dts property under the PCI
> node:
> wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;

Add newline between paragraphs or wrap into a single paragraph.

> +		/* host wakeup support */
> +		imx6_pcie->host_wake_irq = -1;

AFAIK, 0 is an invalid IRQ value.  So why not drop this assignment
since imx6_pcie->host_wake_irq is 0 by default since it was allocated
with devm_kzalloc(), and test like this elsewhere:

  if (imx6_pcie->host_wake_irq) {
    enable_irq_wake(imx6_pcie->host_wake_irq)

> +		host_wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
> +		if (IS_ERR(host_wake_gpio))
> +			return PTR_ERR(host_wake_gpio);
> +
> +		if (host_wake_gpio != NULL) {

  if (host_wake_gpio)

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
  2023-12-13 19:57   ` Bjorn Helgaas
@ 2023-12-13 21:03   ` kernel test robot
  2023-12-14  9:56   ` kernel test robot
  2023-12-14 10:06   ` kernel test robot
  3 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-12-13 21:03 UTC (permalink / raw)
  To: Sherry Sun, hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: oe-kbuild-all, linux-imx, linux-pci, linux-arm-kernel, devicetree,
	linux-kernel

Hi Sherry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231214/202312140402.hZsD0IVQ-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312140402.hZsD0IVQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312140402.hZsD0IVQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: warning: no previous prototype for 'host_wake_irq_handler' [-Wmissing-prototypes]
    1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
         |             ^~~~~~~~~~~~~~~~~~~~~


vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c

  1266	
> 1267	irqreturn_t host_wake_irq_handler(int irq, void *priv)
  1268	{
  1269		struct imx6_pcie *imx6_pcie = priv;
  1270		struct device *dev = imx6_pcie->pci->dev;
  1271	
  1272		/* Notify PM core we are wakeup source */
  1273		pm_wakeup_event(dev, 0);
  1274		pm_system_wakeup();
  1275	
  1276		return IRQ_HANDLED;
  1277	}
  1278	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13 19:57   ` Bjorn Helgaas
@ 2023-12-14  4:08     ` Sherry Sun
  0 siblings, 0 replies; 17+ messages in thread
From: Sherry Sun @ 2023-12-14  4:08 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kw@linux.com, robh@kernel.org, bhelgaas@google.com,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2023年12月14日 3:58
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx
> platforms.
> 
> Drop period at the end of subject line.  It only adds the possibility of
> unnecessary line wrapping in git log.

Hi Bjorn, thanks for the comments, will do in V3.

> 
> On Wed, Dec 13, 2023 at 05:28:47PM +0800, Sherry Sun wrote:
> > Add pci host wakeup feature for imx platforms.
> 
> s/pci/PCI/
> s/imx/i.MX/ (based on how nxp.com web pages refer to it)
> 

Will do.

> > Example of configuring the corresponding dts property under the PCI
> > node:
> > wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
> 
> Add newline between paragraphs or wrap into a single paragraph.

Will do.

> 
> > +		/* host wakeup support */
> > +		imx6_pcie->host_wake_irq = -1;
> 
> AFAIK, 0 is an invalid IRQ value.  So why not drop this assignment since
> imx6_pcie->host_wake_irq is 0 by default since it was allocated with
> devm_kzalloc(), and test like this elsewhere:
> 
>   if (imx6_pcie->host_wake_irq) {
>     enable_irq_wake(imx6_pcie->host_wake_irq)

I plan to change the host_wake_irq to unsigned int type, and add following codes, then "if (imx6_pcie->host_wake_irq)" condition seems more reasonable, let me know if you have any further suggestions. thanks!
-                       imx6_pcie->host_wake_irq = gpiod_to_irq(host_wake_gpio);
+                       ret = gpiod_to_irq(host_wake_gpio);
+                       if (ret < 0)
+                               return dev_err_probe(dev, ret, "Failed to get IRQ for WAKE gpio\n");
+
+                       imx6_pcie->host_wake_irq = (unsigned int)ret;

> 
> > +		host_wake_gpio = devm_gpiod_get_optional(dev, "wake",
> GPIOD_IN);
> > +		if (IS_ERR(host_wake_gpio))
> > +			return PTR_ERR(host_wake_gpio);
> > +
> > +		if (host_wake_gpio != NULL) {
> 
>   if (host_wake_gpio)

Will do.

Best Regards
Sherry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
  2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
                   ` (3 preceding siblings ...)
  2023-12-13  9:28 ` [PATCH V2 4/4] arm64: dts: imx8mq-evk: " Sherry Sun
@ 2023-12-14  5:13 ` Manivannan Sadhasivam
  2023-12-14 10:03   ` Sherry Sun
  4 siblings, 1 reply; 17+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-14  5:13 UTC (permalink / raw)
  To: Sherry Sun
  Cc: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, linux-imx, linux-pci, linux-arm-kernel, devicetree,
	linux-kernel

On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote:
> Add pci host wakeup feature for imx platforms. The host wake pin is a
> standard feature in the PCIe bus specification, so we can add this
> property under PCI dts node to support the host gpio wakeup feature.
> 
> Example of configuring the corresponding dts property under the PCI node:
>     wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
> 

As you mentioned, WAKE# is a standard sideband signal defined in the PCI spec.
So the support for handling it has to be in the PCI core layer, not in the
host controller drivers.

There is already a series floating to add support for WAKE# in PCI core. Please
take a look:

https://lore.kernel.org/linux-pci/20230208111645.3863534-1-mmaddireddy@nvidia.com/

- Mani

> ---
> changes in V2:
> 1. Rename host-wake-gpio property to wake-gpios.
> 2. Improve the wake-gpios property description in the dt-binding doc to avoid
> confusion.
> 3. Remove unnecessary debugging info in host_wake_irq_handler().
> 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error paths.
> 5. Use dev_err_probe() to simplify error path code.
> ---
> 
> Sherry Sun (4):
>   PCI: imx6: Add pci host wakeup support on imx platforms.
>   dt-bindings: imx6q-pcie: Add wake-gpios property
>   arm64: dts: imx8mp-evk: add wake-gpios property for pci bus
>   arm64: dts: imx8mq-evk: add wake-gpios property for pci bus
> 
>  .../bindings/pci/fsl,imx6q-pcie.yaml          |  6 ++
>  arch/arm64/boot/dts/freescale/imx8mp-evk.dts  |  2 +
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  2 +
>  drivers/pci/controller/dwc/pci-imx6.c         | 60 +++++++++++++++++++
>  4 files changed, 70 insertions(+)
> 
> -- 
> 2.34.1
> 
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
  2023-12-13 19:57   ` Bjorn Helgaas
  2023-12-13 21:03   ` kernel test robot
@ 2023-12-14  9:56   ` kernel test robot
  2023-12-14 10:06   ` kernel test robot
  3 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-12-14  9:56 UTC (permalink / raw)
  To: Sherry Sun, hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: oe-kbuild-all, linux-imx, linux-pci, linux-arm-kernel, devicetree,
	linux-kernel

Hi Sherry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: alpha-randconfig-r112-20231214 (https://download.01.org/0day-ci/archive/20231214/202312141719.j5GCLQry-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231214/202312141719.j5GCLQry-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312141719.j5GCLQry-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: sparse: sparse: symbol 'host_wake_irq_handler' was not declared. Should it be static?
   drivers/pci/controller/dwc/pci-imx6.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/umh.h, include/linux/kmod.h, ...):
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false

vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c

  1266	
> 1267	irqreturn_t host_wake_irq_handler(int irq, void *priv)
  1268	{
  1269		struct imx6_pcie *imx6_pcie = priv;
  1270		struct device *dev = imx6_pcie->pci->dev;
  1271	
  1272		/* Notify PM core we are wakeup source */
  1273		pm_wakeup_event(dev, 0);
  1274		pm_system_wakeup();
  1275	
  1276		return IRQ_HANDLED;
  1277	}
  1278	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
  2023-12-14  5:13 ` [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Manivannan Sadhasivam
@ 2023-12-14 10:03   ` Sherry Sun
  2023-12-14 10:15     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 17+ messages in thread
From: Sherry Sun @ 2023-12-14 10:03 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kw@linux.com, robh@kernel.org, bhelgaas@google.com,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Sent: 2023年12月14日 13:13
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
>
> On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote:
> > Add pci host wakeup feature for imx platforms. The host wake pin is a
> > standard feature in the PCIe bus specification, so we can add this
> > property under PCI dts node to support the host gpio wakeup feature.
> >
> > Example of configuring the corresponding dts property under the PCI node:
> >     wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
> >
>
> As you mentioned, WAKE# is a standard sideband signal defined in the PCI
> spec.
> So the support for handling it has to be in the PCI core layer, not in the host
> controller drivers.
>
> There is already a series floating to add support for WAKE# in PCI core.
> Please take a look:
>
> https://lore.k/
> ernel.org%2Flinux-pci%2F20230208111645.3863534-1-
> mmaddireddy%40nvidia.com%2F&data=05%7C02%7Csherry.sun%40nxp.co
> m%7C0254c001df61498c09d408dbfc636f5c%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C638381276239824912%7CUnknown%7CTWFpbGZsb3
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> 3D%7C3000%7C%7C%7C&sdata=IoBAwTy0qeb0J6JrK0WRhI8A4ThUfkVx6mri
> ve%2BK5xs%3D&reserved=0

Hi Manivannan,
I checked the patch set, the implementation of host wake gpio is different from mine, I referred to the mmc bus cd(card detect) pin implementation and I think it is simpler and clearer.
Regarding whether the WAKE# support should be moved to PCI core layer, we may need more research and discussion. Thanks for your suggestions.

Best Regards
Sherry


>
> - Mani
>
> > ---
> > changes in V2:
> > 1. Rename host-wake-gpio property to wake-gpios.
> > 2. Improve the wake-gpios property description in the dt-binding doc
> > to avoid confusion.
> > 3. Remove unnecessary debugging info in host_wake_irq_handler().
> > 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error
> paths.
> > 5. Use dev_err_probe() to simplify error path code.
> > ---
> >
> > Sherry Sun (4):
> >   PCI: imx6: Add pci host wakeup support on imx platforms.
> >   dt-bindings: imx6q-pcie: Add wake-gpios property
> >   arm64: dts: imx8mp-evk: add wake-gpios property for pci bus
> >   arm64: dts: imx8mq-evk: add wake-gpios property for pci bus
> >
> >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  6 ++
> >  arch/arm64/boot/dts/freescale/imx8mp-evk.dts  |  2 +
> > arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  2 +
> >  drivers/pci/controller/dwc/pci-imx6.c         | 60 +++++++++++++++++++
> >  4 files changed, 70 insertions(+)
> >
> > --
> > 2.34.1
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
  2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
                     ` (2 preceding siblings ...)
  2023-12-14  9:56   ` kernel test robot
@ 2023-12-14 10:06   ` kernel test robot
  3 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-12-14 10:06 UTC (permalink / raw)
  To: Sherry Sun, hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam
  Cc: llvm, oe-kbuild-all, linux-imx, linux-pci, linux-arm-kernel,
	devicetree, linux-kernel

Hi Sherry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: arm-randconfig-004-20231213 (https://download.01.org/0day-ci/archive/20231214/202312141726.nPrR7WDt-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312141726.nPrR7WDt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312141726.nPrR7WDt-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: warning: no previous prototype for function 'host_wake_irq_handler' [-Wmissing-prototypes]
    1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
         |             ^
   drivers/pci/controller/dwc/pci-imx6.c:1267:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
         | ^
         | static 
   1 warning generated.


vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c

  1266	
> 1267	irqreturn_t host_wake_irq_handler(int irq, void *priv)
  1268	{
  1269		struct imx6_pcie *imx6_pcie = priv;
  1270		struct device *dev = imx6_pcie->pci->dev;
  1271	
  1272		/* Notify PM core we are wakeup source */
  1273		pm_wakeup_event(dev, 0);
  1274		pm_system_wakeup();
  1275	
  1276		return IRQ_HANDLED;
  1277	}
  1278	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
  2023-12-14 10:03   ` Sherry Sun
@ 2023-12-14 10:15     ` Manivannan Sadhasivam
  0 siblings, 0 replies; 17+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-14 10:15 UTC (permalink / raw)
  To: Sherry Sun
  Cc: Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kw@linux.com, robh@kernel.org, bhelgaas@google.com,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org

On Thu, Dec 14, 2023 at 10:03:51AM +0000, Sherry Sun wrote:
> 
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Sent: 2023年12月14日 13:13
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> > imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
> >
> > On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote:
> > > Add pci host wakeup feature for imx platforms. The host wake pin is a
> > > standard feature in the PCIe bus specification, so we can add this
> > > property under PCI dts node to support the host gpio wakeup feature.
> > >
> > > Example of configuring the corresponding dts property under the PCI node:
> > >     wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
> > >
> >
> > As you mentioned, WAKE# is a standard sideband signal defined in the PCI
> > spec.
> > So the support for handling it has to be in the PCI core layer, not in the host
> > controller drivers.
> >
> > There is already a series floating to add support for WAKE# in PCI core.
> > Please take a look:
> >
> > https://lore.k/
> > ernel.org%2Flinux-pci%2F20230208111645.3863534-1-
> > mmaddireddy%40nvidia.com%2F&data=05%7C02%7Csherry.sun%40nxp.co
> > m%7C0254c001df61498c09d408dbfc636f5c%7C686ea1d3bc2b4c6fa92cd99c5
> > c301635%7C0%7C0%7C638381276239824912%7CUnknown%7CTWFpbGZsb3
> > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> > 3D%7C3000%7C%7C%7C&sdata=IoBAwTy0qeb0J6JrK0WRhI8A4ThUfkVx6mri
> > ve%2BK5xs%3D&reserved=0
> 
> Hi Manivannan,
> I checked the patch set, the implementation of host wake gpio is different from mine, I referred to the mmc bus cd(card detect) pin implementation and I think it is simpler and clearer.

It's not just about simple and clear code, but about scalability. See below.

> Regarding whether the WAKE# support should be moved to PCI core layer, we may need more research and discussion. Thanks for your suggestions.
> 

We can research and come up with a better solution, but the implementation has
to be done in the PCI core layer. Otherwise, host controllers supporting WAKE#
has to duplicate the code which is common.

- Mani

> Best Regards
> Sherry
> 
> 
> >
> > - Mani
> >
> > > ---
> > > changes in V2:
> > > 1. Rename host-wake-gpio property to wake-gpios.
> > > 2. Improve the wake-gpios property description in the dt-binding doc
> > > to avoid confusion.
> > > 3. Remove unnecessary debugging info in host_wake_irq_handler().
> > > 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error
> > paths.
> > > 5. Use dev_err_probe() to simplify error path code.
> > > ---
> > >
> > > Sherry Sun (4):
> > >   PCI: imx6: Add pci host wakeup support on imx platforms.
> > >   dt-bindings: imx6q-pcie: Add wake-gpios property
> > >   arm64: dts: imx8mp-evk: add wake-gpios property for pci bus
> > >   arm64: dts: imx8mq-evk: add wake-gpios property for pci bus
> > >
> > >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  6 ++
> > >  arch/arm64/boot/dts/freescale/imx8mp-evk.dts  |  2 +
> > > arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  2 +
> > >  drivers/pci/controller/dwc/pci-imx6.c         | 60 +++++++++++++++++++
> > >  4 files changed, 70 insertions(+)
> > >
> > > --
> > > 2.34.1
> > >
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
  2023-12-13  9:28 ` [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property Sherry Sun
@ 2025-04-04  9:41   ` Francesco Dolcini
  2025-04-07  7:18     ` Sherry Sun
  0 siblings, 1 reply; 17+ messages in thread
From: Francesco Dolcini @ 2025-04-04  9:41 UTC (permalink / raw)
  To: Sherry Sun
  Cc: hongxing.zhu, l.stach, lpieralisi, kw, robh, bhelgaas,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, linux-imx, linux-pci, linux-arm-kernel, devicetree,
	linux-kernel

Hello

On Wed, Dec 13, 2023 at 05:28:48PM +0800, Sherry Sun wrote:
> Add wake-gpios property that can be used to wakeup the host
> processor.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index 81bbb8728f0f..fba757d937e1 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -72,6 +72,12 @@ properties:
>        L=operation state) (optional required).
>      type: boolean
>  
> +  wake-gpios:
> +    description: If present this property specifies WAKE# sideband signaling
> +      to implement wakeup functionality. This is an input GPIO pin for the Root
> +      Port mode here. Host drivers will wakeup the host using the IRQ
> +      corresponding to the passed GPIO.
> +

From what I know it is possible to share the same WAKE# signal for
multiple root ports. Is this going to work fine with this binding? Same
question on the driver.

We do have design exactly like that, so it's not a theoretical question.

Francesco




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

* RE: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
  2025-04-04  9:41   ` Francesco Dolcini
@ 2025-04-07  7:18     ` Sherry Sun
  2025-04-07  7:23       ` Francesco Dolcini
  0 siblings, 1 reply; 17+ messages in thread
From: Sherry Sun @ 2025-04-07  7:18 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kw@linux.com, robh@kernel.org, bhelgaas@google.com,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Friday, April 4, 2025 5:42 PM
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
> 
> Hello
> 
> On Wed, Dec 13, 2023 at 05:28:48PM +0800, Sherry Sun wrote:
> > Add wake-gpios property that can be used to wakeup the host processor.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > index 81bbb8728f0f..fba757d937e1 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > @@ -72,6 +72,12 @@ properties:
> >        L=operation state) (optional required).
> >      type: boolean
> >
> > +  wake-gpios:
> > +    description: If present this property specifies WAKE# sideband signaling
> > +      to implement wakeup functionality. This is an input GPIO pin for the
> Root
> > +      Port mode here. Host drivers will wakeup the host using the IRQ
> > +      corresponding to the passed GPIO.
> > +
> 
> From what I know it is possible to share the same WAKE# signal for multiple
> root ports. Is this going to work fine with this binding? Same question on the
> driver.
> 
> We do have design exactly like that, so it's not a theoretical question.
> 

Hi Francesco,
The current design doesn't support such case, maybe some changes in the driver could achieve that (mark the wake-gpio as GPIOD_FLAGS_BIT_NONEXCLUSIVE and the interrupt as IRQF_SHARED, etc.).
But usually each RC has its own WAKE# pin assigned. We have not come across a case where multiple RC share the same WAKE# pin.

Best Regards
Sherry


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

* Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
  2025-04-07  7:18     ` Sherry Sun
@ 2025-04-07  7:23       ` Francesco Dolcini
  2025-04-07  9:38         ` Sherry Sun
  0 siblings, 1 reply; 17+ messages in thread
From: Francesco Dolcini @ 2025-04-07  7:23 UTC (permalink / raw)
  To: Sherry Sun
  Cc: Francesco Dolcini, Hongxing Zhu, l.stach@pengutronix.de,
	lpieralisi@kernel.org, kw@linux.com, robh@kernel.org,
	bhelgaas@google.com, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org

On Mon, Apr 07, 2025 at 07:18:32AM +0000, Sherry Sun wrote:
> 
> 
> > -----Original Message-----
> > From: Francesco Dolcini <francesco@dolcini.it>
> > Sent: Friday, April 4, 2025 5:42 PM
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> > imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
> > 
> > Hello
> > 
> > On Wed, Dec 13, 2023 at 05:28:48PM +0800, Sherry Sun wrote:
> > > Add wake-gpios property that can be used to wakeup the host processor.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > index 81bbb8728f0f..fba757d937e1 100644
> > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > @@ -72,6 +72,12 @@ properties:
> > >        L=operation state) (optional required).
> > >      type: boolean
> > >
> > > +  wake-gpios:
> > > +    description: If present this property specifies WAKE# sideband signaling
> > > +      to implement wakeup functionality. This is an input GPIO pin for the
> > Root
> > > +      Port mode here. Host drivers will wakeup the host using the IRQ
> > > +      corresponding to the passed GPIO.
> > > +
> > 
> > From what I know it is possible to share the same WAKE# signal for multiple
> > root ports. Is this going to work fine with this binding? Same question on the
> > driver.
> > 
> > We do have design exactly like that, so it's not a theoretical question.
> > 
> The current design doesn't support such case, maybe some changes in the
> driver could achieve that (mark the wake-gpio as GPIOD_FLAGS_BIT_NONEXCLUSIVE
> and the interrupt as IRQF_SHARED, etc.).

Can you consider implementing this?

> But usually each RC has its own WAKE# pin assigned. We have not come across a
> case where multiple RC share the same WAKE# pin.

We do have such design, with an NXP iMX95 SoC, available now.

Francesco



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

* RE: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
  2025-04-07  7:23       ` Francesco Dolcini
@ 2025-04-07  9:38         ` Sherry Sun
  0 siblings, 0 replies; 17+ messages in thread
From: Sherry Sun @ 2025-04-07  9:38 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kw@linux.com, robh@kernel.org, bhelgaas@google.com,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Monday, April 7, 2025 3:23 PM
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: Francesco Dolcini <francesco@dolcini.it>; Hongxing Zhu
> <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property
> 
> On Mon, Apr 07, 2025 at 07:18:32AM +0000, Sherry Sun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Francesco Dolcini <francesco@dolcini.it>
> > > Sent: Friday, April 4, 2025 5:42 PM
> > > To: Sherry Sun <sherry.sun@nxp.com>
> > > Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de;
> > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> > > bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> > > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> > > imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm-
> > > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org
> > > Subject: Re: [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios
> > > property
> > >
> > > Hello
> > >
> > > On Wed, Dec 13, 2023 at 05:28:48PM +0800, Sherry Sun wrote:
> > > > Add wake-gpios property that can be used to wakeup the host processor.
> > > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6
> > > > ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > index 81bbb8728f0f..fba757d937e1 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > @@ -72,6 +72,12 @@ properties:
> > > >        L=operation state) (optional required).
> > > >      type: boolean
> > > >
> > > > +  wake-gpios:
> > > > +    description: If present this property specifies WAKE# sideband
> signaling
> > > > +      to implement wakeup functionality. This is an input GPIO
> > > > + pin for the
> > > Root
> > > > +      Port mode here. Host drivers will wakeup the host using the IRQ
> > > > +      corresponding to the passed GPIO.
> > > > +
> > >
> > > From what I know it is possible to share the same WAKE# signal for
> > > multiple root ports. Is this going to work fine with this binding?
> > > Same question on the driver.
> > >
> > > We do have design exactly like that, so it's not a theoretical question.
> > >
> > The current design doesn't support such case, maybe some changes in
> > the driver could achieve that (mark the wake-gpio as
> > GPIOD_FLAGS_BIT_NONEXCLUSIVE and the interrupt as IRQF_SHARED,
> etc.).
> 
> Can you consider implementing this?
> 
> > But usually each RC has its own WAKE# pin assigned. We have not come
> > across a case where multiple RC share the same WAKE# pin.
> 
> We do have such design, with an NXP iMX95 SoC, available now.
> 

Hi Francesco,

Now the patch set is pending, please check the comment under
https://patchwork.kernel.org/project/linux-pci/cover/20231213092850.1706042-1-sherry.sun@nxp.com/ .
Seems this property should be put into the common PCI root port schema,
now it relies on the pci-bus.yaml splitting job Rob is doing.

Best Regards
Sherry


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

end of thread, other threads:[~2025-04-07  9:42 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-13  9:28 [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Sherry Sun
2023-12-13  9:28 ` [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms Sherry Sun
2023-12-13 19:57   ` Bjorn Helgaas
2023-12-14  4:08     ` Sherry Sun
2023-12-13 21:03   ` kernel test robot
2023-12-14  9:56   ` kernel test robot
2023-12-14 10:06   ` kernel test robot
2023-12-13  9:28 ` [PATCH V2 2/4] dt-bindings: imx6q-pcie: Add wake-gpios property Sherry Sun
2025-04-04  9:41   ` Francesco Dolcini
2025-04-07  7:18     ` Sherry Sun
2025-04-07  7:23       ` Francesco Dolcini
2025-04-07  9:38         ` Sherry Sun
2023-12-13  9:28 ` [PATCH V2 3/4] arm64: dts: imx8mp-evk: add wake-gpios property for pci bus Sherry Sun
2023-12-13  9:28 ` [PATCH V2 4/4] arm64: dts: imx8mq-evk: " Sherry Sun
2023-12-14  5:13 ` [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support Manivannan Sadhasivam
2023-12-14 10:03   ` Sherry Sun
2023-12-14 10:15     ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).