On Thu, Mar 15, 2012 at 07:56:31AM +0100, Thierry Reding wrote: > * Ryan Mallon wrote: > > On 15/03/12 02:56, Thierry Reding wrote: > > > > > Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx> > > > --- > > > Changes in v3: > > > - update PWM ops for changes in patch 2 > > > > Couple of quick notes, mostly for future work. > > > > > + /* NOTE: the clock to PWM has to be enabled first > > > + * before writing to the registers > > > + */ > > > + clk_enable(pc->clk); > > > > > > Should be fixed to also call clk_prepare (and clk_unprepare after > > clk_disable). Could be done in a follow up patch. > > > > > + __raw_writel(prescale, pc->mmio_base + offset + PWMCR); > > > + __raw_writel(dc, pc->mmio_base + offset + PWMDCR); > > > + __raw_writel(pv, pc->mmio_base + offset + PWMPCR); > > > > > > Should we fix this driver to use readl/writel instead of the __raw > > variants? The memory is properly ioremaped, and to my understanding the > > __raw memory accessors should be avoided outside of core code. This > > could be done in a follow up patch if you want to keep this patch as > > mostly just a move of the code. > > Actually I wasn't planning on keeping this patch at all. Sascha already has > the existing PWM providers converted to his original framework and offered to > rebase them onto this series once the dust settles. I only used them as > testbed to see how the driver interface works out for different hardware. But > I also think that if Sascha hasn't cleaned the driver up yet it should either > be done in his conversion patches or as follow ups. All I have is a simple conversion to the new framework, nothing more. I rebased my patches yesterday onto this series (unfortunately due to the additional argument pwm_device the patches do not look like just moving the drivers anymore). I also skipped the PXA patch since I saw that you already have this one. I just posted the patches to the list. > > Sascha: how do you plan on going forward with this? It seems like the driver > interface is pretty much done now and I expect the next round to be the last, > unless I forget to properly work through the TODO list again. If you are busy > with other stuff I can probably find some time to help with porting your > converted drivers. I haven't done drivers/mfd/twl6030-pwm.c. This one needs a bit more work as it currently does not register itself as a subdriver to the twl6030 but just uses a globally available twl_i2c_write_u8() function. So if you have some time to spare it would be great if you could do this. > > I'm also wondering which tree this will go in through. Does it make sense to > have an extra tree just for the PWM framework or can it go in via some other > general purpose tree? Who do I need to prod? Good question, I don't know. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
