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

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

quoted

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

-- 
With Best Regards,
Andy Shevchenko