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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.