On Fri, Dec 6, 2019 at 11:00 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, 6 Dec 2019, Ikjoon Jang wrote: > > > On Thu, Dec 5, 2019 at 10:26 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > > > > > On Thu, Dec 05, 2019 at 03:32:38PM +0800, Ikjoon Jang wrote: > > > > On Wed, Dec 4, 2019 at 3:55 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > > > > > > > But related to my question above, why do you need to do this during > > > > > enumeration? Why not just set the lower interval value in the hub > > > > > driver? > > > > > > > > Because I want device tree's bInterval to be checked against the same rules > > > > defined in usb_parse_endpoint(). e.g. although hardware says its maximum > > > > is 255, but the practical limit is still 0 to 16, so the code can > > > > print warnings when bInterval from device node is too weird. > > > > > > But that could be handled refactoring the code in question or similar. > > > > > > > Yes, that should be worked. I can't exactly figure out how to refactor > > the code for now, but maybe parsed endpoint descriptors are being > > checked with default hard wired bInterval value and after that > > an overridden value should be checked again. > > > > Actually I don't care about the details of software policies. I just want > > all devices to be handled in the same manner without any further > > special treatments. > > > > > The fundamental problem here is that you're using devicetree, which is > > > supposed to only describe the hardware, to encode policy which should be > > > deferred to user space. > > > > The hub hardware has a default bInterval inside which is actually > > adjustable. So I can think setting bInterval is to describe the hardware > > rather than policy. > > If the hardware is adjustable, why don't you adjust the hardware > instead of changing the software? sorry, I meant "hardware has a default value but it's actually adjustable (by software)". Adjusting hardware is the best option but our hub doesn't allow to do that, so the current approach is patching a hardware descriptor on enumeration stage. > > > > So I think you need to figure out an interface that allows user space to > > > set the polling interval for any hub at runtime instead. > > > > Changing the interval at runtime is an another way to solve the > > power consumption problem, but it's not so easy. At least xhci needs > > to restart an endpoint and no devices are changing the interval after > > enumeration stage. > > Restarting endpoints is easy; just call usb_set_interface(). I thought just changing urb->interval at runtime will be more acceptable. Maybe I'll need an another approach if this patch is unacceptable. Thank you! > > Alan Stern >
