On Wed, Aug 13, 2025 at 10:51 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, Aug 01, 2025 at 03:39:32AM +0000, Guan-Yu Lin wrote: > > +config USB_XHCI_SIDEBAND_SUSPEND > > Again, why is a new config option needed here? Why can't we just use > the existing one? Why would you ever want one and not the other? > USB_XHCI_SIDEBAND focuses on the normal use case where the system is active, while USB_XHCI_SIDEBAND_SUSPEND enables the sideband during system sleep (Suspend-to-RAM). The design allows the user to determine the degree of support for the sideband in the system. We could add the "select" keyword to automatically enable USB_XHCI_SIDEBAND once USB_XHCI_SIDEBAND_SUSPEND is enabled. > > > +EXPORT_SYMBOL_GPL(xhci_sideband_check); > > +#endif /* IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND) */ > > #ifdef in .c files is generally not a good idea, is it really needed > here? Maybe it is, it's hard to unwind... > > thanks, > > greg k-h Given that CONFIG_USB_XHCI_SIDEBAND_SUSPEND depends on CONFIG_USB_XHCI_SIDEBAND and adds only a single function, I think it is preferable to place the new code in the same file. This approach prevents unnecessary code fragmentation and improves maintainability by keeping related functions together. Regards, Guan-Yu
