On Saturday 04 February 2012 05:46 PM, Aneesh V wrote: > EMIF is an SDRAM controller used in various Texas Instruments > SoCs. EMIF supports, based on its revision, one or more of > LPDDR2/DDR2/DDR3 protocols. > > Add the basic infrastructure for EMIF driver that includes > driver registration, probe, parsing of platform data etc. > > Signed-off-by: Aneesh V <[email protected]> > --- > drivers/misc/Kconfig | 12 ++ > drivers/misc/Makefile | 1 + > drivers/misc/emif.c | 300 > +++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/emif.h | 160 ++++++++++++++++++++++++++ > 4 files changed, 473 insertions(+), 0 deletions(-) > create mode 100644 drivers/misc/emif.c > create mode 100644 include/linux/emif.h > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 8337bf6..d68184a 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -459,6 +459,18 @@ config DDR > information. This data is useful for drivers handling > DDR SDRAM controllers. > > +config EMIF
Add TI prefix here since it's TI IP and not a generic one. > + tristate "Texas Instruments EMIF driver" > + select DDR > + help > + This driver is for the EMIF module available in Texas Instruments > + SoCs. EMIF is an SDRAM controller that, based on its revision, > + supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols. > + This driver takes care of only LPDDR2 memories presently. The > + functions of the driver includes re-configuring AC timing > + parameters and other settings during frequency, voltage and > + temperature changes > + > config ARM_CHARLCD > bool "ARM Ltd. Character LCD Driver" > depends on PLAT_VERSATILE > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 4759166..076db0f 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT) += c2port/ > obj-$(CONFIG_IWMC3200TOP) += iwmc3200top/ > obj-$(CONFIG_HMC6352) += hmc6352.o > obj-$(CONFIG_DDR) += jedec_ddr_data.o > +obj-$(CONFIG_EMIF) += emif.o > obj-y += eeprom/ > obj-y += cb710/ > obj-$(CONFIG_SPEAR13XX_PCIE_GADGET) += spear13xx_pcie_gadget.o > diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c > new file mode 100644 > index 0000000..ba1e3f9 > --- /dev/null > +++ b/drivers/misc/emif.c > @@ -0,0 +1,300 @@ > +/* > + * EMIF driver > + * > + * Copyright (C) 2010 Texas Instruments, Inc. Fix year. 2012 > + * > + * Aneesh V <[email protected]> > + * Santosh Shilimkar <[email protected]> > + * > + * 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. > + */ > +#include <linux/kernel.h> > +#include <linux/reboot.h> > +#include <linux/emif.h> > +#include <linux/io.h> > +#include <linux/device.h> > +#include <linux/platform_device.h> > +#include <linux/interrupt.h> > +#include <linux/slab.h> > +#include <linux/seq_file.h> > +#include <linux/module.h> > +#include <linux/spinlock.h> > +#include "emif_regs.h" > + > +/** > + * struct emif_data - Per device static data for driver's use > + * @duplicate: Whether the DDR devices attached to > this EMIF > + * instance are exactly same as that on EMIF1. In > + * this case we can save some memory and processing > + * @temperature_level: Maximum temperature of LPDDR2 devices > attached > + * to this EMIF - read from MR4 register. If there > + * are two devices attached to this EMIF, this > + * value is the maximum of the two temperature > + * levels. > + * @irq: IRQ number > + * @lock: lock for protecting temperature_level and > + * associated actions from race conditions > + * @base: base address of memory-mapped IO registers. > + * @dev: device pointer. > + * @addressing table with addressing information from > the spec > + * @regs_cache: An array of 'struct emif_regs' that > stores > + * calculated register values for different > + * frequencies, to avoid re-calculating them on > + * each DVFS transition. > + * @curr_regs: The set of register values used in the > last > + * frequency change (i.e. corresponding to the > + * frequency in effect at the moment) > + * @plat_data: Pointer to saved platform data. > + */ > +struct emif_data { > + u8 duplicate; > + u8 temperature_level; > + u32 irq; > + spinlock_t lock; /* lock to prevent races */ > + void __iomem *base; > + struct device *dev; > + const struct lpddr2_addressing *addressing; > + struct emif_regs *regs_cache[EMIF_MAX_NUM_FREQUENCIES]; > + struct emif_regs *curr_regs; > + struct emif_platform_data *plat_data; > +}; > + > +static struct emif_data *emif1; > + > +static void __exit cleanup_emif(struct emif_data *emif) > +{ > + if (emif) { > + if (emif->plat_data) { > + kfree(emif->plat_data->custom_configs); > + kfree(emif->plat_data->timings); > + kfree(emif->plat_data->min_tck); > + kfree(emif->plat_data->device_info); > + kfree(emif->plat_data); > + } > + kfree(emif); > + } > +} > + You might want to consider use of devm_kzalloc() kind of API so that you can get rid of above free stuff. > +static void get_default_timings(struct emif_data *emif) > +{ > + struct emif_platform_data *pd = emif->plat_data; > + > + pd->timings = lpddr2_jedec_timings; > + pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings); > + > + dev_warn(emif->dev, "Using default timings\n"); > +} Would be good if you add __line__ in all the errors and warnings. Helps to trace messages faster. > + > +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 > phy_type, > + u32 ip_rev, struct device *dev) > +{ > + int valid; > + > + valid = (type == DDR_TYPE_LPDDR2_S4 || > + type == DDR_TYPE_LPDDR2_S2) > + && (density >= DDR_DENSITY_64Mb > + && density <= DDR_DENSITY_8Gb) > + && (io_width >= DDR_IO_WIDTH_8 > + && io_width <= DDR_IO_WIDTH_32); > + > + /* Combinations of EMIF and PHY revisions that we support today */ > + switch (ip_rev) { > + case EMIF_4D: > + valid = valid && (phy_type == EMIF_PHY_TYPE_ATTILAPHY); > + break; > + case EMIF_4D5: > + valid = valid && (phy_type == EMIF_PHY_TYPE_INTELLIPHY); > + break; > + default: > + valid = 0; > + } > + > + if (!valid) > + dev_err(dev, "Invalid DDR details\n"); > + return valid; Generally return 0 means success. you might want to consider returning -EINVAL or similar. > +} > + > +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, > + struct device *dev) > +{ > + int valid = 1; > + > + if ((cust_cfgs->mask & EMIF_CUSTOM_CONFIG_LPMODE) && > + (cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE)) > + valid = cust_cfgs->lpmode_freq_threshold && > + cust_cfgs->lpmode_timeout_performance && > + cust_cfgs->lpmode_timeout_power; > + > + if (cust_cfgs->mask & EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL) > + valid = valid && cust_cfgs->temp_alert_poll_interval_ms; > + > + if (!valid) > + dev_warn(dev, "Invalid custom configs\n"); > + > + return valid; > +} > + Ditto > +static struct emif_data * __init get_device_details( > + struct platform_device *pdev) > +{ > + u32 size; > + struct emif_data *emif = NULL; > + struct ddr_device_info *dev_info; > + struct emif_platform_data *pd; > + > + pd = pdev->dev.platform_data; > + extra line > + if (!(pd && pd->device_info && is_dev_data_valid(pd->device_info->type, > + pd->device_info->density, pd->device_info->io_width, > + pd->phy_type, pd->ip_rev, &pdev->dev))) > + goto error; > + > + emif = kzalloc(sizeof(struct emif_data), GFP_KERNEL); > + pd = kmemdup(pd, sizeof(*pd), GFP_KERNEL); > + dev_info = kmemdup(pd->device_info, > + sizeof(*pd->device_info), GFP_KERNEL); > + here too > + if (!emif || !pd || !dev_info) > + goto error; > + > + emif->plat_data = pd; > + emif->dev = &pdev->dev; > + emif->temperature_level = SDRAM_TEMP_NOMINAL; > + and here > + pd->device_info = dev_info; > + > + /* > + * For EMIF instances other than EMIF1 see if the devices connected > + * are exactly same as on EMIF1(which is typically the case). If so, > + * mark it as a duplicate of EMIF1 and skip copying timings data. > + * This will save some memory and some computation later. > + */ > + emif->duplicate = emif1 && (memcmp(dev_info, > + emif1->plat_data->device_info, > + sizeof(struct ddr_device_info)) == 0); > + and here > + if (emif->duplicate) { > + pd->timings = NULL; > + pd->min_tck = NULL; > + goto out; > + } > + > + /* > + * Copy custom configs - ignore allocation error, if any, as > + * custom_configs is not very critical > + */ > + if (pd->custom_configs) > + pd->custom_configs = kmemdup(pd->custom_configs, > + sizeof(*pd->custom_configs), GFP_KERNEL); > + > + if (pd->custom_configs && > + !is_custom_config_valid(pd->custom_configs, emif->dev)) { > + kfree(pd->custom_configs); > + pd->custom_configs = NULL; > + } > + > + /* > + * Copy timings and min-tck values from platform data. If it is not > + * available or if memory allocation fails, use JEDEC defaults > + */ > + size = sizeof(struct lpddr2_timings) * pd->timings_arr_size; > + if (pd->timings) > + pd->timings = kmemdup(pd->timings, size, GFP_KERNEL); > + > + if (!pd->timings) > + get_default_timings(emif); > + > + if (pd->min_tck) > + pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck), > + GFP_KERNEL); > + > + if (!pd->min_tck) { else on above if should work, right ? > + pd->min_tck = &lpddr2_min_tck; > + dev_warn(emif->dev, "Using default min-tck values\n"); > + } > + > + goto out; > + > +error: > + dev_err(&pdev->dev, "get_device_details() failure!!\n"); > + cleanup_emif(emif); > + return NULL; > + > +out: > + return emif; > +} > + > +static int __init emif_probe(struct platform_device *pdev) > +{ > + struct emif_data *emif; > + struct resource *res; > + > + emif = get_device_details(pdev); > + extra line > + if (!emif) > + goto error; > + > + if (!emif1) > + emif1 = emif; > + > + /* Save pointers to each other in emif and device structures */ > + emif->dev = &pdev->dev; > + platform_set_drvdata(pdev, emif); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + goto error; > + > + emif->base = ioremap(res->start, SZ_1M); > + if (!emif->base) > + goto error; > + > + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (!res) > + goto error; you might want add a line here > + emif->irq = res->start; > + And remove this one > + dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n", > + emif->base, emif->irq); > + return 0; > + > +error: > + dev_err(&pdev->dev, "probe failure!!\n"); > + cleanup_emif(emif); > + return -ENODEV; > +} > + > +static int __exit emif_remove(struct platform_device *pdev) > +{ > + struct emif_data *emif = platform_get_drvdata(pdev); > + > + cleanup_emif(emif); > + > + return 0; > +} > + > +static struct platform_driver emif_driver = { > + .remove = __exit_p(emif_remove), > + .driver = { > + .name = "emif", > + }, > +}; > + > +static int __init emif_register(void) > +{ > + return platform_driver_probe(&emif_driver, emif_probe); > +} > + > +static void __exit emif_unregister(void) > +{ > + platform_driver_unregister(&emif_driver); > +} > + > +module_init(emif_register); > +module_exit(emif_unregister); > +MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:emif"); > +MODULE_AUTHOR("Texas Instruments Inc"); > diff --git a/include/linux/emif.h b/include/linux/emif.h > new file mode 100644 > index 0000000..4ddf20b > --- /dev/null > +++ b/include/linux/emif.h > @@ -0,0 +1,160 @@ > +/* > + * Definitions for EMIF SDRAM controller in TI SoCs > + * > + * Copyright (C) 2010 Texas Instruments, Inc. > + * 2012 > + * Aneesh V <[email protected]> > + * > + * 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. > + */ > +#ifndef __LINUX_EMIF_H > +#define __LINUX_EMIF_H > + > +#ifndef __ASSEMBLY__ > +#include <linux/jedec_ddr.h> > + > +/* > + * Maximum number of different frequencies supported by EMIF driver > + * Determines the number of entries in the pointer array for register > + * cache > + */ > +#define EMIF_MAX_NUM_FREQUENCIES 6 > + > +/* EMIF_PWR_MGMT_CTRL register */ > +/* Low power modes */ Combine above two line comments in one comment. > +#define EMIF_LP_MODE_DISABLE 0 > +#define EMIF_LP_MODE_CLOCK_STOP 1 > +#define EMIF_LP_MODE_SELF_REFRESH 2 > +#define EMIF_LP_MODE_PWR_DN 4 > + > +/* Hardware capabilities */ > +#define EMIF_HW_CAPS_LL_INTERFACE 0x00000001 > + > +/* EMIF IP Revisions */ > +#define EMIF_4D 1 > +#define EMIF_4D5 2 > + > +/* PHY types */ > +#define EMIF_PHY_TYPE_ATTILAPHY 1 > +#define EMIF_PHY_TYPE_INTELLIPHY 2 > + > +/* Custom config requests */ > +#define EMIF_CUSTOM_CONFIG_LPMODE 0x00000001 > +#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL 0x00000002 > + try to aling numbers for all the above defines. With above minor comments, the patch looks fine to me. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
