Hi,
Peter Chen <peter.chen@xxxxxxx> writes:
> Introduce runtime PM and wakeup interrupt handler for cdns3,
> the runtime PM is default off since other cdns3 may not
> implement glue layer support for runtime PM.
>
> One typical wakeup event use case is xHCI runtime suspend will clear
> USBCMD.RS bit, after that the xHCI will not trigger any interrupts,
> so its parent (cdns core device) needs to resume xHCI device when
> any (wakeup) events occurs at host port.
>
> When the controller is in low power mode, the lpm flag will be set.
> The interrupt triggered later than lpm flag is set considers as
> wakeup interrupt and handled at cdns_wakeup_irq. Once the wakeup
> occurs, it first disables interrupt to avoid later interrupt
> occurrence since the controller is in low power mode at that
> time, and access registers may be invalid at that time. At wakeup
> handler, it will call pm_request_resume to wakeup xHCI device, and
> at runtime resume handler, it will enable interrupt again.
>
> The API platform_suspend is introduced for glue layer to implement
> platform specific PM sequence.
can't you rely on parent->child relationship here?
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index e56dbb6a898c..faee5ec5fc20 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -392,6 +392,29 @@ static void set_phy_power_off(struct cdns3 *cdns)
> phy_power_off(cdns->usb2_phy);
> }
>
> +/**
> + * cdns3_wakeup_irq - interrupt handler for wakeup events
> + * @irq: irq number for cdns3 core device
> + * @data: structure of cdns3
> + *
> + * Returns IRQ_HANDLED or IRQ_NONE
> + */
> +static irqreturn_t cdns3_wakeup_irq(int irq, void *data)
> +{
> + struct cdns3 *cdns = data;
> +
> + if (cdns->in_lpm) {
> + disable_irq_nosync(irq);
why do you need to call disable_irq_nosync()? interrupts are already
disabled.
> + cdns->wakeup_pending = true;
> + if ((cdns->role == USB_ROLE_HOST) && cdns->host_dev)
> + pm_request_resume(&cdns->host_dev->dev);
nothing for peripheral mode?
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> /**
> * cdns3_probe - probe for cdns3 core device
> * @pdev: Pointer to cdns3 core platform device
> @@ -418,6 +441,7 @@ static int cdns3_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> cdns->dev = dev;
> + cdns->pdata = dev_get_platdata(dev);
>
> platform_set_drvdata(pdev, cdns);
>
> @@ -466,6 +490,15 @@ static int cdns3_probe(struct platform_device *pdev)
>
> cdns->otg_res = *res;
>
> + cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
> + if (cdns->wakeup_irq == -EPROBE_DEFER)
> + return cdns->wakeup_irq;
> +
> + if (cdns->wakeup_irq < 0) {
should be <= 0, no?
> @@ -502,6 +535,19 @@ static int cdns3_probe(struct platform_device *pdev)
> goto err3;
> }
>
> + if (cdns->wakeup_irq) {
> + ret = devm_request_threaded_irq(cdns->dev, cdns->wakeup_irq,
> + cdns3_wakeup_irq,
> + NULL,
if the thread handler is NULL, why don't you use devm_request_irq()?
--
balbi
Attachment:
signature.asc
Description: PGP signature
