On Fri, Jul 3, 2026 at 9:33 PM Andy Shevchenko
[off-list ref] wrote:

On Fri, Jul 03, 2026 at 09:17:16PM +0800, Chen-Yu Tsai wrote:

quoted

On Fri, Jul 3, 2026 at 9:11 PM Andy Shevchenko
[off-list ref] wrote:

quoted

On Fri, Jul 03, 2026 at 07:03:07PM +0800, Chen-Yu Tsai wrote:

quoted

usb_port_is_power_on() currently takes |struct usb_hub*|, but only needs
it to tell if the hub/port is SuperSpeed or not.

In a subsequent change, usb_port_is_power_on() needs access to a pwrseq
state tracking field in |struct usb_port|. Either structure can be used
to identify whether a port/hub is SuperSpeed or not, as the field in
|struct usb_port| is inherited from the hub:

    port->is_superspeed = hub_is_superspeed(hub)

Replace usb_port_is_power_on()'s |struct usb_hub*| parameter with
|struct usb_port*| so a subsequent change can use it.

At a brief look this will be the only function that takes usb_port
instead of usb_hub in the entire hub.h (I don't count container_of()
as a function). With that being said I would rather see it to be moved
to port.c altogether (yes, it's more invasive change, but looks more
consistent).  I would even dare to move struct usb_port (and container_of()
accompanied with that) and this function to port.h. This might require
a separate patch, though.

I agree with the reasoning, especially given the function name. However
I wonder if it would cause problems given the linking order. I'll give
it a try nevertheless and report back.

Thanks!

In case it won't fly (but I still think it's better to split), can you at least
group usb_port APIs and struct? Means moving the proto closer to that struct
usb_port followed by container_of().

Looks like it works out fine. port.o is linked in after hub.o, so moving
the function from the latter to the former doesn't cause issues. One might
even say that it is the correct thing to do.

quoted

quoted

Perhaps something like: 1) "move struct usb_port and associated APIs to port.h";
2) "...this patch...".

There are still some left in hub.c:

static void usb_lock_port(struct usb_port *port_dev)
static void usb_unlock_port(struct usb_port *port_dev)
static void port_over_current_notify(struct usb_port *port_dev)
static void hub_usb3_port_prepare_disable(struct usb_hub *hub,
                                          struct usb_port *port_dev)

These are used only in hub.c and moving them probably makes things
messier?

OOTH, the following bits are in port.c but declared in hub.h:

int usb_hub_create_port_device(struct usb_hub *hub,
         int port1);
void usb_hub_remove_port_device(struct usb_hub *hub,
         int port1);

Any preferences for these while we're reorganizing the code?


ChenYu