On Thu, Feb 2, 2012 at 7:02 AM,  [off-list ref] wrote:

For this clock, also set clk->rate = (actual rate of the ADC input clock).
Something like clk->rate = clk->get_rate(clk->parent) / (computed divider)
If the clk->rate stays at 0, the clk_get_rate() function for the ADC will
return 13Mhz instead of around 4.5MHz.

(I know the driver doesn't use clk_get_rate(), but this might save some
debugging later if it ever does).

+
+ ? ? ? if (enable == 0)
+ ? ? ? ? ? ? ? __raw_writel(0, clk->enable_reg);
+ ? ? ? else
+ ? ? ? ? ? ? ? __raw_writel(clk->enable_mask, clk->enable_reg);
+
+ ? ? ? return 0;
+}
+

...
...

+config LPC32XX_ADC
+ ? ? ? tristate "NXP LPC32XX ADC"
+ ? ? ? depends on ARCH_LPC32XX && !TOUCHSCREEN_LPC32XX
+ ? ? ? help
+ ? ? ? ? Say yes here to build support for the integrated ADC inside the
+ ? ? ? ? LPC32XX SoC. Note that this feature uses the same hardware as the
+ ? ? ? ? touchscreen driver, so you can only select one of the two drivers
+ ? ? ? ? (lpc32xx_adc or lpc32xx_ts). Provides direct access via sysfs.

I like how clean this driver is. Thanks for taking the time to write and submit
this.

...
...

+ ? ? ? platform_set_drvdata(pdev, info);
+
+ ? ? ? device_init_wakeup(&pdev->dev, 1);

I don't think you need this for this driver.

+ ? ? ? init_completion(&info->completion);
+
+ ? ? ? printk(KERN_INFO "LPC32XX ADC driver loaded, IRQ %d\n", info->irq);

dev_info instead of printk

+
+ ? ? ? return 0;
+
+errout6:
+ ? ? ? clk_disable(info->clk);
+ ? ? ? clk_put(info->clk);
+errout5:
+ ? ? ? iio_device_unregister(info->dev);
+errout4:
+ ? ? ? iounmap(info->adc_base);
+errout3:
+ ? ? ? iio_free_device(info->dev);
+errout1:
+ ? ? ? return retval;
+}
+
+static int __devexit lpc32xx_adc_remove(struct platform_device *pdev)
+{
+ ? ? ? struct lpc32xx_adc_info *info = platform_get_drvdata(pdev);
+
+ ? ? ? free_irq(info->irq, info->dev);
+ ? ? ? platform_set_drvdata(pdev, NULL);
+ ? ? ? clk_disable(info->clk);
+ ? ? ? clk_put(info->clk);
+ ? ? ? iio_device_unregister(info->dev);
+ ? ? ? iio_free_device(info->dev);
+ ? ? ? iounmap(info->adc_base);
+ ? ? ? kfree(info);

This may be ok. Needs a sanity check only.
kfree() is used in remove() but not used in probe() failure path. Might be
missing in probe() or not needed here.

+
+ ? ? ? printk(KERN_INFO "LPC32XX ADC driver unloaded\n");

dev_info()

+
+ ? ? ? return 0;
+}
+
+#if defined(CONFIG_SUSPEND)
+static int lpc32xx_adc_suspend(struct device *dev)
+{
+ ? ? ? struct lpc32xx_adc_info *info = dev_get_drvdata(dev);
+
+ ? ? ? clk_disable(info->clk);

The ADC block doesn't have to remain clocked when it's not being used.
You might just encapsulate the main ADC read code with clock enable/disable
and remove the suspend code and enable/disable code in probe/remove.
This should save a little power when the ADC is idle.

+ ? ? ? return 0;
+}
+
+static int lpc32xx_adc_resume(struct device *dev)
+{
+ ? ? ? struct lpc32xx_adc_info *info = dev_get_drvdata(dev);
+
+ ? ? ? clk_enable(info->clk);
+ ? ? ? return 0;
+}
+
+static const struct dev_pm_ops lpc32xx_adc_pm_ops = {
+ ? ? ? .suspend ? ? ? ?= lpc32xx_adc_suspend,
+ ? ? ? .resume ? ? ? ? = lpc32xx_adc_resume,
+};
+#define LPC32XX_ADC_PM_OPS (&lpc32xx_adc_pm_ops)
+#else
+#define LPC32XX_ADC_PM_OPS NULL
+#endif
+
+static struct platform_driver lpc32xx_adc_driver = {
+ ? ? ? .probe ? ? ? ? ?= lpc32xx_adc_probe,
+ ? ? ? .remove ? ? ? ? = __devexit_p(lpc32xx_adc_remove),
+ ? ? ? .driver ? ? ? ? = {
+ ? ? ? ? ? ? ? .name ? = MOD_NAME,
+ ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
+ ? ? ? ? ? ? ? .pm ? ? = LPC32XX_ADC_PM_OPS,
+ ? ? ? },
+};
+
+static int __init lpc32xx_adc_init(void)
+{
+ ? ? ? return platform_driver_register(&lpc32xx_adc_driver);
+}
+
+static void __exit lpc32xx_adc_exit(void)
+{
+ ? ? ? platform_driver_unregister(&lpc32xx_adc_driver);
+}
+
+module_init(lpc32xx_adc_init);
+module_exit(lpc32xx_adc_exit);
+
+MODULE_AUTHOR("Roland Stigge [off-list ref]");
+MODULE_DESCRIPTION("LPC32XX ADC driver");
+MODULE_LICENSE("GPL");

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel