Hi Markus,

On Thu, Feb 20, 2014 at 1:21 PM, Markus Pargmann [off-list ref] wrote:

This is the core driver for imx25 touchscreen/adc driver. The module
has one shared ADC and two different conversion queues which use the
ADC. The two queues are identical. Both can be used for general purpose
ADC but one is meant to be used for touchscreens.

This driver is the core which manages the central components and
registers of the TSC/ADC unit. It manages the IRQs and forwards them to
the correct components.

Signed-off-by: Markus Pargmann <redacted>

That's great :-) Nice work!

The meaning of 'tcq' and 'gcq' acronyms are not obvious. Could they be
written more explicitily?

Also, what does the '...' mean?

+static int mx25_tsadc_domain_map(struct irq_domain *d, unsigned int irq,
+                            irq_hw_number_t hwirq)
+{
+       struct mx25_tsadc_priv *priv = d->host_data;
+
+       irq_set_chip_data(irq, priv);
+       irq_set_chip_and_handler(irq, &mx25_tsadc_irq_chip,
+                                handle_level_irq);
+
+#ifdef CONFIG_ARM
+       set_irq_flags(irq, IRQF_VALID);
+#else
+       irq_set_noprobe(irq);
+#endif

Do we really need these ifdef's? Can't we just assume that CONFIG_ARM
is always selected for this driver?

+static int mx25_tsadc_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct device_node *np = dev->of_node;
+       struct mx25_tsadc_priv *priv;
+       struct resource *iores;
+       int ret;
+       void __iomem *iomem;
+
+       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+       if (!priv)
+               return -ENOMEM;
+
+       iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       iomem = devm_ioremap_resource(dev, iores);
+       if (IS_ERR(iomem)) {
+               dev_err(dev, "Failed to remap iomem\n");
+               return PTR_ERR(iomem);
+       }
+
+       priv->regs = regmap_init_mmio(dev, iomem, &mx25_tsadc);
+       if (IS_ERR(priv->regs)) {
+               dev_err(dev, "Failed to initialize regmap\n");
+               return PTR_ERR(priv->regs);
+       }
+
+       priv->clk = devm_clk_get(dev, "ipg");
+       if (IS_ERR(priv->clk)) {
+               dev_err(dev, "Failed to get ipg clock\n");
+               return PTR_ERR(priv->clk);
+       }
+
+       /* Enable clock and reset the component */
+       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_CLK_EN,
+                       MX25_TGCR_CLK_EN);
+       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_TSC_RST,
+                       MX25_TGCR_TSC_RST);
+
+       /* Setup powersaving mode, but enable internal reference voltage */
+       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_POWERMODE_MASK,
+                       MX25_TGCR_POWERMODE_SAVE);
+       regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_INTREFEN,
+                       MX25_TGCR_INTREFEN);
+
+       ret = mx25_tsadc_setup_irq(pdev, priv);
+       if (ret) {
+               dev_err(dev, "Failed to setup irqs\n");
+               return ret;
+       }
+
+       platform_set_drvdata(pdev, priv);
+
+       of_platform_populate(np, NULL, NULL, dev);
+
+       dev_info(dev, "i.MX25/25 Touchscreen and ADC core driver loaded\n");

You could remove the double '25/25'.

+
+       return 0;
+}
+
+static const struct of_device_id mx25_tsadc_ids[] = {
+       { .compatible = "fsl,imx25-tsadc", },
+       { /* Sentinel */ }
+};
+
+static struct platform_driver mx25_tsadc_driver = {
+       .driver         = {
+               .name   = "mx25-tsadc",
+               .owner  = THIS_MODULE,
+               .of_match_table = mx25_tsadc_ids,
+       },
+       .probe          = mx25_tsadc_probe,
+};
+module_platform_driver(mx25_tsadc_driver);
+
+MODULE_DESCRIPTION("MFD for ADC/TSC for Freescale mx25");
+MODULE_AUTHOR("Markus Pargmann [off-list ref]");
+MODULE_LICENSE("GPL v2");

MODULE_ALIAS() as well?