Re: [PATCH v3 06/13] usb: hub: Pass |struct usb_port*| to usb_port_is_power_on()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Jul 03, 2026 at 07:03:07PM +0800, Chen-Yu Tsai wrote:
> > > 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().

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

-- 
With Best Regards,
Andy Shevchenko