| Message ID | 20241023034457.13241-1-huanglei814@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] usb: core: adds support for PM control of specific USB dev skip suspend. | expand |
On 23/10/2024 05:44, huanglei814 wrote: > From: huanglei <huanglei@kylinos.cn> > > All USB devices are brought into suspend power state after system suspend. > It is desirable for some specific manufacturers buses to keep their devices > in normal state even after system suspend. > > Signed-off-by: huanglei <huanglei@kylinos.cn> > --- > v3->v4: > - Cancel SUSPENDED state judgment when enter suspend，because udev will not enter. > - Change "usb-skip-suspend" to "usb-never-suspend" Nothing improved in respect of bindings. Still NAK. Best regards, Krzysztof
On Wed, Oct 23, 2024 at 11:44:57AM +0800, huanglei814 wrote: > From: huanglei <huanglei@kylinos.cn> > > All USB devices are brought into suspend power state after system suspend. > It is desirable for some specific manufacturers buses to keep their devices > in normal state even after system suspend. Here you should explain _why_ this is desirable. People will want to know, so that they can judge whether this is a good thing to do. > Signed-off-by: huanglei <huanglei@kylinos.cn> > --- > v3->v4: > - Cancel SUSPENDED state judgment when enter suspend，because udev will not enter. > - Change "usb-skip-suspend" to "usb-never-suspend" > - Change dev_err to dev_info. > - Remove Kconfig option, it's redundant indeed. > - Update commit message style. > v2->v3: > - Rebase and update commit message. > v1->v2: > - Change to bool type for skip_suspend. > - Kconfig remove "default n", 'n' is the default. > --- > drivers/usb/core/driver.c | 8 ++++++++ > drivers/usb/host/xhci-plat.c | 5 +++++ > include/linux/usb.h | 7 +++++++ > 3 files changed, 20 insertions(+) > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index 0c3f12daac79..93137c7c34df 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -1583,6 +1583,11 @@ int usb_suspend(struct device *dev, pm_message_t msg) > struct usb_device *udev = to_usb_device(dev); > int r; > > + if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) { > + dev_info(dev, "abort suspend\n"); > + return 0; Do you really need this message at all? Maybe it should be dev_dbg, because it's useful only to developers, not to normal users. Also, you're not really aborting the suspend; you're skipping it. > + } > + > unbind_no_pm_drivers_interfaces(udev); > > /* From now on we are sure all drivers support suspend/resume > @@ -1619,6 +1624,9 @@ int usb_resume(struct device *dev, pm_message_t msg) > struct usb_device *udev = to_usb_device(dev); > int status; > > + if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME)) > + return 0; > + > /* For all calls, take the device back to full power and > * tell the PM core in case it was autosuspended previously. > * Unbind the interfaces that will need rebinding later, > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index ecaa75718e59..35062aa19a32 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -265,6 +265,11 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s > if (device_property_read_bool(tmpdev, "xhci-skip-phy-init-quirk")) > xhci->quirks |= XHCI_SKIP_PHY_INIT; > > + if (device_property_read_bool(tmpdev, "usb-never-suspend")) { Hmmm. "usb-never-suspend" implies that the bus won't even be put into runtime suspend, but that's not what you mean. What you really mean is more like "stay-powered-during-system-suspend", but that name is a bit long. Maybe you can think of something better. Also, this sounds more like the manufacturer's policy decision rather than a hardware limitation. Is that correct? If it is, I doubt that the setting really belongs in DT. > + hcd_to_bus(hcd)->skip_suspend = true; > + hcd_to_bus(xhci->shared_hcd)->skip_suspend = true; > + } > + > device_property_read_u32(tmpdev, "imod-interval-ns", > &xhci->imod_interval); > } > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 672d8fc2abdb..c854d1f622ec 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -487,6 +487,13 @@ struct usb_bus { > struct mon_bus *mon_bus; /* non-null when associated */ > int monitored; /* non-zero when monitored */ > #endif > + > + bool skip_suspend; /* All USB devices are brought into suspend > + * power state after system suspend. It is > + * desirable for some specific manufacturers > + * buses to keep their devices in normal > + * state even after system suspend. > + */ This is not a good comment. It doesn't say what the skip_suspend flag means. You don't need to explain the reason for the flag here; just explain what it does. Alan Stern > }; > > struct usb_dev_state; > -- > 2.17.1 > >
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 0c3f12daac79..93137c7c34df 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1583,6 +1583,11 @@ int usb_suspend(struct device *dev, pm_message_t msg) struct usb_device *udev = to_usb_device(dev); int r; + if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) { + dev_info(dev, "abort suspend\n"); + return 0; + } + unbind_no_pm_drivers_interfaces(udev); /* From now on we are sure all drivers support suspend/resume @@ -1619,6 +1624,9 @@ int usb_resume(struct device *dev, pm_message_t msg) struct usb_device *udev = to_usb_device(dev); int status; + if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME)) + return 0; + /* For all calls, take the device back to full power and * tell the PM core in case it was autosuspended previously. * Unbind the interfaces that will need rebinding later, diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index ecaa75718e59..35062aa19a32 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -265,6 +265,11 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s if (device_property_read_bool(tmpdev, "xhci-skip-phy-init-quirk")) xhci->quirks |= XHCI_SKIP_PHY_INIT; + if (device_property_read_bool(tmpdev, "usb-never-suspend")) { + hcd_to_bus(hcd)->skip_suspend = true; + hcd_to_bus(xhci->shared_hcd)->skip_suspend = true; + } + device_property_read_u32(tmpdev, "imod-interval-ns", &xhci->imod_interval); } diff --git a/include/linux/usb.h b/include/linux/usb.h index 672d8fc2abdb..c854d1f622ec 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -487,6 +487,13 @@ struct usb_bus { struct mon_bus *mon_bus; /* non-null when associated */ int monitored; /* non-zero when monitored */ #endif + + bool skip_suspend; /* All USB devices are brought into suspend + * power state after system suspend. It is + * desirable for some specific manufacturers + * buses to keep their devices in normal + * state even after system suspend. + */ }; struct usb_dev_state;