Hi,

Sandeep Maheswaram [off-list ref] writes:

quoted

This means that in order for glue_suspend() to run, dwc3 has to suspend
first and xhci has to suspend before dwc3.

For example, in the suspend call above, qcom (the glue) is directly
accessing dwc3 core data, which is incorrect. It looks like we want to
know if the PHY is not powered off and if it isn't, then we want to
change the power domain ACTIVE_WAKEUP flag. Now, phy_power_off is false
whenever any of xHCI's children enable USB wakeup.

It seems like we need to way to generically propagate that knowledge up
the parent tree. I.e., a parent needs to know if its child is wakeup
capable, then dwc3 could, in its suspend routine:

static int dwc3_suspend(struct device *dev)
{
	/* ... */

	if (device_children_wakeup_capable(dev))
         	device_enable_wakeup(dev);

	/* ... */
}

Can we use like  this device_may_wakeup(&dwc->xhci->dev) to check if
children is wakeup capable like below ?

that really doesn't sound like a good idea, IMHO. We're still passing
through layers of abstraction without anyone's knowledge :-)

It looks to me like we're missing some infrastructure in the wakeup code
so parents can make decisions based on the state of their children.

quoted

and similarly for qcom glue:

static int dwc3_qcom_suspend(struct device *dev)
{
	/* ... */


	if (device_children_wakeup_capable(dev)) {
         	device_enable_wakeup(dev);
		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
         }

	/* ... */
}

It also seems plausible that this could be done at driver core and
completely hidden away from drivers.

And in qcom glue like this

static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
{

/* ... */

    struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);

you see, here there's an assumption that the platform data is still
valid and not some bogus dangling pointer. There's also an assumption
that the type is struct dwc3 (which is unlikely to change, but still).

-- 
balbi