On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin <eduardo.valentin@xxxxxx> wrote: > This patch introduces a MFD core device driver for > OMAP system control module. > > The control module allows software control of > various static modes supported by the device. It is > composed of two control submodules: general control > module and device (padconfiguration) control > module. > > In this patch, the children defined are for: > . USB-phy pin control > . Bangap temperature sensor > > Device driver is probed with postcore_initcall. > However, as some of the APIs exposed by this driver > may be needed in very early init phase, an early init > class is also available: "early_omap_control". > > Signed-off-by: J Keerthy <j-keerthy@xxxxxx> > Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx> > Signed-off-by: Eduardo Valentin <eduardo.valentin@xxxxxx> > --- [..] > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig > index ad95c7a..222dbad 100644 > --- a/arch/arm/plat-omap/Kconfig > +++ b/arch/arm/plat-omap/Kconfig > @@ -5,6 +5,9 @@ menu "TI OMAP Common Features" > config ARCH_OMAP_OTG > bool > > +config ARCH_HAS_CONTROL_MODULE > + bool > + Thanks for getting rid of OMAP CONFIG here. > choice > prompt "OMAP System Type" > default ARCH_OMAP2PLUS > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 11e4438..25a66d8 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -795,6 +795,15 @@ config MFD_WL1273_CORE > driver connects the radio-wl1273 V4L2 module and the wl1273 > audio codec. > > +config MFD_OMAP_CONTROL > + bool "Texas Instruments OMAP System control module" > + depends on ARCH_HAS_CONTROL_MODULE > + help > + This is the core driver for system control module. This driver > + is responsible for creating the control module mfd child, > + like USB-pin control, pin muxing, MMC-pbias and DDR IO dynamic > + change for off mode. > + > config MFD_OMAP_USB_HOST > bool "Support OMAP USBHS core driver" > depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3 > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 05fa538..00f99d6 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -106,6 +106,7 @@ obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o > obj-$(CONFIG_MFD_VX855) += vx855.o > obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o > obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o > +obj-$(CONFIG_MFD_OMAP_CONTROL) += omap-control-core.o > obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o > obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o > obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o > diff --git a/drivers/mfd/omap-control-core.c b/drivers/mfd/omap-control-core.c > new file mode 100644 > index 0000000..7d8d408 > --- /dev/null > +++ b/drivers/mfd/omap-control-core.c > @@ -0,0 +1,211 @@ > +/* > + * OMAP system control module driver file > + * > + * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/ > + * Contacts: > + * Based on original code written by: > + * J Keerthy <j-keerthy@xxxxxx> > + * Moiz Sonasath <m-sonasath@xxxxxx> > + * MFD clean up and re-factoring: > + * Eduardo Valentin <eduardo.valentin@xxxxxx> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + */ > + > +#include <linux/module.h> > +#include <linux/export.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/io.h> > +#include <linux/err.h> > +#include <linux/of_platform.h> > +#include <linux/of_address.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/omap_control.h> > + > +static struct omap_control *omap_control_module; > + > +/** > + * omap_control_readl: Read a single omap control module register. > + * > + * @dev: device to read from. > + * @reg: register to read. > + * @val: output with register value. > + * > + * returns 0 on success or -EINVAL in case struct device is invalid. > + */ > +int omap_control_readl(struct device *dev, u32 reg, u32 *val) > +{ > + struct omap_control *omap_control = dev_get_drvdata(dev); > + > + if (!omap_control) > + return -EINVAL; > + > + *val = readl(omap_control->base + reg); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(omap_control_readl); > + I might have missed in the last scan, but can you let function return the register value. I am guessing, you did this for error case handling. You might want to stick to read API semantic and just have WARN_ON() to take care of error case. > +/** > + * omap_control_writel: Write a single omap control module register. > + * > + * @dev: device to read from. > + * @val: value to write. > + * @reg: register to write to. > + * > + * returns 0 on success or -EINVAL in case struct device is invalid. > + */ > +int omap_control_writel(struct device *dev, u32 val, u32 reg) > +{ > + struct omap_control *omap_control = dev_get_drvdata(dev); > + unsigned long flags; > + > + if (!omap_control) > + return -EINVAL; > + > + spin_lock_irqsave(&omap_control->reg_lock, flags); > + writel(val, omap_control->base + reg); > + spin_unlock_irqrestore(&omap_control->reg_lock, flags); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(omap_control_writel); > + > +/** > + * omap_control_get: returns the control module device pinter > + * > + * The modules which has to use control module API's to read or write should > + * call this API to get the control module device pointer. > + */ > +struct device *omap_control_get(void) > +{ > + unsigned long flags; > + > + if (!omap_control_module) > + return ERR_PTR(-ENODEV); > + > + spin_lock_irqsave(&omap_control_module->reg_lock, flags); > + omap_control_module->use_count++; > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags); > + > + return omap_control_module->dev; > +} > +EXPORT_SYMBOL_GPL(omap_control_get); > + > +/** > + * omap_control_put: returns the control module device pinter > + * > + * The modules which has to use control module API's to read or write should > + * call this API to get the control module device pointer. > + */ > +void omap_control_put(struct device *dev) > +{ > + struct omap_control *omap_control = dev_get_drvdata(dev); > + unsigned long flags; > + > + if (!omap_control) > + return; > + > + spin_lock_irqsave(&omap_control->reg_lock, flags); > + omap_control->use_count--; > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags); > +} > +EXPORT_SYMBOL_GPL(omap_control_put); > + > +static const struct of_device_id of_omap_control_match[] = { > + { .compatible = "ti,omap3-control", }, > + { .compatible = "ti,omap4-control", }, > + { .compatible = "ti,omap5-control", }, > + { }, > +}; > + > +static int __devinit omap_control_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + void __iomem *base; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct omap_control *omap_control; > + > + omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL); > + if (!omap_control) { > + dev_err(dev, "not enough memory for omap_control\n"); > + return -ENOMEM; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(dev, "missing memory base resource\n"); > + return -EINVAL; > + } > + > + base = devm_request_and_ioremap(dev, res); > + if (!base) { > + dev_err(dev, "ioremap failed\n"); > + return -EADDRNOTAVAIL; > + } > + > + omap_control->base = base; > + omap_control->dev = dev; > + spin_lock_init(&omap_control->reg_lock); > + > + platform_set_drvdata(pdev, omap_control); > + omap_control_module = omap_control; > + > + return of_platform_populate(np, of_omap_control_match, NULL, dev); > +} > + Will the probe get called on multiple devices and race ? > +static int __devexit omap_control_remove(struct platform_device *pdev) > +{ > + struct omap_control *omap_control = platform_get_drvdata(pdev); > + > + spin_lock(&omap_control->reg_lock); > + if (omap_control->use_count > 0) { > + spin_unlock(&omap_control->reg_lock); > + dev_err(&pdev->dev, "device removed while still being used\n"); > + return -EBUSY; > + } > + spin_unlock(&omap_control->reg_lock); > + Do you really need above lock where you are just doing the register read. smp_rmb(), should be enough, I guess. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
