| Message ID | 20250303033344.1251076-1-xu.yang_2@nxp.com |
|---|---|
| Headers | show |
| Series | add USB2.0 support for i.MX95-19x19 EVK board | expand |
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) { >
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 > >
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/ > >
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 > > >
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 > > > >
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(-)