Re: [PATCH v8 07/16] usb: core: Move struct usb_port and related APIs to port.h

[Chen-Yu Tsai]

On Sat, Sep 5, 2026 at 12:47 AM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> 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?

This was requested by Andy. And to me it sort of made sense.

> > 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.

Which part? Move the code?

> >
> > 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 <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Signed-off-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
> > ---
> > 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.

OK. Andy, can you accept not doing the move?

> 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.

Well the code is also touched in this series, so it is related. Without
the change, the code movement makes even less sense, as mentioned in the
commit message.

As for missing the patch, I can change the series name to something
like "usb: Hook up pwrseq for USB power control on M.2 E-key slots".
The series spans multiple subsystems and even the driver core, and the
goal is to enable M.2 E-key slots, so hopefully that ticks all the boxes.
Except which platform is affected that is.


ChenYu




> Thanks to Alan's review for making me notice it.
>
> thanks,
>
> greg k-h
>