Messages in this thread | | | Date | Thu, 23 Jul 2009 22:30:40 +0200 | Subject | Re: [PATCH 1/2] MMC Agressive clocking framework v5 | From | Linus Walleij <> |
| |
2009/7/23 David Vrabel <david.vrabel@csr.com>: > Linus Walleij wrote: >> 2009/7/22 David Vrabel <david.vrabel@csr.com>:
>>> 1. With some controllers (e.g., PXA270 I think) turning the clock on and >>> off is slow. This means if you're doing back-to-back commands you >>> should leave the clock on for best performance. >> >> OK, when I've been testing this using the default workqueue and >> schedule_work() covered these cases. Back-on-back commands >> seemingly doesn't allow the timeout work to schedule, but I might be >> overseeing the case of several CPU:s there though :-/ > > Ok, with a configurable timeout your scheme is fine. The timeout can be > extended beyond 8 SDCLKs if this is beneficial.
Yep I have that in debugfs, but when I look at Adrians code I see he instead added a disable delay field to the host struct so let's use his patch instead then.
> I'm not sure what the best way to add this would be. You could: > > 1. Have a special clock frequency to mean idle and fix up all existing > controller drivers to interpret this as 400 kHz unless you know the > controller handles SDIO interrupts with no SDCLK. > > or: > > 2. Add an additional controller method (set_bus_state?) and only provide > this on controller drivers you're interested in.
As discussed with Adrian this is what his patch does (adding a new host->ops function for enable/disable) so let's use his patch.
>>> 3. Regardless of point 1 above. Using a workqueue item in this way >>> seems overkill. Consider using a timer and simply calling mod_timer() >>> at the start of every command. When the timer expires, idle the clock. >>> You will probably need a "command in progress" bit to ensure you don't >>> idle the clock if the timer expires in the middle of a command. >> >> I would agree if I created a new workqueue, but the timeout of this >> particular workqueue is unimportant and that's why I'm using the >> global workqueue and just schedule_work(). This means no extra >> overhead, no extra thread and basically does the exact same thing. > > You currently queue a work item and wake the workqueue every command. > This is considerably more overhead (when doing back-to-back commands) > than simply calling mod_timer(). > > You also potentially delay for a considerable amount of time in the work > item.
Yep that's the idea almost... But let's raise the timer debate again with Adrian's patch instead :-)
Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |