Hi Zhangfei, On Thu, Nov 11, 2010 at 08:19:12AM -0500, zhangfei gao wrote: > Hi, Chirs & Eric > > Thanks for review, here is updated version. > 1. After clk_gating is enabled, set_clock will transfer clock=0, so > clk_disable will be called, currently set_clock will never transfer > clock=0. > Later tune_timing only occurs once clock is started, currently it will > happen when clock is changed. There are still some review comments that haven't been replied to yet: * Eric asked whether you really need to tune on every clock change, or if once at initialization time would be enough. * I asked why we shouldn't just inline tune_timing() at its callsite, since it's a void function that's only called from one place. * Philip points out that SD_CLOCK_AND_BURST_SIZE_SETUP is an MMP2-only register and should be marked as such, and I agree. Even *if* sdhci-pxa wasn't going to support non-MMP2 SoCs¹, you should still mark hardware-specific registers with the hardware they're used on. Speaking generally, please always reply to review comments -- even if it's just to explain why you considered a suggested change and aren't going to make it. Thanks, - Chris. ¹: (.. which doesn't seem to be the case, since you've since posted a patchset that generalizes the driver.) -- Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/> One Laptop Per Child -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
