[PATCH v4 0/6] add USB2.0 support for i.MX95-19x19 EVK board

[PATCH v4 0/6] add USB2.0 support for i.MX95-19x19 EVK board

[Xu Yang]

The i.MX95-19x19 EVK board features a USB 2.0 Type-A port, with this
series primarily introducing USB 2.0 support. In the i.MX95 architecture,
the USB wake-up handling mechanism is integrated within the HSIO block
control module, utilizing a dedicated wake-up interrupt. Therefore, we
also implemented corresponding wake-up logic code to properly manage this
functionality.

For detailed changes can refer to patch commit log.

v4:
 - add blkctl NULL checking
 - warning if wakeup setting is missing for imx95
 - add Tb tag 

v3:
 - fix possible build break
 - remove usbmisc_imx95_init()
 - add Rb tag in some patches

v2:
 - apply some suggestions
 - improve wakeup interrupt handling
 - add Rb tag in some patches

Xu Yang (6):
  dt-bindings: usb: chipidea: Add i.MX95 compatible string
    'fsl,imx95-usb'
  dt-bindings: usb: usbmisc-imx: add support for i.MX95 platform
  usb: chipidea: imx: add wakeup interrupt handling
  usb: chipidea: imx: add HSIO Block Control wakeup setting
  arm64: dts: imx95: add USB2.0 nodes
  arm64: dts: imx95-19x19-evk: enable USB2.0 node

 .../bindings/usb/chipidea,usb2-common.yaml    |  3 +
 .../bindings/usb/chipidea,usb2-imx.yaml       | 24 +++++-
 .../devicetree/bindings/usb/fsl,usbmisc.yaml  | 23 +++++-
 .../boot/dts/freescale/imx95-19x19-evk.dts    | 16 ++++
 arch/arm64/boot/dts/freescale/imx95.dtsi      | 30 ++++++++
 drivers/usb/chipidea/ci_hdrc_imx.c            | 35 +++++++++
 drivers/usb/chipidea/usbmisc_imx.c            | 77 +++++++++++++++++++
 7 files changed, 206 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v4 1/6] dt-bindings: usb: chipidea: Add i.MX95 compatible string 'fsl,imx95-usb'

[Xu Yang]

The i.MX95 USB2.0 controller is mostly compatible with i.MX7D, except it
requires a second interrupt for wakeup handling. Add the compatible string
for the i.MX95 platform, add the iommus property, and enforce the
interrupt property restriction. Keep the same restriction for existing
compatible strings.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - no changes
Changes in v3:
 - add Rb tag
Changes in v2:
 - improve interrupts description
 - improve subject and commit message
---
 .../bindings/usb/chipidea,usb2-common.yaml    |  3 +++
 .../bindings/usb/chipidea,usb2-imx.yaml       | 24 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/chipidea,usb2-common.yaml b/Documentation/devicetree/bindings/usb/chipidea,usb2-common.yaml
index d2a7d2ecf48a..10020af15afc 100644
--- a/Documentation/devicetree/bindings/usb/chipidea,usb2-common.yaml
+++ b/Documentation/devicetree/bindings/usb/chipidea,usb2-common.yaml
@@ -42,6 +42,9 @@ properties:
 
   phy_type: true
 
+  iommus:
+    maxItems: 1
+
   itc-setting:
     description:
       interrupt threshold control register control, the setting should be
diff --git a/Documentation/devicetree/bindings/usb/chipidea,usb2-imx.yaml b/Documentation/devicetree/bindings/usb/chipidea,usb2-imx.yaml
index 8f6136f5d72e..51014955ab3c 100644
--- a/Documentation/devicetree/bindings/usb/chipidea,usb2-imx.yaml
+++ b/Documentation/devicetree/bindings/usb/chipidea,usb2-imx.yaml
@@ -41,6 +41,7 @@ properties:
               - fsl,imx8mm-usb
               - fsl,imx8mn-usb
               - fsl,imx93-usb
+              - fsl,imx95-usb
           - const: fsl,imx7d-usb
           - const: fsl,imx27-usb
       - items:
@@ -54,7 +55,11 @@ properties:
     maxItems: 1
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: USB controller interrupt or combine USB controller
+                     and wakeup interrupts.
+      - description: Wakeup interrupt
 
   clocks:
     minItems: 1
@@ -191,6 +196,7 @@ allOf:
           contains:
             enum:
               - fsl,imx93-usb
+              - fsl,imx95-usb
     then:
       properties:
         clocks:
@@ -238,6 +244,22 @@ allOf:
           maxItems: 1
         clock-names: false
 
+  # imx95 soc use two interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx95-usb
+    then:
+      properties:
+        interrupts:
+          minItems: 2
+    else:
+      properties:
+        interrupts:
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

[PATCH v4 2/6] dt-bindings: usb: usbmisc-imx: add support for i.MX95 platform

[Xu Yang]

Add compatible string "fsl,imx95-usbmisc" for i.MX95 platform and
restriction on reg property.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - no changes
Changes in v3:
 - add Rb tag
Changes in v2:
 - improve subject and add Rb tag
---
 .../devicetree/bindings/usb/fsl,usbmisc.yaml  | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
index 0a6e7ac1b37e..019435540df0 100644
--- a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
+++ b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
@@ -34,6 +34,7 @@ properties:
               - fsl,imx8mm-usbmisc
               - fsl,imx8mn-usbmisc
               - fsl,imx8ulp-usbmisc
+              - fsl,imx95-usbmisc
           - const: fsl,imx7d-usbmisc
           - const: fsl,imx6q-usbmisc
       - items:
@@ -45,7 +46,10 @@ properties:
     maxItems: 1
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Base and length of the Wrapper module register
+      - description: Base and length of the HSIO Block Control register
 
   '#index-cells':
     const: 1
@@ -56,6 +60,23 @@ required:
   - compatible
   - reg
 
+allOf:
+  # imx95 soc needs use HSIO Block Control
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx95-usbmisc
+    then:
+      properties:
+        reg:
+          minItems: 2
+    else:
+      properties:
+        reg:
+          maxItems: 1
+
 additionalProperties: false
 
 examples:
-- 
2.34.1

[PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Xu Yang]

In previous imx platform, normal USB controller interrupt and wakeup
interrupt are bound to one irq line. However, it changes on latest
i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
This will add wakeup interrupt handling for i.MX95 to support various
wakeup events.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v4:
 - warning if no irq provided for imx95
Changes in v3:
 - include <linux/irq.h> to fix possible build issue
Changes in v2:
 - rename irq to wakeup_irq
 - disable irq by default
 - enable irq when suspend, disable irq when resume
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 1a7fc638213e..2baae9e6e673 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/irq.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
@@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
 	struct clk *clk;
 	struct clk *clk_wakeup;
 	struct imx_usbmisc_data *usbmisc_data;
+	int wakeup_irq;
 	bool supports_runtime_pm;
 	bool override_phy_control;
 	bool in_lpm;
@@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
 	return ret;
 }
 
+static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
+{
+	struct ci_hdrc_imx_data *imx_data = data;
+
+	disable_irq_nosync(irq);
+	pm_runtime_resume(&imx_data->ci_pdev->dev);
+
+	return IRQ_HANDLED;
+}
+
 static int ci_hdrc_imx_probe(struct platform_device *pdev)
 {
 	struct ci_hdrc_imx_data *data;
@@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
 		data->supports_runtime_pm = true;
 
+	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
+	if (data->wakeup_irq > 0) {
+		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
+						NULL, ci_wakeup_irq_handler,
+						IRQF_ONESHOT | IRQF_NO_AUTOEN,
+						pdata.name, data);
+		if (ret)
+			goto err_clk;
+	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
+		dev_warn(dev, "wakeup irq is missing\n");
+	}
+
 	ret = imx_usbmisc_init(data->usbmisc_data);
 	if (ret) {
 		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
@@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
 	}
 
 	imx_disable_unprepare_clks(dev);
+	enable_irq(data->wakeup_irq);
 	if (data->plat_data->flags & CI_HDRC_PMQOS)
 		cpu_latency_qos_remove_request(&data->pm_qos_req);
 
@@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
 	if (data->plat_data->flags & CI_HDRC_PMQOS)
 		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
 
+	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
+		disable_irq_nosync(data->wakeup_irq);
+
 	ret = imx_prepare_enable_clks(dev);
 	if (ret)
 		return ret;
@@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
 		return ret;
 
 	pinctrl_pm_select_sleep_state(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(data->wakeup_irq);
+
 	return ret;
 }
 
@@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
 	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
 	int ret;
 
+	if (device_may_wakeup(dev))
+		disable_irq_wake(data->wakeup_irq);
+
 	pinctrl_pm_select_default_state(dev);
 	ret = imx_controller_resume(dev, PMSG_RESUME);
 	if (!ret && data->supports_runtime_pm) {
-- 
2.34.1

[PATCH v4 4/6] usb: chipidea: imx: add HSIO Block Control wakeup setting

[Xu Yang]

On i.MX95 platform, USB wakeup setting is controlled by HSIO Block
Control:

HSIO Block Control Overview:
- The HSIO block control include configuration and status registers that
  provide miscellaneous top-level controls for clocking, beat limiter
  enables, wakeup signal enables and interrupt status for the PCIe and USB
  interfaces.

The wakeup function of HSIO blkctl is basically same as non-core, except
improvements about power lost cases. This will add the wakeup setting for
HSIO blkctl on i.MX95. It will firstly ioremap hsio blkctl memory, then do
wakeup setting as needs.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - add blkctl NULL checking
 - warning if no blkctl reg provided for imx95
Changes in v3:
 - remove usbmisc_imx95_init(), use usbmisc_imx7d_init()
Changes in v2:
 - add Rb tag
---
 drivers/usb/chipidea/usbmisc_imx.c | 77 ++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 1394881fde5f..3b35efd95cdb 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -139,6 +139,22 @@
 #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
 				 MX6_BM_ID_WAKEUP | MX6SX_BM_DPDM_WAKEUP_EN)
 
+/*
+ * HSIO Block Control Register
+ */
+
+#define BLKCTL_USB_WAKEUP_CTRL		0x0
+#define BLKCTL_OTG_WAKE_ENABLE		BIT(31)
+#define BLKCTL_OTG_VBUS_SESSVALID	BIT(4)
+#define BLKCTL_OTG_ID_WAKEUP_EN		BIT(2)
+#define BLKCTL_OTG_VBUS_WAKEUP_EN	BIT(1)
+#define BLKCTL_OTG_DPDM_WAKEUP_EN	BIT(0)
+
+#define BLKCTL_WAKEUP_SOURCE		(BLKCTL_OTG_WAKE_ENABLE	   | \
+					 BLKCTL_OTG_ID_WAKEUP_EN   | \
+					 BLKCTL_OTG_VBUS_WAKEUP_EN | \
+					 BLKCTL_OTG_DPDM_WAKEUP_EN)
+
 struct usbmisc_ops {
 	/* It's called once when probe a usb device */
 	int (*init)(struct imx_usbmisc_data *data);
@@ -159,6 +175,7 @@ struct usbmisc_ops {
 
 struct imx_usbmisc {
 	void __iomem *base;
+	void __iomem *blkctl;
 	spinlock_t lock;
 	const struct usbmisc_ops *ops;
 };
@@ -1016,6 +1033,44 @@ static int usbmisc_imx6sx_power_lost_check(struct imx_usbmisc_data *data)
 		return 0;
 }
 
+static u32 usbmisc_blkctl_wakeup_setting(struct imx_usbmisc_data *data)
+{
+	u32 wakeup_setting = BLKCTL_WAKEUP_SOURCE;
+
+	if (data->ext_id || data->available_role != USB_DR_MODE_OTG)
+		wakeup_setting &= ~BLKCTL_OTG_ID_WAKEUP_EN;
+
+	if (data->ext_vbus || data->available_role == USB_DR_MODE_HOST)
+		wakeup_setting &= ~BLKCTL_OTG_VBUS_WAKEUP_EN;
+
+	/* Select session valid as VBUS wakeup source */
+	wakeup_setting |= BLKCTL_OTG_VBUS_SESSVALID;
+
+	return wakeup_setting;
+}
+
+static int usbmisc_imx95_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
+{
+	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+	unsigned long flags;
+	u32 val;
+
+	if (!usbmisc->blkctl)
+		return 0;
+
+	spin_lock_irqsave(&usbmisc->lock, flags);
+	val = readl(usbmisc->blkctl + BLKCTL_USB_WAKEUP_CTRL);
+	val &= ~BLKCTL_WAKEUP_SOURCE;
+
+	if (enabled)
+		val |= usbmisc_blkctl_wakeup_setting(data);
+
+	writel(val, usbmisc->blkctl + BLKCTL_USB_WAKEUP_CTRL);
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+	return 0;
+}
+
 static const struct usbmisc_ops imx25_usbmisc_ops = {
 	.init = usbmisc_imx25_init,
 	.post = usbmisc_imx25_post,
@@ -1068,6 +1123,14 @@ static const struct usbmisc_ops imx7ulp_usbmisc_ops = {
 	.power_lost_check = usbmisc_imx7d_power_lost_check,
 };
 
+static const struct usbmisc_ops imx95_usbmisc_ops = {
+	.init = usbmisc_imx7d_init,
+	.set_wakeup = usbmisc_imx95_set_wakeup,
+	.charger_detection = imx7d_charger_detection,
+	.power_lost_check = usbmisc_imx7d_power_lost_check,
+	.vbus_comparator_on = usbmisc_imx7d_vbus_comparator_on,
+};
+
 static inline bool is_imx53_usbmisc(struct imx_usbmisc_data *data)
 {
 	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
@@ -1289,6 +1352,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
 		.compatible = "fsl,imx8ulp-usbmisc",
 		.data = &imx7ulp_usbmisc_ops,
 	},
+	{
+		.compatible = "fsl,imx95-usbmisc",
+		.data = &imx95_usbmisc_ops,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
@@ -1296,6 +1363,7 @@ MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
 static int usbmisc_imx_probe(struct platform_device *pdev)
 {
 	struct imx_usbmisc *data;
+	struct resource *res;
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -1307,6 +1375,15 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(data->base))
 		return PTR_ERR(data->base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (res) {
+		data->blkctl = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(data->blkctl))
+			return PTR_ERR(data->blkctl);
+	} else if (device_is_compatible(&pdev->dev, "fsl,imx95-usbmisc")) {
+		dev_warn(&pdev->dev, "wakeup setting is missing");
+	}
+
 	data->ops = of_device_get_match_data(&pdev->dev);
 	platform_set_drvdata(pdev, data);
 
-- 
2.34.1

[PATCH v4 5/6] arm64: dts: imx95: add USB2.0 nodes

[Xu Yang]

Add USB2.0 controller and phy nodes.

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa95xxSA
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - add Tb tag
Changes in v3:
 - no changes
Changes in v2:
 - add Rb tag
---
 arch/arm64/boot/dts/freescale/imx95.dtsi | 30 ++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 3af13173de4b..48c2033971bf 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -463,6 +463,13 @@ its: msi-controller@48040000 {
 		};
 	};
 
+	usbphynop: usbphynop {
+		compatible = "usb-nop-xceiv";
+		clocks = <&scmi_clk IMX95_CLK_HSIO>;
+		clock-names = "main_clk";
+		#phy-cells = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <2>;
@@ -1579,6 +1586,29 @@ usb3_phy: phy@4c1f0040 {
 			status = "disabled";
 		};
 
+		usb2: usb@4c200000 {
+			compatible = "fsl,imx95-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
+			reg = <0x0 0x4c200000 0x0 0x200>;
+			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&scmi_clk IMX95_CLK_HSIO>,
+				 <&scmi_clk IMX95_CLK_32K>;
+			clock-names = "usb_ctrl_root", "usb_wakeup";
+			iommus = <&smmu 0xf>;
+			phys = <&usbphynop>;
+			power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
+			fsl,usbmisc = <&usbmisc 0>;
+			status = "disabled";
+		};
+
+		usbmisc: usbmisc@4c200200 {
+			compatible = "fsl,imx95-usbmisc", "fsl,imx7d-usbmisc",
+				     "fsl,imx6q-usbmisc";
+			reg = <0x0 0x4c200200 0x0 0x200>,
+			      <0x0 0x4c010014 0x0 0x04>;
+			#index-cells = <1>;
+		};
+
 		pcie0: pcie@4c300000 {
 			compatible = "fsl,imx95-pcie";
 			reg = <0 0x4c300000 0 0x10000>,
-- 
2.34.1

[PATCH v4 6/6] arm64: dts: imx95-19x19-evk: enable USB2.0 node

[Xu Yang]

On this board, USB2.0 is a host-only port, add vbus regulator node
and enable USB2.0 node.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - no changes
Changes in v3:
 - no changes
Changes in v2:
 - reorder regulator node
---
 .../arm64/boot/dts/freescale/imx95-19x19-evk.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index 25ac331f0318..a41d542488ed 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -145,6 +145,15 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
 		off-on-delay-us = <12000>;
 	};
 
+	reg_usb_vbus: regulator-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "USB_VBUS";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&i2c7_pcal6524 3 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
 	sound-bt-sco {
 		compatible = "simple-audio-card";
 		simple-audio-card,name = "bt-sco-audio";
@@ -461,6 +470,13 @@ &sai3 {
 	status = "okay";
 };
 
+&usb2 {
+	dr_mode = "host";
+	disable-over-current;
+	vbus-supply = <&reg_usb_vbus>;
+	status = "okay";
+};
+
 &usb3 {
 	status = "okay";
 };
-- 
2.34.1

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Frank Li]

On Mon, Mar 03, 2025 at 11:33:41AM +0800, Xu Yang wrote:
> In previous imx platform, normal USB controller interrupt and wakeup
> interrupt are bound to one irq line. However, it changes on latest
> i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> This will add wakeup interrupt handling for i.MX95 to support various
> wakeup events.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
> Changes in v4:
>  - warning if no irq provided for imx95
> Changes in v3:
>  - include <linux/irq.h> to fix possible build issue
> Changes in v2:
>  - rename irq to wakeup_irq
>  - disable irq by default
>  - enable irq when suspend, disable irq when resume
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 1a7fc638213e..2baae9e6e673 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -6,6 +6,7 @@
>   */
>
>  #include <linux/module.h>
> +#include <linux/irq.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct clk *clk_wakeup;
>  	struct imx_usbmisc_data *usbmisc_data;
> +	int wakeup_irq;
>  	bool supports_runtime_pm;
>  	bool override_phy_control;
>  	bool in_lpm;
> @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
>  	return ret;
>  }
>
> +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> +{
> +	struct ci_hdrc_imx_data *imx_data = data;
> +
> +	disable_irq_nosync(irq);
> +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  {
>  	struct ci_hdrc_imx_data *data;
> @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
>
> +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> +	if (data->wakeup_irq > 0) {
> +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> +						NULL, ci_wakeup_irq_handler,
> +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> +						pdata.name, data);
> +		if (ret)
> +			goto err_clk;
> +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> +		dev_warn(dev, "wakeup irq is missing\n");
> +	}
> +

Suggest add imx95_usb_data, and new flags, like have_wakeup_irq.

	if (imx_platform_flag->have_wakeup_irq) {
		...
		ret = devm_request_threaded_irq();
		if (ret) {
			dev_warning(...);
			goto err_clk;
		}
	}

>  	ret = imx_usbmisc_init(data->usbmisc_data);
>  	if (ret) {
>  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
>  	}
>
>  	imx_disable_unprepare_clks(dev);
> +	enable_irq(data->wakeup_irq);

Suggest check data->wakeup_irq and other places.

	if (data->wakeup_irq > 0)
		enable_irq(data->wakeup_irq);

And you should check enable_irq()'s return value also.

Frank

>  	if (data->plat_data->flags & CI_HDRC_PMQOS)
>  		cpu_latency_qos_remove_request(&data->pm_qos_req);
>
> @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
>  	if (data->plat_data->flags & CI_HDRC_PMQOS)
>  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
>
> +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> +		disable_irq_nosync(data->wakeup_irq);
> +
>  	ret = imx_prepare_enable_clks(dev);
>  	if (ret)
>  		return ret;
> @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
>  		return ret;
>
>  	pinctrl_pm_select_sleep_state(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(data->wakeup_irq);
> +
>  	return ret;
>  }
>
> @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
>  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
>  	int ret;
>
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(data->wakeup_irq);
> +
>  	pinctrl_pm_select_default_state(dev);
>  	ret = imx_controller_resume(dev, PMSG_RESUME);
>  	if (!ret && data->supports_runtime_pm) {
> --
> 2.34.1
>

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Alexander Stein]

Am Montag, 3. März 2025, 04:33:41 CET schrieb Xu Yang:
> In previous imx platform, normal USB controller interrupt and wakeup
> interrupt are bound to one irq line. However, it changes on latest
> i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> This will add wakeup interrupt handling for i.MX95 to support various
> wakeup events.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
> Changes in v4:
>  - warning if no irq provided for imx95
> Changes in v3:
>  - include <linux/irq.h> to fix possible build issue
> Changes in v2:
>  - rename irq to wakeup_irq
>  - disable irq by default
>  - enable irq when suspend, disable irq when resume
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 1a7fc638213e..2baae9e6e673 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/module.h>
> +#include <linux/irq.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct clk *clk_wakeup;
>  	struct imx_usbmisc_data *usbmisc_data;
> +	int wakeup_irq;
>  	bool supports_runtime_pm;
>  	bool override_phy_control;
>  	bool in_lpm;
> @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
>  	return ret;
>  }
>  
> +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> +{
> +	struct ci_hdrc_imx_data *imx_data = data;
> +
> +	disable_irq_nosync(irq);
> +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  {
>  	struct ci_hdrc_imx_data *data;
> @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
>  
> +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> +	if (data->wakeup_irq > 0) {
> +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> +						NULL, ci_wakeup_irq_handler,
> +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> +						pdata.name, data);
> +		if (ret)
> +			goto err_clk;
> +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> +		dev_warn(dev, "wakeup irq is missing\n");
> +	}
> +
>  	ret = imx_usbmisc_init(data->usbmisc_data);
>  	if (ret) {
>  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
>  	}
>  
>  	imx_disable_unprepare_clks(dev);
> +	enable_irq(data->wakeup_irq);
>  	if (data->plat_data->flags & CI_HDRC_PMQOS)
>  		cpu_latency_qos_remove_request(&data->pm_qos_req);
>  
> @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
>  	if (data->plat_data->flags & CI_HDRC_PMQOS)
>  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
>  
> +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> +		disable_irq_nosync(data->wakeup_irq);
> +

if (data->wakeup_irq > 0) is necessary. I'll get a NULL-pointer
dereference on imx93.

Best regards
Alexander

>  	ret = imx_prepare_enable_clks(dev);
>  	if (ret)
>  		return ret;
> @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
>  		return ret;
>  
>  	pinctrl_pm_select_sleep_state(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(data->wakeup_irq);
> +
>  	return ret;
>  }
>  
> @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
>  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
>  	int ret;
>  
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(data->wakeup_irq);
> +
>  	pinctrl_pm_select_default_state(dev);
>  	ret = imx_controller_resume(dev, PMSG_RESUME);
>  	if (!ret && data->supports_runtime_pm) {
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Xu Yang]

On Mon, Mar 03, 2025 at 12:13:34PM -0500, Frank Li wrote:
> On Mon, Mar 03, 2025 at 11:33:41AM +0800, Xu Yang wrote:
> > In previous imx platform, normal USB controller interrupt and wakeup
> > interrupt are bound to one irq line. However, it changes on latest
> > i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> > This will add wakeup interrupt handling for i.MX95 to support various
> > wakeup events.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> > Changes in v4:
> >  - warning if no irq provided for imx95
> > Changes in v3:
> >  - include <linux/irq.h> to fix possible build issue
> > Changes in v2:
> >  - rename irq to wakeup_irq
> >  - disable irq by default
> >  - enable irq when suspend, disable irq when resume
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 1a7fc638213e..2baae9e6e673 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -6,6 +6,7 @@
> >   */
> >
> >  #include <linux/module.h>
> > +#include <linux/irq.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
> >  	struct clk *clk;
> >  	struct clk *clk_wakeup;
> >  	struct imx_usbmisc_data *usbmisc_data;
> > +	int wakeup_irq;
> >  	bool supports_runtime_pm;
> >  	bool override_phy_control;
> >  	bool in_lpm;
> > @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> >  	return ret;
> >  }
> >
> > +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> > +{
> > +	struct ci_hdrc_imx_data *imx_data = data;
> > +
> > +	disable_irq_nosync(irq);
> > +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  {
> >  	struct ci_hdrc_imx_data *data;
> > @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> >  		data->supports_runtime_pm = true;
> >
> > +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > +	if (data->wakeup_irq > 0) {
> > +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> > +						NULL, ci_wakeup_irq_handler,
> > +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > +						pdata.name, data);
> > +		if (ret)
> > +			goto err_clk;
> > +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> > +		dev_warn(dev, "wakeup irq is missing\n");
> > +	}
> > +
> 
> Suggest add imx95_usb_data, and new flags, like have_wakeup_irq.
> 
> 	if (imx_platform_flag->have_wakeup_irq) {
> 		...
> 		ret = devm_request_threaded_irq();
> 		if (ret) {
> 			dev_warning(...);
> 			goto err_clk;
> 		}
> 	}

Thanks for your suggestion. I might have different understanding about
it. I prefer to not add flag here. Given the following points:
(1) the wakeup irq is optional, the USB controller can still functional
well if properly set. (2) I may need add imx_usb_data for each new Soc,
however, it seems like unnecessary. 

> 
> >  	ret = imx_usbmisc_init(data->usbmisc_data);
> >  	if (ret) {
> >  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> > @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
> >  	}
> >
> >  	imx_disable_unprepare_clks(dev);
> > +	enable_irq(data->wakeup_irq);
> 
> Suggest check data->wakeup_irq and other places.
> 
> 	if (data->wakeup_irq > 0)
> 		enable_irq(data->wakeup_irq);
> 
> And you should check enable_irq()'s return value also.

I just find dedicated wakeirq mechanism (drivers/base/power/wakeirq.c)
basically fit my needs. I'm going to switch to use wakeirq. Then I'll
needn't to do such thing manually.

Thanks,
Xu Yang

> 
> Frank
> 
> >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> >  		cpu_latency_qos_remove_request(&data->pm_qos_req);
> >
> > @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
> >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> >  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
> >
> > +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> > +		disable_irq_nosync(data->wakeup_irq);
> > +
> >  	ret = imx_prepare_enable_clks(dev);
> >  	if (ret)
> >  		return ret;
> > @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
> >  		return ret;
> >
> >  	pinctrl_pm_select_sleep_state(dev);
> > +
> > +	if (device_may_wakeup(dev))
> > +		enable_irq_wake(data->wakeup_irq);
> > +
> >  	return ret;
> >  }
> >
> > @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
> >  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> >  	int ret;
> >
> > +	if (device_may_wakeup(dev))
> > +		disable_irq_wake(data->wakeup_irq);
> > +
> >  	pinctrl_pm_select_default_state(dev);
> >  	ret = imx_controller_resume(dev, PMSG_RESUME);
> >  	if (!ret && data->supports_runtime_pm) {
> > --
> > 2.34.1
> >

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Xu Yang]

On Tue, Mar 04, 2025 at 10:50:06AM +0100, Alexander Stein wrote:
> Am Montag, 3. März 2025, 04:33:41 CET schrieb Xu Yang:
> > In previous imx platform, normal USB controller interrupt and wakeup
> > interrupt are bound to one irq line. However, it changes on latest
> > i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> > This will add wakeup interrupt handling for i.MX95 to support various
> > wakeup events.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> > Changes in v4:
> >  - warning if no irq provided for imx95
> > Changes in v3:
> >  - include <linux/irq.h> to fix possible build issue
> > Changes in v2:
> >  - rename irq to wakeup_irq
> >  - disable irq by default
> >  - enable irq when suspend, disable irq when resume
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 1a7fc638213e..2baae9e6e673 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -6,6 +6,7 @@
> >   */
> >  
> >  #include <linux/module.h>
> > +#include <linux/irq.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
> >  	struct clk *clk;
> >  	struct clk *clk_wakeup;
> >  	struct imx_usbmisc_data *usbmisc_data;
> > +	int wakeup_irq;
> >  	bool supports_runtime_pm;
> >  	bool override_phy_control;
> >  	bool in_lpm;
> > @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> >  	return ret;
> >  }
> >  
> > +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> > +{
> > +	struct ci_hdrc_imx_data *imx_data = data;
> > +
> > +	disable_irq_nosync(irq);
> > +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  {
> >  	struct ci_hdrc_imx_data *data;
> > @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> >  		data->supports_runtime_pm = true;
> >  
> > +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > +	if (data->wakeup_irq > 0) {
> > +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> > +						NULL, ci_wakeup_irq_handler,
> > +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > +						pdata.name, data);
> > +		if (ret)
> > +			goto err_clk;
> > +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> > +		dev_warn(dev, "wakeup irq is missing\n");
> > +	}
> > +
> >  	ret = imx_usbmisc_init(data->usbmisc_data);
> >  	if (ret) {
> >  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> > @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
> >  	}
> >  
> >  	imx_disable_unprepare_clks(dev);
> > +	enable_irq(data->wakeup_irq);
> >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> >  		cpu_latency_qos_remove_request(&data->pm_qos_req);
> >  
> > @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
> >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> >  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
> >  
> > +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> > +		disable_irq_nosync(data->wakeup_irq);
> > +
> 
> if (data->wakeup_irq > 0) is necessary. I'll get a NULL-pointer
> dereference on imx93.

Okay. Thanks!

Best Regards,
Xu Yang

> 
> Best regards
> Alexander
> 
> >  	ret = imx_prepare_enable_clks(dev);
> >  	if (ret)
> >  		return ret;
> > @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
> >  		return ret;
> >  
> >  	pinctrl_pm_select_sleep_state(dev);
> > +
> > +	if (device_may_wakeup(dev))
> > +		enable_irq_wake(data->wakeup_irq);
> > +
> >  	return ret;
> >  }
> >  
> > @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
> >  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> >  	int ret;
> >  
> > +	if (device_may_wakeup(dev))
> > +		disable_irq_wake(data->wakeup_irq);
> > +
> >  	pinctrl_pm_select_default_state(dev);
> >  	ret = imx_controller_resume(dev, PMSG_RESUME);
> >  	if (!ret && data->supports_runtime_pm) {
> > 
> 
> 
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
> 
> 

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Frank Li]

On Tue, Mar 04, 2025 at 09:42:21PM +0800, Xu Yang wrote:
> On Mon, Mar 03, 2025 at 12:13:34PM -0500, Frank Li wrote:
> > On Mon, Mar 03, 2025 at 11:33:41AM +0800, Xu Yang wrote:
> > > In previous imx platform, normal USB controller interrupt and wakeup
> > > interrupt are bound to one irq line. However, it changes on latest
> > > i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> > > This will add wakeup interrupt handling for i.MX95 to support various
> > > wakeup events.
> > >
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > > Changes in v4:
> > >  - warning if no irq provided for imx95
> > > Changes in v3:
> > >  - include <linux/irq.h> to fix possible build issue
> > > Changes in v2:
> > >  - rename irq to wakeup_irq
> > >  - disable irq by default
> > >  - enable irq when suspend, disable irq when resume
> > > ---
> > >  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > index 1a7fc638213e..2baae9e6e673 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > @@ -6,6 +6,7 @@
> > >   */
> > >
> > >  #include <linux/module.h>
> > > +#include <linux/irq.h>
> > >  #include <linux/of.h>
> > >  #include <linux/of_platform.h>
> > >  #include <linux/platform_device.h>
> > > @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
> > >  	struct clk *clk;
> > >  	struct clk *clk_wakeup;
> > >  	struct imx_usbmisc_data *usbmisc_data;
> > > +	int wakeup_irq;
> > >  	bool supports_runtime_pm;
> > >  	bool override_phy_control;
> > >  	bool in_lpm;
> > > @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> > >  	return ret;
> > >  }
> > >
> > > +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> > > +{
> > > +	struct ci_hdrc_imx_data *imx_data = data;
> > > +
> > > +	disable_irq_nosync(irq);
> > > +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  {
> > >  	struct ci_hdrc_imx_data *data;
> > > @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> > >  		data->supports_runtime_pm = true;
> > >
> > > +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > > +	if (data->wakeup_irq > 0) {
> > > +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> > > +						NULL, ci_wakeup_irq_handler,
> > > +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > > +						pdata.name, data);
> > > +		if (ret)
> > > +			goto err_clk;
> > > +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> > > +		dev_warn(dev, "wakeup irq is missing\n");
> > > +	}
> > > +
> >
> > Suggest add imx95_usb_data, and new flags, like have_wakeup_irq.
> >
> > 	if (imx_platform_flag->have_wakeup_irq) {
> > 		...
> > 		ret = devm_request_threaded_irq();
> > 		if (ret) {
> > 			dev_warning(...);
> > 			goto err_clk;
> > 		}
> > 	}
>
> Thanks for your suggestion. I might have different understanding about
> it. I prefer to not add flag here. Given the following points:
> (1) the wakeup irq is optional, the USB controller can still functional
> well if properly set. (2) I may need add imx_usb_data for each new Soc,
> however, it seems like unnecessary.

You need add more code about device_is_compatible(dev, "fsl,imx95-usb")

device_is_compatible(dev, "fsl,imx95-usb") |
device_is_compatible(dev, "abc") |
...

the below code will be simple

	if (imx_platform_flag->have_wakeup_irq) {
		data->wakeup_irq = platform_get_irq_optional(pdev, 1);
		if (data->wakeup_irq > 0)
			...
		else
			dev_warn_once(dev, "wakeup irq is missing\n");

	}

Frank

>
> >
> > >  	ret = imx_usbmisc_init(data->usbmisc_data);
> > >  	if (ret) {
> > >  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> > > @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
> > >  	}
> > >
> > >  	imx_disable_unprepare_clks(dev);
> > > +	enable_irq(data->wakeup_irq);
> >
> > Suggest check data->wakeup_irq and other places.
> >
> > 	if (data->wakeup_irq > 0)
> > 		enable_irq(data->wakeup_irq);
> >
> > And you should check enable_irq()'s return value also.
>
> I just find dedicated wakeirq mechanism (drivers/base/power/wakeirq.c)
> basically fit my needs. I'm going to switch to use wakeirq. Then I'll
> needn't to do such thing manually.
>
> Thanks,
> Xu Yang
>
> >
> > Frank
> >
> > >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> > >  		cpu_latency_qos_remove_request(&data->pm_qos_req);
> > >
> > > @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
> > >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> > >  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
> > >
> > > +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> > > +		disable_irq_nosync(data->wakeup_irq);
> > > +
> > >  	ret = imx_prepare_enable_clks(dev);
> > >  	if (ret)
> > >  		return ret;
> > > @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
> > >  		return ret;
> > >
> > >  	pinctrl_pm_select_sleep_state(dev);
> > > +
> > > +	if (device_may_wakeup(dev))
> > > +		enable_irq_wake(data->wakeup_irq);
> > > +
> > >  	return ret;
> > >  }
> > >
> > > @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
> > >  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > >  	int ret;
> > >
> > > +	if (device_may_wakeup(dev))
> > > +		disable_irq_wake(data->wakeup_irq);
> > > +
> > >  	pinctrl_pm_select_default_state(dev);
> > >  	ret = imx_controller_resume(dev, PMSG_RESUME);
> > >  	if (!ret && data->supports_runtime_pm) {
> > > --
> > > 2.34.1
> > >

Re: [PATCH v4 4/6] usb: chipidea: imx: add HSIO Block Control wakeup setting

[Peter Chen]

On 25-03-03 11:33:42, Xu Yang wrote:
> On i.MX95 platform, USB wakeup setting is controlled by HSIO Block
> Control:
> 
> HSIO Block Control Overview:
> - The HSIO block control include configuration and status registers that
>   provide miscellaneous top-level controls for clocking, beat limiter
>   enables, wakeup signal enables and interrupt status for the PCIe and USB
>   interfaces.
> 
> The wakeup function of HSIO blkctl is basically same as non-core, except
> improvements about power lost cases. This will add the wakeup setting for
> HSIO blkctl on i.MX95. It will firstly ioremap hsio blkctl memory, then do
> wakeup setting as needs.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

> 
> ---
> Changes in v4:
>  - add blkctl NULL checking
>  - warning if no blkctl reg provided for imx95
> Changes in v3:
>  - remove usbmisc_imx95_init(), use usbmisc_imx7d_init()
> Changes in v2:
>  - add Rb tag
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 77 ++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 1394881fde5f..3b35efd95cdb 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -139,6 +139,22 @@
>  #define MX6_USB_OTG_WAKEUP_BITS (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP | \
>  				 MX6_BM_ID_WAKEUP | MX6SX_BM_DPDM_WAKEUP_EN)
>  
> +/*
> + * HSIO Block Control Register
> + */
> +
> +#define BLKCTL_USB_WAKEUP_CTRL		0x0
> +#define BLKCTL_OTG_WAKE_ENABLE		BIT(31)
> +#define BLKCTL_OTG_VBUS_SESSVALID	BIT(4)
> +#define BLKCTL_OTG_ID_WAKEUP_EN		BIT(2)
> +#define BLKCTL_OTG_VBUS_WAKEUP_EN	BIT(1)
> +#define BLKCTL_OTG_DPDM_WAKEUP_EN	BIT(0)
> +
> +#define BLKCTL_WAKEUP_SOURCE		(BLKCTL_OTG_WAKE_ENABLE	   | \
> +					 BLKCTL_OTG_ID_WAKEUP_EN   | \
> +					 BLKCTL_OTG_VBUS_WAKEUP_EN | \
> +					 BLKCTL_OTG_DPDM_WAKEUP_EN)
> +
>  struct usbmisc_ops {
>  	/* It's called once when probe a usb device */
>  	int (*init)(struct imx_usbmisc_data *data);
> @@ -159,6 +175,7 @@ struct usbmisc_ops {
>  
>  struct imx_usbmisc {
>  	void __iomem *base;
> +	void __iomem *blkctl;
>  	spinlock_t lock;
>  	const struct usbmisc_ops *ops;
>  };
> @@ -1016,6 +1033,44 @@ static int usbmisc_imx6sx_power_lost_check(struct imx_usbmisc_data *data)
>  		return 0;
>  }
>  
> +static u32 usbmisc_blkctl_wakeup_setting(struct imx_usbmisc_data *data)
> +{
> +	u32 wakeup_setting = BLKCTL_WAKEUP_SOURCE;
> +
> +	if (data->ext_id || data->available_role != USB_DR_MODE_OTG)
> +		wakeup_setting &= ~BLKCTL_OTG_ID_WAKEUP_EN;
> +
> +	if (data->ext_vbus || data->available_role == USB_DR_MODE_HOST)
> +		wakeup_setting &= ~BLKCTL_OTG_VBUS_WAKEUP_EN;
> +
> +	/* Select session valid as VBUS wakeup source */
> +	wakeup_setting |= BLKCTL_OTG_VBUS_SESSVALID;
> +
> +	return wakeup_setting;
> +}
> +
> +static int usbmisc_imx95_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
> +{
> +	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (!usbmisc->blkctl)
> +		return 0;
> +
> +	spin_lock_irqsave(&usbmisc->lock, flags);
> +	val = readl(usbmisc->blkctl + BLKCTL_USB_WAKEUP_CTRL);
> +	val &= ~BLKCTL_WAKEUP_SOURCE;
> +
> +	if (enabled)
> +		val |= usbmisc_blkctl_wakeup_setting(data);
> +
> +	writel(val, usbmisc->blkctl + BLKCTL_USB_WAKEUP_CTRL);
> +	spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> +	return 0;
> +}
> +
>  static const struct usbmisc_ops imx25_usbmisc_ops = {
>  	.init = usbmisc_imx25_init,
>  	.post = usbmisc_imx25_post,
> @@ -1068,6 +1123,14 @@ static const struct usbmisc_ops imx7ulp_usbmisc_ops = {
>  	.power_lost_check = usbmisc_imx7d_power_lost_check,
>  };
>  
> +static const struct usbmisc_ops imx95_usbmisc_ops = {
> +	.init = usbmisc_imx7d_init,
> +	.set_wakeup = usbmisc_imx95_set_wakeup,
> +	.charger_detection = imx7d_charger_detection,
> +	.power_lost_check = usbmisc_imx7d_power_lost_check,
> +	.vbus_comparator_on = usbmisc_imx7d_vbus_comparator_on,
> +};
> +
>  static inline bool is_imx53_usbmisc(struct imx_usbmisc_data *data)
>  {
>  	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> @@ -1289,6 +1352,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
>  		.compatible = "fsl,imx8ulp-usbmisc",
>  		.data = &imx7ulp_usbmisc_ops,
>  	},
> +	{
> +		.compatible = "fsl,imx95-usbmisc",
> +		.data = &imx95_usbmisc_ops,
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
> @@ -1296,6 +1363,7 @@ MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
>  static int usbmisc_imx_probe(struct platform_device *pdev)
>  {
>  	struct imx_usbmisc *data;
> +	struct resource *res;
>  
>  	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -1307,6 +1375,15 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>  	if (IS_ERR(data->base))
>  		return PTR_ERR(data->base);
>  
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (res) {
> +		data->blkctl = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(data->blkctl))
> +			return PTR_ERR(data->blkctl);
> +	} else if (device_is_compatible(&pdev->dev, "fsl,imx95-usbmisc")) {
> +		dev_warn(&pdev->dev, "wakeup setting is missing");
> +	}
> +
>  	data->ops = of_device_get_match_data(&pdev->dev);
>  	platform_set_drvdata(pdev, data);
>  
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Xu Yang]

On Tue, Mar 04, 2025 at 10:20:48AM -0500, Frank Li wrote:
> On Tue, Mar 04, 2025 at 09:42:21PM +0800, Xu Yang wrote:
> > On Mon, Mar 03, 2025 at 12:13:34PM -0500, Frank Li wrote:
> > > On Mon, Mar 03, 2025 at 11:33:41AM +0800, Xu Yang wrote:
> > > > In previous imx platform, normal USB controller interrupt and wakeup
> > > > interrupt are bound to one irq line. However, it changes on latest
> > > > i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> > > > This will add wakeup interrupt handling for i.MX95 to support various
> > > > wakeup events.
> > > >
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > ---
> > > > Changes in v4:
> > > >  - warning if no irq provided for imx95
> > > > Changes in v3:
> > > >  - include <linux/irq.h> to fix possible build issue
> > > > Changes in v2:
> > > >  - rename irq to wakeup_irq
> > > >  - disable irq by default
> > > >  - enable irq when suspend, disable irq when resume
> > > > ---
> > > >  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
> > > >  1 file changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > index 1a7fc638213e..2baae9e6e673 100644
> > > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > @@ -6,6 +6,7 @@
> > > >   */
> > > >
> > > >  #include <linux/module.h>
> > > > +#include <linux/irq.h>
> > > >  #include <linux/of.h>
> > > >  #include <linux/of_platform.h>
> > > >  #include <linux/platform_device.h>
> > > > @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
> > > >  	struct clk *clk;
> > > >  	struct clk *clk_wakeup;
> > > >  	struct imx_usbmisc_data *usbmisc_data;
> > > > +	int wakeup_irq;
> > > >  	bool supports_runtime_pm;
> > > >  	bool override_phy_control;
> > > >  	bool in_lpm;
> > > > @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> > > >  	return ret;
> > > >  }
> > > >
> > > > +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> > > > +{
> > > > +	struct ci_hdrc_imx_data *imx_data = data;
> > > > +
> > > > +	disable_irq_nosync(irq);
> > > > +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> > > > +
> > > > +	return IRQ_HANDLED;
> > > > +}
> > > > +
> > > >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > >  {
> > > >  	struct ci_hdrc_imx_data *data;
> > > > @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> > > >  		data->supports_runtime_pm = true;
> > > >
> > > > +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > > > +	if (data->wakeup_irq > 0) {
> > > > +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> > > > +						NULL, ci_wakeup_irq_handler,
> > > > +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > > > +						pdata.name, data);
> > > > +		if (ret)
> > > > +			goto err_clk;
> > > > +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> > > > +		dev_warn(dev, "wakeup irq is missing\n");
> > > > +	}
> > > > +
> > >
> > > Suggest add imx95_usb_data, and new flags, like have_wakeup_irq.
> > >
> > > 	if (imx_platform_flag->have_wakeup_irq) {
> > > 		...
> > > 		ret = devm_request_threaded_irq();
> > > 		if (ret) {
> > > 			dev_warning(...);
> > > 			goto err_clk;
> > > 		}
> > > 	}
> >
> > Thanks for your suggestion. I might have different understanding about
> > it. I prefer to not add flag here. Given the following points:
> > (1) the wakeup irq is optional, the USB controller can still functional
> > well if properly set. (2) I may need add imx_usb_data for each new Soc,
> > however, it seems like unnecessary.
> 
> You need add more code about device_is_compatible(dev, "fsl,imx95-usb")
> 
> device_is_compatible(dev, "fsl,imx95-usb") |
> device_is_compatible(dev, "abc") |
> ...
> 
> the below code will be simple
> 
> 	if (imx_platform_flag->have_wakeup_irq) {
> 		data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> 		if (data->wakeup_irq > 0)
> 			...
> 		else
> 			dev_warn_once(dev, "wakeup irq is missing\n");
> 
> 	}

Sure, thanks for your comments.

Given that this irq is optional, it appears reasonable not to print a
warning, especially since the dt-binding check will provide warnings.
Then, I'll need not to add device_is_compatible() or have_wakeup_irq
flag.

Thanks,
Xu Yang

> 
> Frank
> 
> >
> > >
> > > >  	ret = imx_usbmisc_init(data->usbmisc_data);
> > > >  	if (ret) {
> > > >  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> > > > @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
> > > >  	}
> > > >
> > > >  	imx_disable_unprepare_clks(dev);
> > > > +	enable_irq(data->wakeup_irq);
> > >
> > > Suggest check data->wakeup_irq and other places.
> > >
> > > 	if (data->wakeup_irq > 0)
> > > 		enable_irq(data->wakeup_irq);
> > >
> > > And you should check enable_irq()'s return value also.
> >
> > I just find dedicated wakeirq mechanism (drivers/base/power/wakeirq.c)
> > basically fit my needs. I'm going to switch to use wakeirq. Then I'll
> > needn't to do such thing manually.
> >
> > Thanks,
> > Xu Yang
> >
> > >
> > > Frank
> > >
> > > >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> > > >  		cpu_latency_qos_remove_request(&data->pm_qos_req);
> > > >
> > > > @@ -608,6 +633,9 @@ static int imx_controller_resume(struct device *dev,
> > > >  	if (data->plat_data->flags & CI_HDRC_PMQOS)
> > > >  		cpu_latency_qos_add_request(&data->pm_qos_req, 0);
> > > >
> > > > +	if (!irqd_irq_disabled(irq_get_irq_data(data->wakeup_irq)))
> > > > +		disable_irq_nosync(data->wakeup_irq);
> > > > +
> > > >  	ret = imx_prepare_enable_clks(dev);
> > > >  	if (ret)
> > > >  		return ret;
> > > > @@ -643,6 +671,10 @@ static int ci_hdrc_imx_suspend(struct device *dev)
> > > >  		return ret;
> > > >
> > > >  	pinctrl_pm_select_sleep_state(dev);
> > > > +
> > > > +	if (device_may_wakeup(dev))
> > > > +		enable_irq_wake(data->wakeup_irq);
> > > > +
> > > >  	return ret;
> > > >  }
> > > >
> > > > @@ -651,6 +683,9 @@ static int ci_hdrc_imx_resume(struct device *dev)
> > > >  	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > > >  	int ret;
> > > >
> > > > +	if (device_may_wakeup(dev))
> > > > +		disable_irq_wake(data->wakeup_irq);
> > > > +
> > > >  	pinctrl_pm_select_default_state(dev);
> > > >  	ret = imx_controller_resume(dev, PMSG_RESUME);
> > > >  	if (!ret && data->supports_runtime_pm) {
> > > > --
> > > > 2.34.1
> > > >

Re: [PATCH v4 3/6] usb: chipidea: imx: add wakeup interrupt handling

[Xu Yang]

On Tue, Mar 04, 2025 at 09:42:21PM +0800, Xu Yang wrote:
> On Mon, Mar 03, 2025 at 12:13:34PM -0500, Frank Li wrote:
> > On Mon, Mar 03, 2025 at 11:33:41AM +0800, Xu Yang wrote:
> > > In previous imx platform, normal USB controller interrupt and wakeup
> > > interrupt are bound to one irq line. However, it changes on latest
> > > i.MX95 platform since it has a dedicated irq line for wakeup interrupt.
> > > This will add wakeup interrupt handling for i.MX95 to support various
> > > wakeup events.
> > >
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > > Changes in v4:
> > >  - warning if no irq provided for imx95
> > > Changes in v3:
> > >  - include <linux/irq.h> to fix possible build issue
> > > Changes in v2:
> > >  - rename irq to wakeup_irq
> > >  - disable irq by default
> > >  - enable irq when suspend, disable irq when resume
> > > ---
> > >  drivers/usb/chipidea/ci_hdrc_imx.c | 35 ++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > index 1a7fc638213e..2baae9e6e673 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > @@ -6,6 +6,7 @@
> > >   */
> > >
> > >  #include <linux/module.h>
> > > +#include <linux/irq.h>
> > >  #include <linux/of.h>
> > >  #include <linux/of_platform.h>
> > >  #include <linux/platform_device.h>
> > > @@ -98,6 +99,7 @@ struct ci_hdrc_imx_data {
> > >  	struct clk *clk;
> > >  	struct clk *clk_wakeup;
> > >  	struct imx_usbmisc_data *usbmisc_data;
> > > +	int wakeup_irq;
> > >  	bool supports_runtime_pm;
> > >  	bool override_phy_control;
> > >  	bool in_lpm;
> > > @@ -336,6 +338,16 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> > >  	return ret;
> > >  }
> > >
> > > +static irqreturn_t ci_wakeup_irq_handler(int irq, void *data)
> > > +{
> > > +	struct ci_hdrc_imx_data *imx_data = data;
> > > +
> > > +	disable_irq_nosync(irq);
> > > +	pm_runtime_resume(&imx_data->ci_pdev->dev);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  {
> > >  	struct ci_hdrc_imx_data *data;
> > > @@ -476,6 +488,18 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> > >  		data->supports_runtime_pm = true;
> > >
> > > +	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> > > +	if (data->wakeup_irq > 0) {
> > > +		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
> > > +						NULL, ci_wakeup_irq_handler,
> > > +						IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > > +						pdata.name, data);
> > > +		if (ret)
> > > +			goto err_clk;
> > > +	} else if (device_is_compatible(dev, "fsl,imx95-usb")) {
> > > +		dev_warn(dev, "wakeup irq is missing\n");
> > > +	}
> > > +
> > 
> > Suggest add imx95_usb_data, and new flags, like have_wakeup_irq.
> > 
> > 	if (imx_platform_flag->have_wakeup_irq) {
> > 		...
> > 		ret = devm_request_threaded_irq();
> > 		if (ret) {
> > 			dev_warning(...);
> > 			goto err_clk;
> > 		}
> > 	}
> 
> Thanks for your suggestion. I might have different understanding about
> it. I prefer to not add flag here. Given the following points:
> (1) the wakeup irq is optional, the USB controller can still functional
> well if properly set. (2) I may need add imx_usb_data for each new Soc,
> however, it seems like unnecessary. 
> 
> > 
> > >  	ret = imx_usbmisc_init(data->usbmisc_data);
> > >  	if (ret) {
> > >  		dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> > > @@ -584,6 +608,7 @@ static int imx_controller_suspend(struct device *dev,
> > >  	}
> > >
> > >  	imx_disable_unprepare_clks(dev);
> > > +	enable_irq(data->wakeup_irq);
> > 
> > Suggest check data->wakeup_irq and other places.
> > 
> > 	if (data->wakeup_irq > 0)
> > 		enable_irq(data->wakeup_irq);
> > 
> > And you should check enable_irq()'s return value also.
> 
> I just find dedicated wakeirq mechanism (drivers/base/power/wakeirq.c)
> basically fit my needs. I'm going to switch to use wakeirq. Then I'll
> needn't to do such thing manually.

The dedicated wakeirq mechanism doesn't suit my needs well. It can't
pm_runtime_resume another device, and /proc/interrupts show the wakeup irq
triggered many times after system resume, because it's a level type and
the irq doesn't get disabled at the first time. So I will keep current
style.

Thanks,
Xu Yang