On Fri, Oct 11, 2024 at 12:14:00AM +0800, Guan-Yu Lin wrote: > On Thu, Oct 10, 2024 at 2:33 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, Oct 10, 2024 at 01:30:00PM +0800, Guan-Yu Lin wrote: > > > On Wed, Oct 9, 2024 at 8:44 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > On Wed, Oct 09, 2024 at 05:42:57AM +0000, Guan-Yu Lin wrote: > > > > > + parent = parent->parent; > > > > > + } while (parent); > > > > > > > > Woah, walking up the device chain? That should not be needed, or if so, > > > > then each device's "usage count" is pointless. > > > > > > > > > > Say a hub X with usb devices A,B,C attached on it, where usb device A > > > is actively used by sideband now. We'd like to introduce a mechanism > > > so that hub X won't have to iterate through all its children to > > > determine sideband activities under this usb device tree. > > > > Why would a hub care? > > > > Without the information of sideband activities on the usb devices > connected to the hub, the hub couldn't determine if it could suspend > or not. You are talking about an "internal" hub, right? And isn't this already covered by the original sideband patchset? If not, how is power management being handled there? > > > This problem > > > is similar to runtime suspending a device, where rpm uses > > > power.usage_count for tracking activity of the device itself and > > > power.child_count to check the children's activity. In our scenario, > > > we don't see the need to separate activities on the device itself or > > > on its children. > > > > But that's exactly what is needed here, if a hub wants to know what is > > happening on a child device, it should just walk the list of children > > and look :) > > > > > So we combine two counters in rpm as sb_usage_count, > > > > Combining counters is almost always never a good idea and will come back > > to bite you in the end. Memory isn't an issue here, speed isn't an > > issue here, so why not just do it properly? > > > > By combining the two comments above, my understanding is that we should either: > 1. separating the counter to one recording the sideband activity of > itself, one for its children. > 2. walk the list of children to check sideband activities on demand. > Please correct me if I mistake your messages. I think 2 is better, as this is infrequent and should be pretty fast to do when needed, right? But I really don't care, just don't combine things together that shouldn't be combined. > > > denoting the sideband activities under a specific usb device. We have > > > to keep a counter in each device so that we won't influence the usb > > > devices that aren't controlled by a sideband. > > > > I can understand that for the device being "controlled" by a sideband, > > but that's it. > > > > > When sideband activity changes on a usb device, its usb device parents > > > should all get notified to maintain the correctness of sb_usage_count. > > > > Why "should" they? They shouldn't really care. > > > > Hubs need the sideband activity information on downstream usb devices, > so that the hub won't suspend the upstream usb port when there is a > sideband accessing the usb device connected to it. Then why doesn't the sideband code just properly mark the "downstream" device a being used like any other normal device? Why is this acting "special"? thanks, greg k-h
