Re: [PATCH v8 07/16] usb: core: Move struct usb_port and related APIs to port.h
Greg Kroah-Hartman <[email protected]>
| Newsgroups | org.kernel.vger.linux-acpi,dev.linux.lists.driver-core,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mediatek,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm,org.kernel.vger.linux-usb |
|---|---|
| Message-ID | <2026090430-botanist-ungodly-0299@gregkh> |
On Fri, Sep 04, 2026 at 04:41:46PM +0800, Chen-Yu Tsai wrote: > |struct usb_port| and its matching container_of() macro should live in > its own port.h, matching the split we have for .c files. Why? > Move them as described. Also move usb_port_is_power_on(), since in the > next change its |struct usb_hub *| parameter will be changed to > |struct usb_port *|, and becomes a non-static function that only > references |struct usb_port|. No, please don't do that unless you have a really good reason. > > port.h is only included from hub.h, as a subsequent patch will directly > use fields from |struct usb_port| in a static inline helper in hub.h. > The USB internal headers don't have header guards to help with this. > > Also drop 'extern' from the header declaration of usb_port_is_power_on(). > This is not needed in modern C. > > Suggested-by: Andy Shevchenko <[email protected]> > Reviewed-by: Bartosz Golaszewski <[email protected]> > Reviewed-by: Andy Shevchenko <[email protected]> > Signed-off-by: Chen-Yu Tsai <[email protected]> > --- > Changes since v4: > - Dropped 'extern' (Andy) > > Changes since v3: > - New patch (Andy) > --- > drivers/usb/core/hub.c | 16 ---------- > drivers/usb/core/hub.h | 48 ++--------------------------- > drivers/usb/core/port.c | 16 ++++++++++ > drivers/usb/core/port.h | 68 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 86 insertions(+), 62 deletions(-) > create mode 100644 drivers/usb/core/port.h hub.h is small enough, making a single .h file that is 68 lines long for no real good reason isn't a good idea. Odds are this will make the build take longer as you now need to open/read/close another file. And if we really want to do this, it shouldn't be burried in the middle of a long "enable this platform" series where people will miss it. Thanks to Alan's review for making me notice it. thanks, greg k-h