Hi Rachna,
On Wed, Sep 26, 2012 at 10:50:11AM +0530, Patil, Rachna wrote:
> Add the mfd core driver which supports touchscreen
> and ADC.
> With this patch we are only adding infrastructure to
> support the MFD clients.
>
> Signed-off-by: Patil, Rachna <rac...@ti.com>
> ---
> Changes in v2:
> Merged "[PATCH 5/5] MFD: ti_tscadc: Add check on number of i/p
> channels",
> patch submitted in previous version into this file.
>
> Changes in v3:
> No changes
>
> Changes in v4:
> No changes
>
> drivers/mfd/Kconfig | 9 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/ti_am335x_tscadc.c | 193
> ++++++++++++++++++++++++++++++++++
> include/linux/mfd/ti_am335x_tscadc.h | 133 +++++++++++++++++++++++
> 4 files changed, 336 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mfd/ti_am335x_tscadc.c
> create mode 100644 include/linux/mfd/ti_am335x_tscadc.h
Looks good to me, I only have a few comments:
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b1a1462..e472184 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -94,6 +94,15 @@ config MFD_TI_SSP
> To compile this driver as a module, choose M here: the
> module will be called ti-ssp.
>
> +config MFD_TI_AM335X_TSCADC
> + tristate "TI ADC / Touch Screen chip support"
> + depends on ARCH_OMAP2PLUS
- Why do you need to depend on that architecture ?
- You must select MFD_CORE.
> +static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int
> reg)
> +{
> + return readl(tsadc->tscadc_base + reg);
> +}
> +
> +static void tscadc_writel(struct ti_tscadc_dev *tsadc, unsigned int reg,
> + unsigned int val)
> +{
> + writel(val, tsadc->tscadc_base + reg);
> +}
> +
Have you considered using the regmap MMIO API ? The benefits are not very
obvious in that case, but it's always better to use a common framework.
> +static int __devinit ti_tscadc_probe(struct platform_device *pdev)
> +{
> + struct ti_tscadc_dev *tscadc;
> + struct resource *res;
> + struct clk *clk;
> + struct mfd_tscadc_board *pdata = pdev->dev.platform_data;
> + int irq;
> + int err, ctrl;
> + int clk_value, clock_rate;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "Could not find platform data\n");
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no memory resource defined.\n");
> + return -EINVAL;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no irq ID is specified.\n");
> + return -EINVAL;
> + }
> +
> + /* Allocate memory for device */
> + tscadc = kzalloc(sizeof(struct ti_tscadc_dev), GFP_KERNEL);
devm_kzalloc ?
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
