On Wed, Feb 19, 2025 at 05:31:01PM +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 wakup interrupt handling for i.MX95 to support various
> wakeup events.
>
> Reviewed-by: Jun Li <jun.li@xxxxxxx>
> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 42 ++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 1a7fc638213e..5779568ebcf6 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -98,9 +98,12 @@ struct ci_hdrc_imx_data {
> struct clk *clk;
> struct clk *clk_wakeup;
> struct imx_usbmisc_data *usbmisc_data;
> + /* wakeup interrupt*/
> + int irq;
use "wakeup_irq" to avoid confuse with normal controller irq?
> bool supports_runtime_pm;
> bool override_phy_control;
> bool in_lpm;
> + bool wakeup_pending;
> struct pinctrl *pinctrl;
> struct pinctrl_state *pinctrl_hsic_active;
> struct regulator *hsic_pad_regulator;
> @@ -336,6 +339,24 @@ 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;
> +
> + if (imx_data->in_lpm) {
> + if (imx_data->wakeup_pending)
> + return IRQ_HANDLED;
> +
> + disable_irq_nosync(irq);
> + imx_data->wakeup_pending = true;
> + pm_runtime_resume(&imx_data->ci_pdev->dev);
Not sure why need pm_runtime_resume here? There are not access register
here.
> +
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> static int ci_hdrc_imx_probe(struct platform_device *pdev)
> {
> struct ci_hdrc_imx_data *data;
> @@ -476,6 +497,15 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> data->supports_runtime_pm = true;
>
> + data->irq = platform_get_irq_optional(pdev, 1);
> + if (data->irq > 0) {
> + ret = devm_request_threaded_irq(dev, data->irq,
> + NULL, ci_wakeup_irq_handler,
> + IRQF_ONESHOT, pdata.name, data);
> + if (ret)
> + goto err_clk;
> + }
> +
> ret = imx_usbmisc_init(data->usbmisc_data);
> if (ret) {
> dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> @@ -621,6 +651,11 @@ static int imx_controller_resume(struct device *dev,
> goto clk_disable;
> }
>
> + if (data->wakeup_pending) {
> + data->wakeup_pending = false;
> + enable_irq(data->irq);
> + }
> +
> return 0;
>
> clk_disable:
> @@ -643,6 +678,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->irq);
> +
> return ret;
> }
>
> @@ -651,6 +690,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->irq);
> +
Look like only want enable wakeup irq between suspend and resume. There are
some questions to understand hehavor.
1 driver probe --> 2. hdrc suspend -->3 hdrc resume --> 4 controller resume
1--2, look like wakeup irq is enabled. maybe controller have some bit to
disable issue wakeup irq, otherwise flood irq will happen because you check
lpm in irq handler.
after 2. wakeup irq enable, if wakeup irq happen, system resume.
ci_hdrc_imx_resume() only clear a flags, not any sync problem.
But sequence imx_controller_resume() and ci_wakeup_irq_handler() may not
guaranteed.
If ci_wakeup_irq_handler() call first, ci_wakeup_irq_handler() disable
itself. imx_controller_resume() will enable wakeup irq, which will be same
state 1--2. but if imx_controller_resume() run firstly,
ci_wakeup_irq_handler() will disable wakeup irq, which difference state
with 1--2.
If there are a bit(WAKEUP_EN) in controller can control wakeup irq
enable/disable,
will below logic be simple?
ci_hdrc_imx_suspend()
{
writel(WAKEUP_EN, ...);
....
}
ci_wakeup_irq_handler()
{
...
pm_runtime_resume(&imx_data->ci_pdev->dev);
writel(~WAKEUP_EN, ...);
...
}
Frank
> pinctrl_pm_select_default_state(dev);
> ret = imx_controller_resume(dev, PMSG_RESUME);
> if (!ret && data->supports_runtime_pm) {
> --
> 2.34.1
>
